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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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