| Message ID | 20220919150927.30081-1-marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: credit2: respect credit2_runqueue=all when arranging runqueues | expand |
On 19.09.2022 17:09, Marek Marczykowski-Górecki wrote: > --- a/xen/common/sched/credit2.c > +++ b/xen/common/sched/credit2.c > @@ -996,9 +996,13 @@ 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. This last part is questionable, partly because the command line doc is ambiguous as to which of the two options is intended to "win". I guess one needs to know the original intentions to resolve this. > */ > if ( rqd->refcnt < max_cpus_runq && (ops->cpupool->gran != SCHED_GRAN_cpu || > - cpu_runqueue_siblings_match(rqd, cpu, max_cpus_runq)) ) > + cpu_runqueue_siblings_match(rqd, cpu, max_cpus_runq) || > + opt_runqueue == OPT_RUNQUEUE_ALL) ) Indentation was already broken here. As you're touching it, may I ask that you correct that aspect? Jan
On Tue, Sep 20, 2022 at 11:06:57AM +0200, Jan Beulich wrote: > On 19.09.2022 17:09, Marek Marczykowski-Górecki wrote: > > --- a/xen/common/sched/credit2.c > > +++ b/xen/common/sched/credit2.c > > @@ -996,9 +996,13 @@ 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. > > This last part is questionable, partly because the command line doc is > ambiguous as to which of the two options is intended to "win". I guess > one needs to know the original intentions to resolve this. Right, I've chosen this approach, because you can still emulate the other by setting sufficiently large max_cpus_runq. I can add doc clarification in v2.
On 20.09.22 15:23, Marek Marczykowski-Górecki wrote: > On Tue, Sep 20, 2022 at 11:06:57AM +0200, Jan Beulich wrote: >> On 19.09.2022 17:09, Marek Marczykowski-Górecki wrote: >>> --- a/xen/common/sched/credit2.c >>> +++ b/xen/common/sched/credit2.c >>> @@ -996,9 +996,13 @@ 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. >> >> This last part is questionable, partly because the command line doc is >> ambiguous as to which of the two options is intended to "win". I guess >> one needs to know the original intentions to resolve this. > > Right, I've chosen this approach, because you can still emulate the > other by setting sufficiently large max_cpus_runq. I can add doc > clarification in v2. > I think this is the better approach, as it allows more flexibility. Updating the doc would be mandatory, though. With that added you can have my: Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c index 0e3f89e5378e..9b8ca4d5ae24 100644 --- a/xen/common/sched/credit2.c +++ b/xen/common/sched/credit2.c @@ -996,9 +996,13 @@ 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)) ) + 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> --- The whole thing is under cpu_runqueue_match() already, so maybe cpu_runqueue_siblings_match() isn't needed at all? [resent with fixed xen-devel address...] --- xen/common/sched/credit2.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)