Re: [PATCH] sched: fix race between sched_move_domain() and vcpu_wake()

[George Dunlap <george.dunlap@eu.citrix.com>]

On 11/10/13 12:15, Dario Faggioli wrote:
> On ven, 2013-10-11 at 11:32 +0100, George Dunlap wrote:
>> So going through the code and trying to reconstruct all the state in my
>> head...
>>
>> If you look at vcpu_migrate(), it grabs both locks.  But it looks like
>> the main purpose for that is so that we can call the migrate SCHED_OP(),
>> which for credit2 needs to do some mucking about with runqueues, and
>> thus needs both locks.
>>
> Just to make sure I understood what's going on, the SCHED_OP you're
> referring is SCHED_OP(..,migrate,..) here, right?
>
>> In the case of move_domain, this is unnecessary,
>> since it is removed from the old scheduler and then added to the new one.
>>
> I think that too, and that's why I wouldn't take both locks: it'd
> actually be misleading rather than enlightening for people reading the
> code, at least that's how I see it.
>
> Perhaps, we can put a comment somewhere (as George is also saying).
>
> Regarding the patch, I personally like Jan's idea.
>
>> But I think this patch is still not quite right: both v->processor and
>> per_cpu(schedule_data, ...).schedule_lock may change under your feet; so
>> you always need to do the lock in a loop, checking to make sure that you
>> *still* have the right lock after you have actually grabbed it.
>>
> Which, if I'm not mistaken, we sort of get for free it we go Jan's way,
> don't we?

You mean, we could just call vcpu_schedule_lock..() instead of writing a 
bespoke loop code?  Sure, that's definitely an advantage.

  -George