Re: [Xen-devel] [PATCH v3] x86/vpmu: add cpu hot unplug notifier for vpmu

["Kang, Luwei" <luwei.kang@xxxxxxxxx>]

> >>> On 21.05.17 at 15:09, <luwei.kang@xxxxxxxxx> wrote:
> > v3:
> >  1.add cpu_online() check in vpm_load() and vpmu_arch_destroy();
> > 2.add vpmu_ prefix. rename cpu_callback() to vpmu_cpu_callback();
> 
> I had specifically objected to the latter.

Sorry, will rollback it.

> 
> > @@ -394,8 +395,11 @@ int vpmu_load(struct vcpu *v, bool_t from_guest)
> >      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
> >          return 0;
> >
> > -    /* First time this VCPU is running here */
> > -    if ( vpmu->last_pcpu != pcpu )
> > +    /*
> > +     * The last pCPU is still online and this is the first time this vCPU
> > +     * running here.
> > +     */
> > +    if ( cpu_online(vpmu->last_pcpu) && vpmu->last_pcpu != pcpu )
> 
> Adding a cpu_online() check here is unlikely to be helpful. Actually I may 
> have misguided you with prior comments (and if so, I'm
> sorry) - the LOADED check following this one makes sure on_selected_cpus() 
> won't be called with an offline CPU here. IOW I think
> the code can be left untouched, but the reason why should be spelled out in 
> the commit message (matching the reasoning why
> adding the LOADED check to vpmu_arch_destroy() is sufficient for the second 
> use of last_pcpu there).
> 

So, remove cpu_online() check here, because of LOADED check can make sure don't 
send remote call to an offline cpu (cpu_callback() will
reset this flag).  
The cpu_online() check in vpmu_arch_destroy() should be reserved due to 
per_cpu(last_vcpu, vpmu->last_pcpu) has become an invalid value(Not NULL).
Is that right?

Thanks,
Luwei Kang

> > @@ -575,15 +579,21 @@ static void vpmu_arch_destroy(struct vcpu *v)
> >       * We will test it again in vpmu_clear_last() with interrupts
> >       * disabled to make sure we don't clear someone else.
> >       */
> > -    if ( per_cpu(last_vcpu, vpmu->last_pcpu) == v )
> > +    if ( cpu_online(vpmu->last_pcpu) &&
> > +            per_cpu(last_vcpu, vpmu->last_pcpu) == v )
> 
> Indentation.
> 
> Jan


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