On Tue, May 31, 2016 at 11:31 AM, Wu, Feng <feng...@intel.com> wrote: > > >> -----Original Message----- >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: Friday, May 27, 2016 10:57 PM >> To: Wu, Feng <feng...@intel.com> >> Cc: andrew.coop...@citrix.com; dario.faggi...@citrix.com; >> george.dun...@eu.citrix.com; Tian, Kevin <kevin.t...@intel.com>; xen- >> de...@lists.xen.org; konrad.w...@oracle.com; k...@xen.org >> Subject: Re: [PATCH v2 4/4] VMX: fixup PI descritpor when cpu is offline >> >> >>> On 26.05.16 at 15:39, <feng...@intel.com> wrote: >> > @@ -102,9 +103,10 @@ void vmx_pi_per_cpu_init(unsigned int cpu) >> > { >> > INIT_LIST_HEAD(&per_cpu(vmx_pi_blocking, cpu).list); >> > spin_lock_init(&per_cpu(vmx_pi_blocking, cpu).lock); >> > + per_cpu(vmx_pi_blocking, cpu).down = 0; >> >> This seems pointless - per-CPU data starts out all zero (and there >> are various places already which rely on that). >> >> > @@ -122,10 +124,25 @@ static void vmx_vcpu_block(struct vcpu *v) >> > * new vCPU to the list. >> > */ >> > spin_unlock_irqrestore(&v->arch.hvm_vmx.pi_hotplug_lock, flags); >> > - return; >> > + return 1; >> > } >> > >> > spin_lock(pi_blocking_list_lock); >> > + if ( unlikely(per_cpu(vmx_pi_blocking, v->processor).down) ) >> >> Is this something that can actually happen? vmx_pi_desc_fixup() >> runs in stop-machine context, i.e. no CPU can actively be here (or >> anywhere near the arch_vcpu_block() call sites). > > This is related to scheduler, maybe Dario can give some input about > this. Dario?
Yeah, I was going to say just looking through this -- there's no way that vmx_cpu_dead() will be called while there are vcpus *still running* on that pcpu. vcpus will be migrated to other pcpus before that happens. All of your changes around vmx_vcpu_block() are unnecessary. >> > + new_lock = &per_cpu(vmx_pi_blocking, cpu).lock; >> >> DYM new_cpu here? In fact with ... >> >> > + spin_lock(new_lock); >> >> ... this I can't see how you would have successfully tested this >> new code path, as I can't see how this would end in other than >> a deadlock (as you hold this very lock already). Indeed, it's not very nice to send us complicated code that obviously can't even run. -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
