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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.06.15 at 10:56, <feng.wu@xxxxxxxxx> wrote:

> 
>> -----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.

This is the obvious part you state. What needs to be explained is
why it would be impossible (or a t least a bad idea) to move the
vCPU between blocked lists when its v->processor changes.

Jan


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