On Thu, 2015-07-09 at 15:27 +0100, George Dunlap wrote: > On 07/09/2015 03:18 PM, Dario Faggioli wrote: > > On Thu, 2015-07-09 at 14:44 +0100, Jan Beulich wrote: > >>>>> On 09.07.15 at 14:53, wrote: > > > >>> Consider the following scenario: > >>> - v1 blocks on pcpu 0. > >>> - vcpu_runstate_change() will do everything necessary for v1 on p0. > >>> - The scheduler does load balancing and moves v1 to p1, calling > >>> vcpu_migrate(). Because the vcpu is still blocked, > >>> vcpu_runstate_change() is not called. > >>> - A device interrupt is generated. > >>> > >>> What happens to the interrupt? Does everything still work properly, or > >>> will the device wake-up interrupt go to the wrong pcpu (p0 rather than p1)? > >> > >> I think much of this was discussed before, since I also disliked the > >> hooking into vcpu_runstate_change(). What I remember having > >> been told is that it really only matters which pCPU's list a vCPU is > >> on, not what v->processor says. > >> > > Right. > > > > But, as far as I could understand from the patches I've seen, a vcpu > > ends up in a list when it blocks, and when it blocks there will be a > > context switch, and hence we can deal with the queueing during the the > > context switch itself (which is, in part, an arch specific operation > > already). > > > > What am I missing? > > I think what you're missing is that Jan is answering my question about > migrating a blocked vcpu, not arguing that vcpu_runstate_change() is the > right way to go. At least that's how I understood him. :-) > Eheh, no, I got that... I just wanted to take one more chance to try to get an answer (from Feng, mainly, but from anyone who knows or has an idea, in reality! :-D) on why another approach is not possible. Sorry for abusing the reply for my own mean purposes. :-D > But regarding context_switch: I think the reason we need more hooks than > that is that context_switch only changes into and out of running state. > Sure. > There are also changes that need to happen when you change from blocked > to offline, offline to blocked, blocked to runnable, &c; these don't go > through context_switch. > Right, those ones. And about them, I could say for the third time that having a non-runstate biased description of what we need would help to understand where to do things, whether there are suitable spots already or we need to add new ones, etc.... But I guess I don't want to be so repetitive! :-PP > That's why I was suggesting some architectural > equivalents to the SCHED_OP() callbacks to be added to vcpu_wake &c. > Yep. If (or at least for those of which) really there is no other place already, adding these would be TheRightThing for me as well. > vcpu_runstate_change() is at the moment a nice quiet cul-de-sac that > just does a little bit of accounting; > Indeed! > I'd rather not have it suddenly > become a major thoroughfare for runstate change hooks, if we can avoid > it. :-) > Yep, that was my understanding of it, and my idea too. Regards, Dario -- <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)