Re: [PATCH 3/3] VMX: Remove the vcpu from the per-cpu blocking list after domain termination

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2552 bytes --]

On Mon, 2016-05-23 at 13:32 +0000, Wu, Feng wrote:
> 
> > > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > > @@ -248,6 +248,36 @@ void vmx_pi_hooks_deassign(struct domain *d)
> > >      d->arch.hvm_domain.vmx.pi_switch_to = NULL;
> > >  }
> > > 
> > > +static void vmx_pi_blocking_list_cleanup(struct domain *d)
> > > +{
> > > +    unsigned int cpu;
> > > +
> > > +    for_each_online_cpu ( cpu )
> > > +    {
> > > +        struct vcpu *v;
> > > +        unsigned long flags;
> > > +        struct arch_vmx_struct *vmx, *tmp;
> > > +        spinlock_t *lock = &per_cpu(vmx_pi_blocking, cpu).lock;
> > > +        struct list_head *blocked_vcpus =
> > > &per_cpu(vmx_pi_blocking,
> > > cpu).list;
> > > +
> > > +        spin_lock_irqsave(lock, flags);
> > > +
> > > +        list_for_each_entry_safe(vmx, tmp, blocked_vcpus,
> > > pi_blocking.list)
> > > +        {
> > > +            v = container_of(vmx, struct vcpu, arch.hvm_vmx);
> > > +
> > > +            if (v->domain == d)
> > > +            {
> > > +                list_del(&vmx->pi_blocking.list);
> > > +                ASSERT(vmx->pi_blocking.lock == lock);
> > > +                vmx->pi_blocking.lock = NULL;
> > > +            }
> > > +        }
> > > +
> > > +        spin_unlock_irqrestore(lock, flags);
> > > +    }
> > > 
> > So, I'm probably missing something very ver basic, but I don't see
> > what's the reason why we need this loop... can't we arrange for
> > checking
> > 
> >  list_empty(&v->arch.hvm_vmx.pi_blocking.list)
> Yes, I also cannot find the reason why can't we use this good
> suggestion, Except we need use list_del_init() instead of
> list_del() in the current code. 
>
Yes, I saw that, and it's well worth doing that, to get rid of the
loop. :-)

> Or we can just check whether
> ' vmx->pi_blocking.lock ' is NULL? 
>
I guess that will work as well. Still, if it were me doing this, I'd go
for the list_del_init()/list_empty() approach.

> I total don't know why I
> missed it! :)
> 
:-)

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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