Re: [Xen-devel] Fwd: [v3 14/15] Update Posted-Interrupts Descriptor during vCPU scheduling

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

> -----Original Message-----
> From: Dario Faggioli [mailto:dario.faggioli@xxxxxxxxxx]
> Sent: Thursday, July 02, 2015 4:21 PM
> To: Wu, Feng
> Cc: xen-devel; keir@xxxxxxx; jbeulich@xxxxxxxx; andrew.cooper3@xxxxxxxxxx;
> Tian, Kevin; Zhang, Yang Z; george.dunlap@xxxxxxxxxxxxx
> Subject: Re: Fwd: [v3 14/15] Update Posted-Interrupts Descriptor during vCPU
> scheduling
> 
> On Thu, 2015-07-02 at 04:32 +0000, Wu, Feng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dario Faggioli [mailto:dario.faggioli@xxxxxxxxxx]
> > > Sent: Tuesday, June 30, 2015 10:58 AM
> > > To: Wu, Feng
> > > Cc: xen-devel; keir@xxxxxxx; jbeulich@xxxxxxxx;
> andrew.cooper3@xxxxxxxxxx;
> > > Tian, Kevin; Zhang, Yang Z; george.dunlap@xxxxxxxxxxxxx; Wu, Feng
> > > Subject: Re: Fwd: [v3 14/15] Update Posted-Interrupts Descriptor during
> vCPU
> > > scheduling
> > >
> > > On Mon, 2015-06-29 at 18:36 +0100, Andrew Cooper wrote:
> > >
> > > >
> > > > The basic idea here is:
> > > > 1. When vCPU's state is RUNSTATE_running,
> > > >         - set 'NV' to 'Notification Vector'.
> > > >         - Clear 'SN' to accpet PI.
> > > >         - set 'NDST' to the right pCPU.
> > > > 2. When vCPU's state is RUNSTATE_blocked,
> > > >         - set 'NV' to 'Wake-up Vector', so we can wake up the
> > > >           related vCPU when posted-interrupt happens for it.
> > > >         - Clear 'SN' to accpet PI.
> > > > 3. When vCPU's state is RUNSTATE_runnable/RUNSTATE_offline,
> > > >         - Set 'SN' to suppress non-urgent interrupts.
> > > >           (Current, we only support non-urgent interrupts)
> > > >         - Set 'NV' back to 'Notification Vector' if needed.
> > > >
> > > It might be me, but it feels a bit odd to see RUNSTATE-s being (ab)used
> > > directly for this, as it does feel odd to see arch specific code being
> > > added in there.
> > >
> > > Can't this be done in context_switch(), which is already architecture
> > > specific? I was thinking to something very similar to what has been done
> > > for PSR, i.e., on x86, put everything in __context_switch().
> > >
> > > Looking at who's prev and who's next, and at what pause_flags each has
> > > set, you should be able to implement all of the above logic.
> > >
> > > Or am I missing something?
> >
> > As mentioned in the description of this patch, here we need to do
> > something when the vCPU's state is changed, can we get the
> > state transition in __context_switch(), such as "running -> blocking"?
> >
> Well, in the patch description you mention how you've done it, so of
> course it mentions runstates.
> 
> That does not necessarily means "we need to do something" in
> vcpu_runstate_change(). Actually, that's exactly what I'm asking: can
> you check whether this thing that you need doing can be done somewhere
> else than in vcpu_runstaete_change() ?

Why do you think vcpu_runstaete_change() is not the right place to do this?

> 
> In fact, looking at how, where and what for, runstetes are used, that
> really does not feel right, at least to me. What you seem to be
> interested is whether a vCPU blocks and/or unblocks

Not just blocks, unblocks, we need to track the state transition, and
update posted-interrupt descriptor accordingly.

 Runstates are an
> abstraction, build up on top of (mostly) pause_flags, like _VPF_blocked
> (look at how runstate is updated).
> 
> I think you should not build on top of such abstraction, but on top of
> pause_flags directly.

I don't think building on top of vCPU state is not suitable for my case,
the abstraction is there, why cannot people use it?

Thanks,
Feng

> I had a quick look, and it indeed seems to me that
> you can get all you need from there too. It might even result in the
> code looking simpler (but that's of course hard to tell without actually
> trying). In fact, inside the context switching code, you already know
> that prev was running so, if it has the proper flag set, it means it's
> blocking (i.e., going to RUNSTATE_blocked, in runstates language), if
> not, it maybe is being preempted (i.e., going to RUNSTATE_runnable).
> Therefore, you can enact all your logic, even without any need to keep
> track of the previous runstate, and without needing to build up a full
> state machine and looking at all possible transitions.
> 
> So, can you have a look at whether that solution can fly? Because, if it
> does, I think it would be a lot better.
> 
> 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