Re: [Xen-devel] [RFC v2 13/15] Update Posted-Interrupts Descriptor during vCPU scheduling

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

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Tuesday, June 16, 2015 4:29 PM
> To: Wu, Feng
> Cc: andrew.cooper3@xxxxxxxxxx; george.dunlap@xxxxxxxxxxxxx; Tian, Kevin;
> Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx
> Subject: RE: [RFC v2 13/15] Update Posted-Interrupts Descriptor during vCPU
> scheduling
> 
> >>> On 16.06.15 at 10:13, <feng.wu@xxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: Tuesday, June 16, 2015 3:53 PM
> >> >>> On 16.06.15 at 02:17, <feng.wu@xxxxxxxxx> wrote:
> >> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> >> Sent: Wednesday, June 10, 2015 2:44 PM
> >> >> >>> On 08.05.15 at 11:07, <feng.wu@xxxxxxxxx> wrote:
> >> >> > +
> >> >> > +           /*
> >> >> > +            * Delete the vCPU from the related block list
> >> >> > +            * if we are resuming from blocked state
> >> >> > +            */
> >> >> > +           spin_lock_irqsave(&per_cpu(blocked_vcpu_lock,
> >> >> > +                             v->pre_pcpu), flags);
> >> >> > +           list_del(&v->blocked_vcpu_list);
> >> >> > +           spin_unlock_irqrestore(&per_cpu(blocked_vcpu_lock,
> >> >> > +                                  v->pre_pcpu), flags);
> >> >> > +        }
> >> >> > +        break;
> >> >> > +
> >> >> > +    case RUNSTATE_blocked:
> >> >> > +        /*
> >> >> > +         * The vCPU is blocked on the block list.
> >> >> > +         * Add the blocked vCPU on the list of the
> >> >> > +         * vcpu->pre_pcpu, which is the destination
> >> >> > +         * of the wake-up notification event.
> >> >> > +         */
> >> >> > +        v->pre_pcpu = v->processor;
> >> >>
> >> >> Is latching this upon runstate change really enough? I.e. what about
> >> >> the v->processor changes that sched_move_domain() or individual
> >> >> schedulers do? Or if it really just matters on which CPU's blocked list
> >> >> the vCPU is (while its ->processor changing subsequently doesn't
> >> >> matter) I'd like to see the field named more after its purpose (e.g.
> >> >> pi_block_cpu; list and lock should btw also have a connection to PI
> >> >> in their names).
> >> >
> >> > Yes, It doesn't matter if vCPU changes. The key point is that we put
> >> > the vCPU on a pCPU list and we change the NDST field to this pCPU,
> >> > then the wakeup notification event will get there. You are right, I
> >> > need to rename them to reflect the real purpose of it.
> >> >
> >> >>
> >> >> In the end, if the placement on a list followed v->processor, you
> >> >> would likely get away without the extra new field. Are there
> >> >> synchronization constraints speaking against such a model?
> >> >
> >> > I don't understand this quit well. Do you mean using 'v->processor'
> >> > as the pCPU list for the blocked vCPUs? Then what about 'v->processor'
> >> > changes, seems we cannot handle this case.
> >>
> >> That was the question - does anything speak against such a model?
> >
> > Do you mean still using v->processor as the pCPU to store the blocked vCPU?
> 
> Not "to store", but "to indicate", but yes, the question still is regarding
> the need for the new field. I'm fine with adding the field if there is a
> proper explanation for it being needed; I'm not going to agree to add
> new fields when existing ones can serve the purpose.

Fair enough. The basic reason for adding this new field is 'v->processor' can
be changed behind, so we lost control of it. This is straightforward way I
can think of right now.

Thanks,
Feng

> 
> Jan


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