[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] xen: credit2: limit the max number of CPUs in a runqueue

On 30.04.20 14:28, Dario Faggioli wrote:
On Thu, 2020-04-30 at 09:35 +0200, Jürgen Groß wrote:
On 29.04.20 19:36, Dario Faggioli wrote:

Therefore, let's set a limit to the max number of CPUs that can
share a
Credit2 runqueue. The actual value is configurable (at boot time),
default being 16. If, for instance,  there are more than 16 CPUs in
socket, they'll be split among two (or more) runqueues.

Did you think about balancing the runqueues regarding the number of
cpus? E.g. in case of max being 16 and having 20 cpus to put 10 in
runqueue? I know this will need more logic as cpus are added one by
but the result would be much better IMO.

I know. Point is, CPUs not only are added one by one, but they can,
once the system is running, be offlined/onlined or moved among

Therefore, if we have 20 CPUs, even if we put 10 in each runqueue at
boot, if the admin removes 4 CPUs that happened to be all in the same
runqueue, we end up in an unbalanced (6 vs 10) situation again. So we'd
indeed need full runqueue online rebalancing logic, which will probably
end up being quite complex and I'm not sure it's worth it.

That being said, I can try to make things a bit more fair, when CPUs
come up and are added to the pool. Something around the line of adding
them to the runqueue with the least number of CPUs in it (among the
suitable ones, of course).

With that, when the user removes 4 CPUs, we will have the 6 vs 10
situation. But we would make sure that, when she adds them back, we
will go back to 10 vs. 10, instead than, say, 6 vs 14 or something like

Was something like this that you had in mind? And in any case, what do
you think about it?

Yes, this would be better already.

--- a/xen/common/sched/cpupool.c
+++ b/xen/common/sched/cpupool.c
@@ -37,7 +37,7 @@ static cpumask_t cpupool_locked_cpus;
static DEFINE_SPINLOCK(cpupool_lock); -static enum sched_gran __read_mostly opt_sched_granularity =
+enum sched_gran __read_mostly opt_sched_granularity =

Please don't use the global option value, but the per-cpupool one.

Yep, you're right. Will do.

+/* Additional checks, to avoid separating siblings in different
runqueues. */
+static bool
+cpu_runqueue_smt_match(const struct csched2_runqueue_data *rqd,
unsigned int cpu)
+    unsigned int nr_sibl =
cpumask_weight(per_cpu(cpu_sibling_mask, cpu));

Shouldn't you mask away siblings not in the cpupool?

So, point here is: if I have Pool-0 and Pool-1, each with 2 runqueues
and CPU 0 is in Pool-1, when I add CPU 1 --which is CPU 0's sibling--
to Pool-0, I always want to make sure that there is room for both CPUs
0 and 1 in the runqueue of Pool-0 where I'm putting it (CPU 0). Even if
CPU 1 is currently in another pool.

This way if, in future, CPU 1 is removed from Pool-1 and added to
Pool-0, I am sure it can go in the same runqueue where CPU 0 is. If I
don't consider CPUs which currently are in another pool, we risk that
when/if they're added to this very pool, they'll end up in a different

And we don't want that.

Makes sense?


You should add a comment in this regard.

And you should either reject the case of less cpus per queue than
siblings per core, or you should handle this situation. Otherwise you
won't ever find a suitable run-queue. :-)




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.