Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Wu, Feng
> Sent: Tuesday, May 24, 2016 6:08 PM
> To: Dario Faggioli <dario.faggioli@xxxxxxxxxx>; Jan Beulich 
> <JBeulich@xxxxxxxx>
> Cc: andrew.cooper3@xxxxxxxxxx; george.dunlap@xxxxxxxxxxxxx; Tian, Kevin
> <kevin.tian@xxxxxxxxx>; xen-devel@xxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx;
> keir@xxxxxxx; Wu, Feng <feng.wu@xxxxxxxxx>
> Subject: RE: [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu
> blocking list
> 
> 
> 
> > -----Original Message-----
> > From: Dario Faggioli [mailto:dario.faggioli@xxxxxxxxxx]
> > Sent: Monday, May 23, 2016 8:39 PM
> > To: Jan Beulich <JBeulich@xxxxxxxx>; Wu, Feng <feng.wu@xxxxxxxxx>
> > Cc: andrew.cooper3@xxxxxxxxxx; george.dunlap@xxxxxxxxxxxxx; Tian, Kevin
> > <kevin.tian@xxxxxxxxx>; xen-devel@xxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx;
> > keir@xxxxxxx
> > Subject: Re: [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu
> > blocking list
> >
> > On Mon, 2016-05-23 at 02:51 -0600, Jan Beulich wrote:
> > > > > > On 23.05.16 at 10:44, <feng.wu@xxxxxxxxx> wrote:
> > > > >
> > > > > vCPU-s currently having their v->processor set to the pCPU being
> > > > > hot removed would simply get migrated elsewhere. If that's not
> > > > > accompanied by respective PI blocking list adjustments, then I
> > > > > can
> > > > > only refer you back to the original series' review process, where
> > > > > it was (iirc) more than once that it was pointed out that getting
> > > > > out
> > > > > of sync v->processor and the PI list a vCPU sits on may casue
> > > > > issues. And if there is an issue here, I'm not sure what a good
> > > > > solution would be.
> > > > Okay, here is my understanding about the blocking and vCPU
> > > > migration things, if vCPU is blocking and its v->processor is pCPU
> > > > 0,
> > > > at some point, pCPU0 is removed from the system, it will also
> > > > migrate the vCPU to another pCPU, say, pCPU1 as the response
> > > > to pCPU0 being unplugged, hence, v->processor = pCPU1, right?
> > >
> > > Yes.
> > >
> > See, for instance, cpu_disable_scheduler() in schedule.c. What we do is
> > go over all the vcpus of all domains of either the system or the
> > cpupool, and force the ones that we found with v->processor set to the
> > pCPU that is going down, to perform migration (system_state will be
> > different than SYS_STATE_suspend, and we hence take the 'else' branch).
> >
> > Considering that the pCPU is no longer part of the relevant bitmask-s
> > during the migration, the vCPUs will figure out that they just can't
> > stay there, and move somewhere else.
> 
> Thanks a lot for the elaboration, it is really helpful.
> 
> >
> > Note, however, that this happens for running and runnable vCPUs.
> 
> I don't quite understand this, do you mean cpu_disable_scheduler()
> only handle running and runnable vCPUs, I tried to find some hints
> from the code, but I didn't get it. Could you please give some more
> information about this?
> 
> > If a
> > vCPU is blocker, there is nothing to do, and in fact, nothing happens
> > (as vcpu_sleep_nosync() and vcpu_wake() are NOP in that case).
> 
> What do you mean by saying ' vcpu_sleep_nosync() and vcpu_wake()
> are NOP '?

I think I understand what you meant above now.

Do you think the following idea makes sense?

When a pCPU is unplugged, we can just remove the vcpus on the
associated per-cpu blocking list, then we can choose another online
cpu, set the right NDST value, and put the vCPU the new per-cpu list?
Meanwhile, we may need some special treatment to the case that
vmx_vcpu_block() may run concurrently with cpu hotplug.

Thanks,
Feng

> 
> > For
> > those vCPUs, as soon as they wake up, they'll figure out that their own
> > v->processor is not there any longer, and will move somewhere else.
> 
> So basically, when vCPU is blocking, it has no impact to the blocking vcpu
> when 'v->processor' is removed. When the vCPU is waken up, it will find
> another pCPU to run, since the original 'v->processor' is down and no
> longer in the cpu bitmask, right?
> 
> >
> > > > If that is the case, I think the current code may have issues in
> > > > the
> > > > above case, since 'NDST' filed in PI descriptor is still vCPU0 even
> > > > it is removed from the system. I need to consider how to handle
> > > > this case.
> > >
> > Exactly. I don't think you can do that, for instance, from within
> > cpu_disable_scheduler(), or anythign that gets called from there, as it
> > basically deals with running vCPUs, while you're interested in blocked
> > ones.
> >
> > I wonder whether you can register your own notifier, and, inside it,
> > scan the blocked list and fixup things... (just the first idea that
> > came up in my mind)
> >
> > > > But this is not an issue  in non pCPU hotplug scenario.
> > > >
> > It's probably an issue even if you remove a cpu from a cpupool (and
> > even a more "interesting" one, if you also manage to add it to another
> > pool, in the meantime) isn't it?
> 
> Yes, things become more complex in that case ....
> 
> Thanks,
> Feng
> 
> >
> > 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)

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