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

Re: [Xen-devel] [PATCH] xen: Always ask the scheduler to re-place the vcpu when the affinity changes



>>> On 04.03.13 at 15:22, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 04/03/13 14:23, Keir Fraser wrote:
>> On 04/03/2013 14:03, "George Dunlap" <George.Dunlap@xxxxxxxxxxxxx> wrote:
>>
>>>> But that's probably a rare enough occurrence that it's better overall
>>>> to take the occasional double-migrate.
>>> Hmm -- but thinking it further, it actually seems likely that a
>>> different double-migrate race will happen:
>>>
>>> 1. vcpu is running on pcpu A
>>> 2. pcpu B runs set_affinity, setting VPF_migrate
>>> 3. pcpu B calls vcpu_sleep_nosync
>>> 4. pcpu A wakes up and grabs the schedule lock
>>> 5. pcpu A notices that VPF_migrate is set, and calls vcpu_migrate()
>>> 6. pcpu B calls vcpu_migrate()
>>>
>>> Either that, or 6 happens before 4, but 4 still happens before pcpu B
>>> clears VPF_migrate.
>>>
>>> It seems like we should really only call if (!v->is_running ||
>>> v->processor == this_cpu).
>> It's harmless for vcpu_migrate() to be called twice. It grabs locks then
>> checks it has work to do (and can do that work!).
> 
> If it's calling it twice occasionally, that's fine.  But if it's calling 
> it twice most of the time (and given the context I think that's 
> relatively likely), I think we should try to change that.
> 
> However, that may require some careful reworking; Jan, are you OK with 
> checking the current patch in as-is, and having a separate patch to try 
> to remove the if and avoid the double call?

Yes, with at least the description saying why the check is to remain
for the time being (to avoid unsuspecting readers like me asking why
it's still there).

Jan


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


 


Rackspace

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