Re: [PATCH 3/3] xen: Remove buggy initial placement algorithm

From: Dario Faggioli <dario.faggioli@citrix.com>
To: George Dunlap <george.dunlap@citrix.com>, xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Anshul Makkar <anshul.makkar@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Meng Xu <mengxu@cis.upenn.edu>, Jan Beulich <jbeulich@suse.com>
Subject: Re: [PATCH 3/3] xen: Remove buggy initial placement algorithm
Date: Sat, 16 Jul 2016 15:55:07 +0200	[thread overview]
Message-ID: <1468677307.13039.116.camel@citrix.com> (raw)
In-Reply-To: <1468605722-24239-3-git-send-email-george.dunlap@citrix.com>

[-- Attachment #1.1: Type: text/plain, Size: 2121 bytes --]

On Fri, 2016-07-15 at 19:02 +0100, George Dunlap wrote:
> The initial placement algorithm sometimes picks cpus outside of the
> mask it's given, does a lot of unnecessary bitmasking, does its own
> separate load calculation, and completely ignores vcpu hard and soft
> affinities.  
>
Not to mention that I wouldn't even call what it does "load
calculation".

It just counts the number of vcpus that are executing (or have executed
their last instance) on each CPU, which tells very few about load. And
it does that without any locking at all, which I see the reason why,
but the net effect is that the final result comes from a wrong
calculation done on unreliable data... I don't see how this could be
more broken! :-P

> Just get rid of it and rely on the schedulers to do
> initial placement.
> 
This all makes a lot of sense to me, and I'm all for it, thanks for
doing it! :-)

> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> Since many of scheduler cpu_pick functions have a strong preference
> to
> just leave the cpu where it is (in particular, credit1 and rt), this
> may cause some cpus to be overloaded when creating a lot of domains.
> Arguably this should be fixed in the schedulers themselves.
> 
Indeed. Still, maybe...

> @@ -691,7 +643,7 @@ long
> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>                  continue;
>  
>              cpu = (i == 0) ?
> -                default_vcpu0_location(online) :
> +                cpumask_first(online) :
>                  cpumask_cycle(d->vcpu[i-1]->processor, online);
>
...cpumask_any() ?

It's a bit more expensive, but it at least would provided a less biased
(toward lower CPU indexes) basis to the schedulers?

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel