On Wed, 2018-04-11 at 11:37 +0100, George Dunlap wrote: > On 04/10/2018 11:59 PM, Dario Faggioli wrote: > > > > So, basically, the race is between context_saved() and > > vcpu_set_affinity(). Basically, vcpu_set_affinity() sets the > > VPF_migrating pause flags on a vcpu in a runqueue, with the intent > > of > > letting either a vcpu_sleep_nosync() or a reschedule remove it from > > there, but context_saved() manage to see the flag, before the > > removal > > can happen. > > > Yep, that looks correct. I had considered some sort of race between > set_affinity() and context_switch(), but just never noticed that it > could omit to take the vcpu off the runqueue. > Yeah, it's very subtle. IN fact, when I considered that race, I was assuming that, if we are in context_saved() with VPF_migrating set, the vcpu can't be in any runqueue, as the scheduler would have seen it was not runnable, and not queue it.
I was missing the fact that someone could raise the flag on a vcpu which is already in the runqueue, between when the scheduler lock is dropped, and this check in context_saved()! > > TBH, I have actually never fully understood what that comment > > really > > meant, what the barrier was protecting, and how... e.g., isn't it > > missing its paired one? In fact, there's another comment, clearly > > related, right in vcpu_set_affinity(). But again I'm a bit at loss > > at > > properly figuring out what the big idea is. > > I think the idea is to make sure that the change to v->is_running > happens before whatever happens to come next (i.e., that the compiler > doesn't reorder the write as part of its normal optimization > activities). As it happens nothing that comes next looks like it > really > needs such ordering (particularly as you can't reorder things over a > function call, AFAIUI), but it's good to have those in place in case > anybody *does* add that sort of thing. > Sure, I wasn't planning to remove them. I was curious, and in particular, I was curious of whether they were actually meant at trying to prevent this (or a similar) race... I'll do a bit of archeology, if I find some time. Thanks and Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/
Description: This is a digitally signed message part
_______________________________________________ Xen-devel mailing list Xenfirstname.lastname@example.org https://lists.xenproject.org/mailman/listinfo/xen-devel