* [PATCH v2 6/7] locking/spinlocks, paravirt, hyperv: Correct the hv_nopvspin case [not found] <1561377779-28036-1-git-send-email-zhenzhong.duan@oracle.com> @ 2019-06-24 12:02 ` Zhenzhong Duan 2019-06-27 22:28 ` Sasha Levin 0 siblings, 1 reply; 3+ messages in thread From: Zhenzhong Duan @ 2019-06-24 12:02 UTC (permalink / raw) To: linux-kernel Cc: tglx, mingo, bp, hpa, boris.ostrovsky, jgross, sstabellini, peterz, srinivas.eeda, Zhenzhong Duan, Waiman Long, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ingo Molnar, linux-hyperv With the boot parameter "hv_nopvspin" specified a Hyperv guest should not make use of paravirt spinlocks, but behave as if running on bare metal. This is not true, however, as the qspinlock code will fall back to a test-and-set scheme when it is detecting a hypervisor. In order to avoid this disable the virt_spin_lock_key. Same change for XEN is already in Commit e6fd28eb3522 ("locking/spinlocks, paravirt, xen: Correct the xen_nopvspin case") Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com> Cc: Waiman Long <longman@redhat.com> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: "K. Y. Srinivasan" <kys@microsoft.com> Cc: Haiyang Zhang <haiyangz@microsoft.com> Cc: Stephen Hemminger <sthemmin@microsoft.com> Cc: Sasha Levin <sashal@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: linux-hyperv@vger.kernel.org --- arch/x86/hyperv/hv_spinlock.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c index 07f21a0..d90b4b0 100644 --- a/arch/x86/hyperv/hv_spinlock.c +++ b/arch/x86/hyperv/hv_spinlock.c @@ -64,6 +64,9 @@ __visible bool hv_vcpu_is_preempted(int vcpu) void __init hv_init_spinlocks(void) { + if (unlikely(!hv_pvspin)) + static_branch_disable(&virt_spin_lock_key); + if (!hv_pvspin || !apic || !(ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) || !(ms_hyperv.features & HV_X64_MSR_GUEST_IDLE_AVAILABLE)) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 6/7] locking/spinlocks, paravirt, hyperv: Correct the hv_nopvspin case 2019-06-24 12:02 ` [PATCH v2 6/7] locking/spinlocks, paravirt, hyperv: Correct the hv_nopvspin case Zhenzhong Duan @ 2019-06-27 22:28 ` Sasha Levin 2019-06-28 0:53 ` Zhenzhong Duan 0 siblings, 1 reply; 3+ messages in thread From: Sasha Levin @ 2019-06-27 22:28 UTC (permalink / raw) To: Zhenzhong Duan Cc: linux-kernel, tglx, mingo, bp, hpa, boris.ostrovsky, jgross, sstabellini, peterz, srinivas.eeda, Waiman Long, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Ingo Molnar, linux-hyperv On Mon, Jun 24, 2019 at 08:02:58PM +0800, Zhenzhong Duan wrote: >With the boot parameter "hv_nopvspin" specified a Hyperv guest should >not make use of paravirt spinlocks, but behave as if running on bare >metal. This is not true, however, as the qspinlock code will fall back >to a test-and-set scheme when it is detecting a hypervisor. > >In order to avoid this disable the virt_spin_lock_key. > >Same change for XEN is already in Commit e6fd28eb3522 >("locking/spinlocks, paravirt, xen: Correct the xen_nopvspin case") > >Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com> >Cc: Waiman Long <longman@redhat.com> >Cc: Peter Zijlstra (Intel) <peterz@infradead.org> >Cc: "K. Y. Srinivasan" <kys@microsoft.com> >Cc: Haiyang Zhang <haiyangz@microsoft.com> >Cc: Stephen Hemminger <sthemmin@microsoft.com> >Cc: Sasha Levin <sashal@kernel.org> >Cc: Thomas Gleixner <tglx@linutronix.de> >Cc: Ingo Molnar <mingo@redhat.com> >Cc: Borislav Petkov <bp@alien8.de> >Cc: linux-hyperv@vger.kernel.org >--- > arch/x86/hyperv/hv_spinlock.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c >index 07f21a0..d90b4b0 100644 >--- a/arch/x86/hyperv/hv_spinlock.c >+++ b/arch/x86/hyperv/hv_spinlock.c >@@ -64,6 +64,9 @@ __visible bool hv_vcpu_is_preempted(int vcpu) > > void __init hv_init_spinlocks(void) > { >+ if (unlikely(!hv_pvspin)) >+ static_branch_disable(&virt_spin_lock_key); This should be combined in the conditional under it, which already attempts to disable PV spinlocks, note how hv_pvspin is checked there. hc_pvspin isn't the only reason we would disable PV spinlocks on hyperv. Also, there's no need for the unlikely() here, it's only getting called once... -- Thanks, Sasha ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 6/7] locking/spinlocks, paravirt, hyperv: Correct the hv_nopvspin case 2019-06-27 22:28 ` Sasha Levin @ 2019-06-28 0:53 ` Zhenzhong Duan 0 siblings, 0 replies; 3+ messages in thread From: Zhenzhong Duan @ 2019-06-28 0:53 UTC (permalink / raw) To: Sasha Levin Cc: linux-kernel, tglx, mingo, bp, hpa, boris.ostrovsky, jgross, sstabellini, peterz, srinivas.eeda, Waiman Long, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Ingo Molnar, linux-hyperv On 2019/6/28 6:28, Sasha Levin wrote: > On Mon, Jun 24, 2019 at 08:02:58PM +0800, Zhenzhong Duan wrote: >> With the boot parameter "hv_nopvspin" specified a Hyperv guest should >> not make use of paravirt spinlocks, but behave as if running on bare >> metal. This is not true, however, as the qspinlock code will fall back >> to a test-and-set scheme when it is detecting a hypervisor. >> >> In order to avoid this disable the virt_spin_lock_key. >> >> Same change for XEN is already in Commit e6fd28eb3522 >> ("locking/spinlocks, paravirt, xen: Correct the xen_nopvspin case") >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com> >> Cc: Waiman Long <longman@redhat.com> >> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> >> Cc: "K. Y. Srinivasan" <kys@microsoft.com> >> Cc: Haiyang Zhang <haiyangz@microsoft.com> >> Cc: Stephen Hemminger <sthemmin@microsoft.com> >> Cc: Sasha Levin <sashal@kernel.org> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: linux-hyperv@vger.kernel.org >> --- >> arch/x86/hyperv/hv_spinlock.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/x86/hyperv/hv_spinlock.c >> b/arch/x86/hyperv/hv_spinlock.c >> index 07f21a0..d90b4b0 100644 >> --- a/arch/x86/hyperv/hv_spinlock.c >> +++ b/arch/x86/hyperv/hv_spinlock.c >> @@ -64,6 +64,9 @@ __visible bool hv_vcpu_is_preempted(int vcpu) >> >> void __init hv_init_spinlocks(void) >> { >> + if (unlikely(!hv_pvspin)) >> + static_branch_disable(&virt_spin_lock_key); > > This should be combined in the conditional under it, which already > attempts to disable PV spinlocks, note how hv_pvspin is checked there. > hc_pvspin isn't the only reason we would disable PV spinlocks on hyperv. In virt_spin_lock() there is a comment as below. The test-and-set spinlock is an optimization to hypervisor platform when PV spinlock is unsupported. /* * On hypervisors without PARAVIRT_SPINLOCKS support we fall * back to a Test-and-Set spinlock, because fair locks have * horrible lock 'holder' preemption issues. */ So my understanding is: If hv_pvspin=0 by command line, we want to behave as if running on bare metal(the fair locks path). Though there is performance regression, but it's not that important when we use hv_pvspin=0. If PV spinlock is disabled by other reasons, we prefer the optimization path. > > Also, there's no need for the unlikely() here, it's only getting called > once... Ok, I'll removed it. Thanks Zhenzhong ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-06-28 0:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1561377779-28036-1-git-send-email-zhenzhong.duan@oracle.com>
2019-06-24 12:02 ` [PATCH v2 6/7] locking/spinlocks, paravirt, hyperv: Correct the hv_nopvspin case Zhenzhong Duan
2019-06-27 22:28 ` Sasha Levin
2019-06-28 0:53 ` Zhenzhong Duan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox