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

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

> -----Original Message-----
> From: dunlapg@xxxxxxxxx [mailto:dunlapg@xxxxxxxxx] On Behalf Of George
> Dunlap
> Sent: Thursday, July 09, 2015 7:20 PM
> To: Wu, Feng
> Cc: Dario Faggioli; Tian, Kevin; keir@xxxxxxx; andrew.cooper3@xxxxxxxxxx;
> xen-devel; jbeulich@xxxxxxxx; Zhang, Yang Z
> Subject: Re: [Xen-devel] Fwd: [v3 14/15] Update Posted-Interrupts Descriptor
> during vCPU scheduling
> 
> On Thu, Jul 9, 2015 at 4:09 AM, Wu, Feng <feng.wu@xxxxxxxxx> wrote:
> >> 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?
> 
> Because what the vcpu_runstate_change() function does at the moment is
> *update the vcpu runstate variable*.  It doesn't actually change the
> runstate -- the runstate is changed in the various bits of code that
> call it; and it's not designed to be a generic place to put hooks on
> the runstate changing.
> 
> I haven't done a thorough review of this yet, but at least looking
> through this patch, and skimming the titles, I don't see anywhere you
> handle migration -- what happens if a vcpu that's blocked / offline /
> runnable migrates from one cpu to another?  Is the information
> updated?

Thanks for your review!

The migration is handled in arch_pi_desc_update() which is called
by vcpu_runstate_change().

> 
> The right thing to do in this situation is either to change
> vcpu_runstate_change() so that it is the central place to make all (or
> most) hooks happen;

Yes, this is my implementation. I think vcpu_runstate_change()
is the _central_ place to do things when vCPU state is changed. This
makes things clear and simple. I call an arch hooks to update
posted-interrupt descriptor in this function.


 or to add a set of architectural hooks (similar to
> the SCHED_OP() hooks) in the various places you need them.

I don't have a picture of this method, but from your comments, seems
we need to put the logic to many different places, and must be very
careful so as to not miss some places. I think the above method
is more clear and straightforward, since we have a central place to
handle all the cases. Anyway, if you prefer to this one, it would be
highly appreciated if you can give a more detailed solution! Thank you!

Thanks,
Feng

> 
> I'm inclined to think that the second is the better option; if for no
> other reason that it makes it more clear which states are handled.
> 
>  -George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel