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 <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Peter Zijlstra (Intel) <[email protected]>
Cc: "K. Y. Srinivasan" <[email protected]>
Cc: Haiyang Zhang <[email protected]>
Cc: Stephen Hemminger <[email protected]>
Cc: Sasha Levin <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: [email protected]
---
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

Reply via email to