| Message ID | 20221003162158.2042-1-marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] xen: credit2: respect credit2_runqueue=all when arranging runqueues | expand |
On 03.10.2022 18:21, Marek Marczykowski-Górecki wrote: > Documentation for credit2_runqueue=all says it should create one queue > for all pCPUs on the host. But since introduction > sched_credit2_max_cpus_runqueue, it actually created separate runqueue > per socket, even if the CPUs count is below > sched_credit2_max_cpus_runqueue. > > Adjust the condition to skip syblink check in case of > credit2_runqueue=all. > > Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Reviewed-by: Juergen Gross <jgross@suse.com> > --- > Changes in v2: > - fix indentation > - adjust doc > > The whole thing is under cpu_runqueue_match() already, so maybe > cpu_runqueue_siblings_match() isn't needed at all? > --- > docs/misc/xen-command-line.pandoc | 5 +++++ > xen/common/sched/credit2.c | 9 +++++++-- > 2 files changed, 12 insertions(+), 2 deletions(-) George, Dario - any chance of an ack (or reasons not to)? Jan
On 07.12.2022 12:41, Jan Beulich wrote: > On 03.10.2022 18:21, Marek Marczykowski-Górecki wrote: >> Documentation for credit2_runqueue=all says it should create one queue >> for all pCPUs on the host. But since introduction >> sched_credit2_max_cpus_runqueue, it actually created separate runqueue >> per socket, even if the CPUs count is below >> sched_credit2_max_cpus_runqueue. >> >> Adjust the condition to skip syblink check in case of >> credit2_runqueue=all. >> >> Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") >> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >> Reviewed-by: Juergen Gross <jgross@suse.com> >> --- >> Changes in v2: >> - fix indentation >> - adjust doc >> >> The whole thing is under cpu_runqueue_match() already, so maybe >> cpu_runqueue_siblings_match() isn't needed at all? >> --- >> docs/misc/xen-command-line.pandoc | 5 +++++ >> xen/common/sched/credit2.c | 9 +++++++-- >> 2 files changed, 12 insertions(+), 2 deletions(-) > > George, Dario - any chance of an ack (or reasons not to)? Another two months later I'm inclined to commit this with just Jürgen's R-b (assuming it still applies cleanly). I'll give it a day or two more ... Jan
On 03.10.2022 18:21, Marek Marczykowski-Górecki wrote: > Documentation for credit2_runqueue=all says it should create one queue > for all pCPUs on the host. But since introduction > sched_credit2_max_cpus_runqueue, it actually created separate runqueue > per socket, even if the CPUs count is below > sched_credit2_max_cpus_runqueue. > > Adjust the condition to skip syblink check in case of > credit2_runqueue=all. > > Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Reviewed-by: Juergen Gross <jgross@suse.com> I've now committed this without maintainer ack. > Changes in v2: > - fix indentation I didn't go check v1, but ... > --- a/xen/common/sched/credit2.c > +++ b/xen/common/sched/credit2.c > @@ -996,9 +996,14 @@ cpu_add_to_runqueue(const struct scheduler *ops, unsigned int cpu) > * > * Otherwise, let's try to make sure that siblings stay in the > * same runqueue, pretty much under any cinrcumnstances. > + * > + * Furthermore, try to respect credit2_runqueue=all, as long as > + * max_cpus_runq isn't violated. > */ > - if ( rqd->refcnt < max_cpus_runq && (ops->cpupool->gran != SCHED_GRAN_cpu || > - cpu_runqueue_siblings_match(rqd, cpu, max_cpus_runq)) ) > + if ( rqd->refcnt < max_cpus_runq && > + (ops->cpupool->gran != SCHED_GRAN_cpu || > + cpu_runqueue_siblings_match(rqd, cpu, max_cpus_runq) || > + opt_runqueue == OPT_RUNQUEUE_ALL) ) ... this still looked like too deep indentation to me. I've taken the liberty to adjust this while committing. Jan
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 74b519f0c5bd..057cdb903042 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -724,6 +724,11 @@ Available alternatives, with their meaning, are: * `all`: just one runqueue shared by all the logical pCPUs of the host +Regardless of the above choice, Xen attempts to respect +`sched_credit2_max_cpus_runqueue` limit, which may mean more than one runqueue +for the `all` value. If that isn't intended, raise +the `sched_credit2_max_cpus_runqueue` value. + ### dbgp > `= ehci[ <integer> | @pci<bus>:<slot>.<func> ]` > `= xhci[ <integer> | @pci<bus>:<slot>.<func> ][,share=<bool>|hwdom]` diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c index 0e3f89e5378e..afff23b56238 100644 --- a/xen/common/sched/credit2.c +++ b/xen/common/sched/credit2.c @@ -996,9 +996,14 @@ cpu_add_to_runqueue(const struct scheduler *ops, unsigned int cpu) * * Otherwise, let's try to make sure that siblings stay in the * same runqueue, pretty much under any cinrcumnstances. + * + * Furthermore, try to respect credit2_runqueue=all, as long as + * max_cpus_runq isn't violated. */ - if ( rqd->refcnt < max_cpus_runq && (ops->cpupool->gran != SCHED_GRAN_cpu || - cpu_runqueue_siblings_match(rqd, cpu, max_cpus_runq)) ) + if ( rqd->refcnt < max_cpus_runq && + (ops->cpupool->gran != SCHED_GRAN_cpu || + cpu_runqueue_siblings_match(rqd, cpu, max_cpus_runq) || + opt_runqueue == OPT_RUNQUEUE_ALL) ) { /* * This runqueue is ok, but as we said, we also want an even
Documentation for credit2_runqueue=all says it should create one queue for all pCPUs on the host. But since introduction sched_credit2_max_cpus_runqueue, it actually created separate runqueue per socket, even if the CPUs count is below sched_credit2_max_cpus_runqueue. Adjust the condition to skip syblink check in case of credit2_runqueue=all. Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Juergen Gross <jgross@suse.com> --- Changes in v2: - fix indentation - adjust doc The whole thing is under cpu_runqueue_match() already, so maybe cpu_runqueue_siblings_match() isn't needed at all? --- docs/misc/xen-command-line.pandoc | 5 +++++ xen/common/sched/credit2.c | 9 +++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)