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

[Dario Faggioli <dario.faggioli@citrix.com>]

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

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?

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: 198 bytes --]

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

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