On 07.11.2013, at 07:22, Liu Ping Fan <kernelf...@gmail.com> wrote: > Since kvmppc_hv_find_lock_hpte() is called from both virtmode and > realmode, so it can trigger the deadlock. > > Suppose the following scene: > > Two physical cpuM, cpuN, two VM instances A, B, each VM has a group of vcpus. > > If on cpuM, vcpu_A_1 holds bitlock X (HPTE_V_HVLOCK), then is switched out, > and on cpuN, vcpu_A_2 try to lock X in realmode, then cpuN will be caught in > realmode for a long time. > > What makes things even worse if the following happens, > On cpuM, bitlockX is hold, on cpuN, Y is hold. > vcpu_B_2 try to lock Y on cpuM in realmode > vcpu_A_2 try to lock X on cpuN in realmode > > Oops! deadlock happens > > Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
Very nice catch :). I think it makes a lot of sense to document the fact that kvmppc_hv_find_lock_hpte() should be called with preemption disabled in a comment above the function, so that next time when someone potentially calls it, he knows that he needs to put preempt_disable() around it. Thanks a lot for finding this pretty subtle issue. May I ask how you got there? Did you actually see systems deadlock because of this? Alex > --- > arch/powerpc/kvm/book3s_64_mmu_hv.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c > b/arch/powerpc/kvm/book3s_64_mmu_hv.c > index 043eec8..dbc1478 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c > @@ -474,10 +474,13 @@ static int kvmppc_mmu_book3s_64_hv_xlate(struct > kvm_vcpu *vcpu, gva_t eaddr, > } > > /* Find the HPTE in the hash table */ > + preempt_disable(); > index = kvmppc_hv_find_lock_hpte(kvm, eaddr, slb_v, > HPTE_V_VALID | HPTE_V_ABSENT); > - if (index < 0) > + if (index < 0) { > + preempt_enable(); > return -ENOENT; > + } > hptep = (unsigned long *)(kvm->arch.hpt_virt + (index << 4)); > v = hptep[0] & ~HPTE_V_HVLOCK; > gr = kvm->arch.revmap[index].guest_rpte; > @@ -485,6 +488,7 @@ static int kvmppc_mmu_book3s_64_hv_xlate(struct kvm_vcpu > *vcpu, gva_t eaddr, > /* Unlock the HPTE */ > asm volatile("lwsync" : : : "memory"); > hptep[0] = v; > + preempt_enable(); > > gpte->eaddr = eaddr; > gpte->vpage = ((v & HPTE_V_AVPN) << 4) | ((eaddr >> 12) & 0xfff); > -- > 1.8.1.4 > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev