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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 16/07/16 14:55, Dario Faggioli wrote:
> 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@xxxxxxxxxx>
>> ---
>> 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?

That sounds reasonable -- this is only called when vcpus are created;
and it's almost certainly faster than all the calculations done by
default_vcpu0_location().

 -George

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