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 3:53 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 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?

Thanks,
Feng

> 
> >> > @@ -157,7 +158,11 @@ static inline void vcpu_runstate_change(
> >> >          v->runstate.state_entry_time = new_entry_time;
> >> >      }
> >> >
> >> > +    old_state = v->runstate.state;
> >> >      v->runstate.state = new_state;
> >> > +
> >> > +    if ( is_hvm_vcpu(v) && hvm_funcs.pi_desc_update )
> >> > +        hvm_funcs.pi_desc_update(v, old_state);
> >>
> >> I don't see how this would build on ARM.
> >
> > So what about adding " #ifdef CONFIG_X86 ..." here?
> 
> That would yield ugly code. If this needs to be here, you'll have
> to introduce a suitable arch_...() hook (doing nothing on ARM).
> 
> Jan


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