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

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

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

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