Enable PV qspinlock for Hyper-V
Subsystems (3)
cover Enable PV qspinlock for Hyper-V
This patch adds the necessary Hyper-V specific code to allow PV qspinlock work on Hyper-V.
In wait callback function, read HV_X64_MSR_GUEST_IDLE MSR to trigger the guest's transition to the idle power state which can be exited by an IPI even if IF flag is disabled. Beside that, make HVCALL_NOTIFY_LONG_SPIN_WAIT hypervall to notify the hypervisor that the calling virtual processor is attempting to acquire a resource that is potentially held by another virtual processor within the same partition. This scheduling hint improves the scalability of partitions with more than one virtual processor.
In kick callback function, just send platform IPI to make waiting vcpu exit idle state.
In vcpu_is_preempted callback function, return false directly because Hyper-V does not provide such interface so far.
Yi Sun (3):
X86/Hyper-V: Add Guest IDLE MSR support locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
hv: add description for hv_nopvspin
Documentation/admin-guide/kernel-parameters.txt | 5 ++ arch/x86/hyperv/Makefile | 2 +- arch/x86/hyperv/hv_spinlock.c | 99 +++++++++++++++++++++++++ arch/x86/include/asm/hyperv-tlfs.h | 5 ++ arch/x86/include/asm/mshyperv.h | 3 + arch/x86/kernel/smpboot.c | 2 + 6 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 arch/x86/hyperv/hv_spinlock.c
Signature · 1 line
1.9.1
PATCH 1/3 X86/Hyper-V: Add Guest IDLE MSR support
Hyper-V may expose a HV_X64_MSR_GUEST_IDLE MSR. We can read HYPERV_CPUID_FEATURES eax to check it. Read this MSR can trigger the guest's transition to the idle power state which can be exited by an IPI even if IF flag is disabled.
--- arch/x86/include/asm/hyperv-tlfs.h | 5 +++++ 1 file changed, 5 insertions(+)
arch/x86/include/asm/hyperv-tlfs.h +5 -0
@@ -38,6 +38,8 @@#define HV_MSR_TIME_REF_COUNT_AVAILABLE (1 << 1)/* Partition reference TSC MSR is available */ #define HV_MSR_REFERENCE_TSC_AVAILABLE (1 << 9) +/* Partition Guest IDLE MSR is available */ +#define HV_X64_MSR_GUEST_IDLE_AVAILABLE (1 << 10)/* A partition's reference time stamp counter (TSC) page */ #define HV_X64_MSR_REFERENCE_TSC 0x40000021
@@ -246,6 +248,9 @@#define HV_X64_MSR_STIMER3_CONFIG 0x400000B6#define HV_X64_MSR_STIMER3_COUNT 0x400000B7 +/* Hyper-V guest idle MSR */ +#define HV_X64_MSR_GUEST_IDLE 0x400000F0 +/* Hyper-V guest crash notification MSR's */ #define HV_X64_MSR_CRASH_P0 0x40000100#define HV_X64_MSR_CRASH_P1 0x40000101
Signature · 1 line
1.9.1
PATCH 2/3 locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
In reply to Yi SunFollow PV spinlock mechanism to implement the callback functions to allow the CPU idling and kicking operations on Hyper-V.
HVCALL_NOTIFY_LONG_SPIN_WAIT is a hypercall used by a guest OS to notify the hypervisor that the calling virtual processor is attempting to acquire a resource that is potentially held by another virtual processor within the same partition. This scheduling hint improves the scalability of partitions with more than one virtual processor.
--- arch/x86/hyperv/Makefile | 2 +- arch/x86/hyperv/hv_spinlock.c | 99 +++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/mshyperv.h | 3 ++ arch/x86/kernel/smpboot.c | 2 + 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 arch/x86/hyperv/hv_spinlock.c
arch/x86/hyperv/Makefile +1 -1
@@ -1,2 +1,2 @@ obj-y := hv_init.o mmu.o nested.o -obj-$(CONFIG_X86_64) += hv_apic.o +obj-$(CONFIG_X86_64) += hv_apic.o hv_spinlock.o
arch/x86/hyperv/hv_spinlock.c new file +99 -0
99 lines not on this page · raw message
arch/x86/include/asm/mshyperv.h +3 -0
@@ -355,6 +355,8 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,static inline void hv_apic_init (void ) {}#endif +void __init hv_init_spinlocks (void ); +#else /* CONFIG_HYPERV */ static inline void hyperv_init (void ) {}static inline bool hv_is_hyperv_initialized (void ) {return false; }
@@ -368,6 +370,7 @@ static inline struct hv_vp_assist_page *hv_get_vp_assist_page(unsigned int cpu) return NULL; }static inline int hyperv_flush_guest_mapping (u64 as) {return -1 ; } +void __init hv_init_spinlocks (void ) {}#endif /* CONFIG_HYPERV */ #ifdef CONFIG_HYPERV_TSCPAGE
arch/x86/kernel/smpboot.c +2 -0
@@ -81,6 +81,7 @@#include <asm/cpu_device_id.h> #include <asm/spec-ctrl.h> #include <asm/hw_irq.h> +#include <asm/mshyperv.h> /* representing HT siblings of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY (cpumask_var_t, cpu_sibling_map);
@@ -1335,6 +1336,7 @@ void __init native_smp_prepare_boot_cpu(void)/* already set me in cpu_online_mask in boot_cpu_init() */ cpumask_set_cpu (me, cpu_callout_mask);cpu_set_state_online (me); +hv_init_spinlocks (); }void __init calculate_max_logical_packages (void )
Signature · 1 line
1.9.1
Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
arch/x86/kernel/smpboot.c void __init native_smp_prepare_boot_cpu(void) · line 1339
Quoted 9 lines · Yi Sun
arch/x86/kernel/smpboot.c +2
-0+#include <asm/mshyperv.h> /* representing HT siblings of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY (cpumask_var_t, cpu_sibling_map);@@ -1335,6 +1336,7 @@ void __init native_smp_prepare_boot_cpu(void)/* already set me in cpu_online_mask in boot_cpu_init() */ cpumask_set_cpu (me, cpu_callout_mask);cpu_set_state_online (me); +hv_init_spinlocks ();
No. We have smp_ops.smp_prepare_boot_cpu for that.
Thanks,
tglx
Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
Quoted 13 lines · Thomas Gleixner
arch/x86/kernel/smpboot.c void __init native_smp_prepare_boot_cpu(void) · line 1339
Quoted 9 lines · Yi Sun
arch/x86/kernel/smpboot.c +2
-0+#include <asm/mshyperv.h> /* representing HT siblings of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY (cpumask_var_t, cpu_sibling_map);@@ -1335,6 +1336,7 @@ void __init native_smp_prepare_boot_cpu(void)/* already set me in cpu_online_mask in boot_cpu_init() */ cpumask_set_cpu (me, cpu_callout_mask);cpu_set_state_online (me); +hv_init_spinlocks ();No. We have smp_ops.smp_prepare_boot_cpu for that.
Got it, will change it. Thanks!
Quoted 3 lines
Thanks,
tglx
Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
In reply to Yi SunQuoted 10 lines · Yi Sun
+++
b/arch/x86/hyperv/hv_spinlock.carch/x86/hyperv/hv_spinlock.c new file +8
-0@@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* +* Hyper-V specific spinlock code. +* +* Copyright (C) 2018, Intel, Inc. +* +* Author : Yi Sun <[email protected]>
arch/x86/hyperv/hv_spinlock.c line 18
Quoted 10 lines
arch/x86/hyperv/hv_spinlock.c new file +10
-0+* +* This program is free software; you can redistribute it and/or modify it +* under the terms of the GNU General Public License version 2 as published +* by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or +* NON INFRINGEMENT. See the GNU General Public License for more +* details.
Please remove the boilerplate. The SPDX identifier is enough. If in doubt talk to Intel legal.
arch/x86/hyperv/hv_spinlock.c line 34
Quoted 1 line
arch/x86/hyperv/hv_spinlock.c new file +1
-0+static bool hv_pvspin = true;
__initdata please.
arch/x86/hyperv/hv_spinlock.c line 35
Quoted 1 line
arch/x86/hyperv/hv_spinlock.c new file +1
-0+static u32 spin_wait_info =0 ;
No need for 0 initialization.
arch/x86/hyperv/hv_spinlock.c line 39
Quoted 4 lines
arch/x86/hyperv/hv_spinlock.c new file +4
-0+ +static void hv_notify_long_spin_wait (void ) +{ +u64 input = spin_wait_info |0x00000000ffffffff ;
What? The result is always 0x00000000ffffffff .....
arch/x86/hyperv/hv_spinlock.c line 47
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c new file +7
-0+ spin_wait_info++; +hv_do_fast_hypercall8 (HVCALL_NOTIFY_LONG_SPIN_WAIT , input); +} + +static void hv_qlock_kick (int cpu) +{ + spin_wait_info--;
Looking at the above this is completely pointless and I have no idea what that variable is supposed to do.
Quoted 6 lines
arch/x86/hyperv/hv_spinlock.c new file +6
-0+ apic->send_IPI (cpu,X86_PLATFORM_IPI_VECTOR ); +} + +/* +* Halt the current CPU & release it back to the host +*/
arch/x86/hyperv/hv_spinlock.c line 63
Quoted 10 lines
arch/x86/hyperv/hv_spinlock.c new file +10
-0+static void hv_qlock_wait (u8 *byte,u8 val) +{ +unsigned long msr_val; + +if (READ_ONCE (*byte) != val) +return ; + +hv_notify_long_spin_wait (); + +rdmsrl (HV_X64_MSR_GUEST_IDLE , msr_val);
Magic rdmsrl(). That wants a comment what this is for.
arch/x86/hyperv/hv_spinlock.c line 69
Quoted 4 lines
arch/x86/hyperv/hv_spinlock.c new file +4
-0+/* +* Hyper-V does not support this so far. +*/ +bool hv_vcpu_is_preempted (int vcpu)
static ?
arch/x86/hyperv/hv_spinlock.c line 81
Quoted 12 lines
arch/x86/hyperv/hv_spinlock.c new file +12
-0+{ +return false; +} +PV_CALLEE_SAVE_REGS_THUNK (hv_vcpu_is_preempted); + +void __init hv_init_spinlocks (void ) +{ +if (!hv_pvspin || + !apic || + !(ms_hyperv.hints &HV_X64_CLUSTER_IPI_RECOMMENDED ) || + !(ms_hyperv.features &HV_X64_MSR_GUEST_IDLE_AVAILABLE )) { +pr_warn ("hv: PV spinlocks disabled\n" );
Why does this need to be pr_warn? This is purely informational. Also please use pr_fmt instead of the 'hv:' prefix.
arch/x86/hyperv/hv_spinlock.c line 99
Quoted 6 lines
arch/x86/hyperv/hv_spinlock.c new file +6
-0+static __init int hv_parse_nopvspin (char *arg) +{ + hv_pvspin = false; +return 0 ; +} +early_param ("hv_nopvspin" , hv_parse_nopvspin);
That lacks Documentation of the command line parameter. Wants to be in the same patch.
Thanks,
tglx
Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
Quoted 16 lines · Thomas Gleixner
Quoted 15 lines · Yi Sun
+++
b/arch/x86/hyperv/hv_spinlock.carch/x86/hyperv/hv_spinlock.c new file +13
-0@@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* +* Hyper-V specific spinlock code. +* +* Copyright (C) 2018, Intel, Inc. +* +* Author : Yi Sun <[email protected]> +* +* This program is free software; you can redistribute it and/or modify it +* under the terms of the GNU General Public License version 2 as published +* by the Free Software Foundation. +*
Quoted 9 lines
arch/x86/hyperv/hv_spinlock.c line 18
Quoted 5 lines
arch/x86/hyperv/hv_spinlock.c new file +5
-0+* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or +* NON INFRINGEMENT. See the GNU General Public License for more +* details. Please remove the boilerplate. The SPDX identifier is enough. If in doubt talk to Intel legal.
I will check this. Thanks!
Quoted 8 lines
arch/x86/hyperv/hv_spinlock.c line 34
Quoted 1 line
arch/x86/hyperv/hv_spinlock.c new file +1
-0+static bool hv_pvspin = true;
__initdataplease.arch/x86/hyperv/hv_spinlock.c line 35
Quoted 1 line
arch/x86/hyperv/hv_spinlock.c new file +1
-0+static u32 spin_wait_info =0 ;No need for 0 initialization.
Will modify them.
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c line 39
Quoted 4 lines
arch/x86/hyperv/hv_spinlock.c new file +4
-0+ +static void hv_notify_long_spin_wait (void ) +{ +u64 input = spin_wait_info |0x00000000ffffffff ;What? The result is always
0x00000000ffffffff.....
Oh, sorry for such error.
Quoted 11 lines
arch/x86/hyperv/hv_spinlock.c line 47
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c new file +7
-0+ spin_wait_info++; +hv_do_fast_hypercall8 (HVCALL_NOTIFY_LONG_SPIN_WAIT , input); +} + +static void hv_qlock_kick (int cpu) +{ + spin_wait_info--;Looking at the above this is completely pointless and I have no idea what that variable is supposed to do.
Per Microsoft Hypervisor Top Level Functional Specification, the input parameter of HVCALL_NOTIFY_LONG_SPIN_WAIT is defined as below:
SpinwaitInfo – Specifies the accumulated count the guest was spinning.
So, it is increased when guest is spinning and reduced when spinlock is released.
I may add comments to explain it.
Quoted 10 lines
Quoted 10 lines
arch/x86/hyperv/hv_spinlock.c new file +10
-0+ apic->send_IPI (cpu,X86_PLATFORM_IPI_VECTOR ); +} + +/* +* Halt the current CPU & release it back to the host +*/ +static void hv_qlock_wait (u8 *byte,u8 val) +{ +unsigned long msr_val; +
Quoted 9 lines
arch/x86/hyperv/hv_spinlock.c line 63
Quoted 6 lines
arch/x86/hyperv/hv_spinlock.c new file +6
-0+if (READ_ONCE (*byte) != val) +return ; + +hv_notify_long_spin_wait (); + +rdmsrl (HV_X64_MSR_GUEST_IDLE , msr_val);Magic
rdmsrl(). That wants a comment what this is for.
Per above spec, reading HV_X64_MSR_GUEST_IDLE can make guest idle.
I will add comment here.
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c line 69
Quoted 4 lines
arch/x86/hyperv/hv_spinlock.c new file +4
-0+/* +* Hyper-V does not support this so far. +*/ +bool hv_vcpu_is_preempted (int vcpu)static ?
PV_CALLEE_SAVE_REGS_THUNK definition is below.
#define PV_CALLEE_SAVE_REGS_THUNK(func) \
extern typeof(func) __raw_callee_save_##func; \
......So, the hv_vcpu_is_preempted cannot be declared as 'static'. Otherwise, the make fails with below info.
arch/x86/hyperv/hv_spinlock.o: In function `__raw_callee_save_hv_vcpu_is_preempted':
hv_spinlock.c:(.text+0xd): undefined reference to `hv_vcpu_is_preempted'
Quoted 7 lines
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c new file +7
-0+{ +return false; +} +PV_CALLEE_SAVE_REGS_THUNK (hv_vcpu_is_preempted); + +void __init hv_init_spinlocks (void ) +{
Quoted 9 lines
arch/x86/hyperv/hv_spinlock.c line 81
Quoted 5 lines
arch/x86/hyperv/hv_spinlock.c new file +5
-0+if (!hv_pvspin || + !apic || + !(ms_hyperv.hints &HV_X64_CLUSTER_IPI_RECOMMENDED ) || + !(ms_hyperv.features &HV_X64_MSR_GUEST_IDLE_AVAILABLE )) { +pr_warn ("hv: PV spinlocks disabled\n" );Why does this need to be
pr_warn? This is purely informational. Also please usepr_fmtinstead of the 'hv:' prefix.
Got it. Thanks!
Quoted 10 lines
arch/x86/hyperv/hv_spinlock.c line 99
Quoted 6 lines
arch/x86/hyperv/hv_spinlock.c new file +6
-0+static __init int hv_parse_nopvspin (char *arg) +{ + hv_pvspin = false; +return 0 ; +} +early_param ("hv_nopvspin" , hv_parse_nopvspin);That lacks Documentation of the command line parameter. Wants to be in the same patch.
Will merge patch 3 into 2. Thanks!
Quoted 3 lines
Thanks,
tglx
Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
Quoted 19 lines · Yi Sun
Quoted 6 lines
arch/x86/hyperv/hv_spinlock.c line 39
Quoted 3 lines
arch/x86/hyperv/hv_spinlock.c new file +3
-0+static void hv_notify_long_spin_wait (void ) +{ +u64 input = spin_wait_info |0x00000000ffffffff ;What? The result is always
0x00000000ffffffff.....Oh, sorry for such error.
Quoted 11 lines
arch/x86/hyperv/hv_spinlock.c line 47
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c new file +7
-0+ spin_wait_info++; +hv_do_fast_hypercall8 (HVCALL_NOTIFY_LONG_SPIN_WAIT , input); +} + +static void hv_qlock_kick (int cpu) +{ + spin_wait_info--;Looking at the above this is completely pointless and I have no idea what that variable is supposed to do.
Quoted 7 lines
Per Microsoft Hypervisor Top Level Functional Specification, the input parameter of
HVCALL_NOTIFY_LONG_SPIN_WAITis defined as below:SpinwaitInfo – Specifies the accumulated count the guest was spinning.
So, it is increased when guest is spinning and reduced when spinlock is released.
But that's a global variable, so it might be incremented and decremented by several CPUs at once. I don't have the spec and have no time to study it either, but global does not make any sense to me. The spin wait info comes from a single guest CPU and the wakeup is targeted at that guest CPU as well. So why global? It might be defined that way, but then you really want to explain it proper.
Thanks,
tglx
Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
Quoted 35 lines · Thomas Gleixner
Quoted 19 lines · Yi Sun
Quoted 6 lines
arch/x86/hyperv/hv_spinlock.c line 39
Quoted 3 lines
arch/x86/hyperv/hv_spinlock.c new file +3
-0+static void hv_notify_long_spin_wait (void ) +{ +u64 input = spin_wait_info |0x00000000ffffffff ;What? The result is always
0x00000000ffffffff.....Oh, sorry for such error.
Quoted 11 lines
arch/x86/hyperv/hv_spinlock.c line 47
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c new file +7
-0+ spin_wait_info++; +hv_do_fast_hypercall8 (HVCALL_NOTIFY_LONG_SPIN_WAIT , input); +} + +static void hv_qlock_kick (int cpu) +{ + spin_wait_info--;Looking at the above this is completely pointless and I have no idea what that variable is supposed to do.
Quoted 7 lines
Per Microsoft Hypervisor Top Level Functional Specification, the input parameter of
HVCALL_NOTIFY_LONG_SPIN_WAITis defined as below:SpinwaitInfo – Specifies the accumulated count the guest was spinning.
So, it is increased when guest is spinning and reduced when spinlock is released.
But that's a global variable, so it might be incremented and decremented by several CPUs at once. I don't have the spec and have no time to study it either, but global does not make any sense to me. The spin wait info comes from a single guest CPU and the wakeup is targeted at that guest CPU as well. So why global? It might be defined that way, but then you really want to explain it proper.
Quoted 4 lines
Thanks,
tglx
Let me check this more. Will reply to you later. Thanks!
BRs,
Yi Sun
Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
Hi,
Quoted 36 lines · Yi Sun
Quoted 35 lines · Thomas Gleixner
Quoted 19 lines · Yi Sun
Quoted 6 lines
arch/x86/hyperv/hv_spinlock.c line 39
Quoted 3 lines
arch/x86/hyperv/hv_spinlock.c new file +3
-0+static void hv_notify_long_spin_wait (void ) +{ +u64 input = spin_wait_info |0x00000000ffffffff ;What? The result is always
0x00000000ffffffff.....Oh, sorry for such error.
Quoted 11 lines
arch/x86/hyperv/hv_spinlock.c line 47
Quoted 7 lines
arch/x86/hyperv/hv_spinlock.c new file +7
-0+ spin_wait_info++; +hv_do_fast_hypercall8 (HVCALL_NOTIFY_LONG_SPIN_WAIT , input); +} + +static void hv_qlock_kick (int cpu) +{ + spin_wait_info--;Looking at the above this is completely pointless and I have no idea what that variable is supposed to do.
Quoted 7 lines
Per Microsoft Hypervisor Top Level Functional Specification, the input parameter of
HVCALL_NOTIFY_LONG_SPIN_WAITis defined as below:SpinwaitInfo – Specifies the accumulated count the guest was spinning.
So, it is increased when guest is spinning and reduced when spinlock is released.
But that's a global variable, so it might be incremented and decremented by several CPUs at once. I don't have the spec and have no time to study it either, but global does not make any sense to me. The spin wait info comes from a single guest CPU and the wakeup is targeted at that guest CPU as well. So why global? It might be defined that way, but then you really want to explain it proper.
Quoted 8 lines
Quoted 4 lines
Thanks,
tglx
Let me check this more. Will reply to you later. Thanks!
BRs,
Yi Sun
I have got the details from Microsoft.
Notify_Long_Spin_Wait hypercall should be called after a specific times spinning which can be got through CPUID. When hypervisor receives this, it will try to ensure that all vcpus are scheduled.
Notify_Long_Spin_Wait is a standalone feature which should be split out as a new patch set.
So I would like split it into a new patch set and submit it after PV Hyper-V Spinlock patch set merged.
kbuild test robot
build botRe: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
In reply to Yi SunThank you for the patch! Yet something to improve:
Hi Yi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.19-rc3 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Yi-Sun/Enable-PV-qspinlock-for-Hyper-V/20180913-220827
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 All errors (new ones prefixed by >>):
arch/x86/hyperv/hv_spinlock.c: In function 'hv_init_spinlocks':
>> arch/x86/hyperv/hv_spinlock.c:86:2: error: implicit declaration of function '__pv_init_lock_hash'; did you mean 'spin_lock_bh'? [-Werror=implicit-function-declaration]
__pv_init_lock_hash();
^~~~~~~~~~~~~~~~~~~
spin_lock_bh
>> arch/x86/hyperv/hv_spinlock.c:87:42: error: '__pv_queued_spin_lock_slowpath' undeclared (first use in this function); did you mean 'queued_spin_lock_slowpath'?
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
queued_spin_lock_slowpath
arch/x86/hyperv/hv_spinlock.c:87:42: note: each undeclared identifier is reported only once for each function it appears in
In file included from arch/x86/include/asm/msr.h:246:0,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/smp.h:60,
from include/linux/kernel_stat.h:5,
from arch/x86/hyperv/hv_spinlock.c:22:
arch/x86/include/asm/paravirt.h:775:35: error: '__raw_callee_save___pv_queued_spin_unlock' undeclared (first use in this function); did you mean '__raw_callee_save_hv_vcpu_is_preempted'?
((struct paravirt_callee_save) { __raw_callee_save_##func })
^
arch/x86/hyperv/hv_spinlock.c:88:35: note: in expansion of macro 'PV_CALLEE_SAVE'
pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
^~~~~~~~~~~~~~cc1: some warnings being treated as errors
vim +86 arch/x86/hyperv/hv_spinlock.c
74
75 void __init hv_init_spinlocks(void)
76 {
77 if (!hv_pvspin ||
78 !apic ||
79 !(ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) ||
80 !(ms_hyperv.features & HV_X64_MSR_GUEST_IDLE_AVAILABLE)) {
81 pr_warn("hv: PV spinlocks disabled\n");
82 return;
83 }
84 pr_info("hv: PV spinlocks enabled\n");
85
> 86 __pv_init_lock_hash();
> 87 pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
88 pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
89 pv_lock_ops.wait = hv_qlock_wait;
90 pv_lock_ops.kick = hv_qlock_kick;
91 pv_lock_ops.vcpu_is_preempted = PV_CALLEE_SAVE(hv_vcpu_is_preempted);
92 }
93 ---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kbuild test robot
build botRe: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
In reply to Yi SunThank you for the patch! Yet something to improve:
Hi Yi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.19-rc3 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Yi-Sun/Enable-PV-qspinlock-for-Hyper-V/20180913-220827
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 All error/warnings (new ones prefixed by >>):
arch/x86//hyperv/hv_spinlock.c: In function 'hv_init_spinlocks':
arch/x86//hyperv/hv_spinlock.c:86:2: error: implicit declaration of function '__pv_init_lock_hash'; did you mean 'spin_lock_bh'? [-Werror=implicit-function-declaration]
__pv_init_lock_hash();
^~~~~~~~~~~~~~~~~~~
spin_lock_bh
arch/x86//hyperv/hv_spinlock.c:87:42: error: '__pv_queued_spin_lock_slowpath' undeclared (first use in this function); did you mean 'queued_spin_lock_slowpath'?
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
queued_spin_lock_slowpath
arch/x86//hyperv/hv_spinlock.c:87:42: note: each undeclared identifier is reported only once for each function it appears in
In file included from arch/x86/include/asm/msr.h:246:0,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/smp.h:60,
from include/linux/kernel_stat.h:5,
from arch/x86//hyperv/hv_spinlock.c:22:
>> arch/x86/include/asm/paravirt.h:775:35: error: '__raw_callee_save___pv_queued_spin_unlock' undeclared (first use in this function); did you mean '__raw_callee_save_hv_vcpu_is_preempted'?
((struct paravirt_callee_save) { __raw_callee_save_##func })
^
>> arch/x86//hyperv/hv_spinlock.c:88:35: note: in expansion of macro 'PV_CALLEE_SAVE'
pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
^~~~~~~~~~~~~~ cc1: some warnings being treated as errors
--
arch/x86/hyperv/hv_spinlock.c: In function 'hv_init_spinlocks':
arch/x86/hyperv/hv_spinlock.c:86:2: error: implicit declaration of function '__pv_init_lock_hash'; did you mean 'spin_lock_bh'? [-Werror=implicit-function-declaration]
__pv_init_lock_hash();
^~~~~~~~~~~~~~~~~~~
spin_lock_bh
arch/x86/hyperv/hv_spinlock.c:87:42: error: '__pv_queued_spin_lock_slowpath' undeclared (first use in this function); did you mean 'queued_spin_lock_slowpath'?
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
queued_spin_lock_slowpath
arch/x86/hyperv/hv_spinlock.c:87:42: note: each undeclared identifier is reported only once for each function it appears in
In file included from arch/x86/include/asm/msr.h:246:0,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/smp.h:60,
from include/linux/kernel_stat.h:5,
from arch/x86/hyperv/hv_spinlock.c:22:
>> arch/x86/include/asm/paravirt.h:775:35: error: '__raw_callee_save___pv_queued_spin_unlock' undeclared (first use in this function); did you mean '__raw_callee_save_hv_vcpu_is_preempted'?
((struct paravirt_callee_save) { __raw_callee_save_##func })
^
arch/x86/hyperv/hv_spinlock.c:88:35: note: in expansion of macro 'PV_CALLEE_SAVE'
pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
^~~~~~~~~~~~~~cc1: some warnings being treated as errors
vim +775 arch/x86/include/asm/paravirt.h
2e47d3e6 include/asm-x86/paravirt.h Glauber de Oliveira Costa 2008-01-30 744
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 745 /*
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 746 * Generate a thunk around a function which saves all caller-save
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 747 * registers except for the return value. This allows C functions to
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 748 * be called from assembler code where fewer than normal registers are
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 749 * available. It may also help code generation around calls from C
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 750 * code if the common case doesn't use many registers.
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 751 *
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 752 * When a callee is wrapped in a thunk, the caller can assume that all
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 753 * arg regs and all scratch registers are preserved across the
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 754 * call. The return value in rax/eax will not be saved, even for void
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 755 * functions.
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 756 */
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 757 #define PV_THUNK_NAME(func) "__raw_callee_save_" #func
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 758 #define PV_CALLEE_SAVE_REGS_THUNK(func) \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 759 extern typeof(func) __raw_callee_save_##func; \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 760 \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 761 asm(".pushsection .text;" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 762 ".globl " PV_THUNK_NAME(func) ";" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 763 ".type " PV_THUNK_NAME(func) ", @function;" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 764 PV_THUNK_NAME(func) ":" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 765 FRAME_BEGIN \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 766 PV_SAVE_ALL_CALLER_REGS \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 767 "call " #func ";" \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 768 PV_RESTORE_ALL_CALLER_REGS \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 769 FRAME_END \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 770 "ret;" \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 771 ".popsection")
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 772
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 773 /* Get a reference to a callee-save function */
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 774 #define PV_CALLEE_SAVE(func) \ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 @775 ((struct paravirt_callee_save) { __raw_callee_save_##func })ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 776
:::::: The code at line 775 was first introduced by commit
:::::: ecb93d1ccd0aac63f03be2db3cac3fa974716f4c x86/paravirt: add register-saving thunks to reduce caller register pressure
:::::: TO: Jeremy Fitzhardinge <[email protected]> :::::: CC: H. Peter Anvin <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
PATCH 3/3 hv: add description for hv_nopvspin
In reply to Yi Sunhv_nopvspin is a command line parameter used to disable Hyper-V PV spinlock.
--- Documentation/admin-guide/kernel-parameters.txt | 5 +++++ 1 file changed, 5 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +5 -0
@@ -1385,6 +1385,11 @@ hvc_iucv_allow= [S390] Comma-separated list of z/VM user IDs. If specified, z/VM IUCV HVC accepts connections from listed z/VM user IDs only. + + hv_nopvspin [X86,HYPER_V] + Disables the ticketlock slowpath using HYPER-V PV + optimizations. + keep_bootcon [KNL] Do not unregister boot console at start. This is only useful for debugging when something happens in the window
Signature · 1 line
1.9.1
From Yi Sun Sent: Thursday, September 13, 2018 2:13 AM
Quoted 3 lines
This patch adds the necessary Hyper-V specific code to allow PV qspinlock work on Hyper-V.
Have you done any performance measurements with this new code, so that we know whether there is any improvement, or even potentially any degradation in some circumstances?
Michael
Quoted 6 lines · Michael Kelley (EOSG)
Quoted 5 lines
From Yi Sun Sent: Thursday, September 13, 2018 2:13 AM
This patch adds the necessary Hyper-V specific code to allow PV qspinlock work on Hyper-V.
Quoted 6 lines
Have you done any performance measurements with this new code, so that we know whether there is any improvement, or even potentially any degradation in some circumstances?
Michael
I executed reaim in Guest (20 vcpus, 30G memory). Reaim is one of the performance test methods used by qspinlock patch set. Results are below. With hv_spinlock enabled, performance is better.
hv_spinlock disabled:
$ src/reaim -c data/reaim.config -f data/workfile.compute -i 16 -e 256 Num Parent Child Child Jobs per Jobs/min/ Std_dev Std_dev JTI Forked Time SysTime UTime Minute Child Time Percent 1 1.16 0.05 1.08 5224.14 5224.14 0.00 0.00 100 17 1.40 0.75 19.22 73585.71 4328.57 0.05 4.03 95 33 2.17 1.29 36.71 92156.68 2792.63 0.12 6.13 93 49 2.84 1.59 50.50 104556.34 2133.80 0.17 6.68 93 65 3.71 2.17 68.11 106172.51 1633.42 0.24 7.14 92 81 4.63 2.87 84.00 106017.28 1308.86 0.35 8.31 91 97 5.55 3.40 101.11 105913.51 1091.89 0.32 6.27 93 113 6.28 3.90 117.70 109041.40 964.97 0.38 6.51 93 129 7.20 4.44 134.00 108575.00 841.67 0.62 9.31 90 145 8.02 4.98 150.90 109563.59 755.61 0.61 8.18 91 178 9.75 6.46 184.96 110633.85 621.54 0.62 6.78 93 211 11.91 7.65 226.10 107360.20 508.82 0.88 7.96 92 244 13.30 8.28 253.82 111175.94 455.64 0.80 6.44 93 Max Jobs per Minute 111175.94
hv_spinlock enabled:
Num Parent Child Child Jobs per Jobs/min/ Std_dev Std_dev JTI Forked Time SysTime UTime Minute Child Time Percent 1 1.07 0.04 1.00 5663.55 5663.55 0.00 0.00 100 17 1.21 0.58 17.63 85140.50 5008.26 0.03 3.05 96 33 1.88 1.18 34.22 106372.34 3223.40 0.09 5.27 94 49 3.04 1.83 56.27 97677.63 1993.42 0.20 7.35 92 65 3.66 2.26 68.64 107622.95 1655.74 0.26 7.86 92 81 4.50 2.85 84.45 109080.00 1346.67 0.25 6.03 93 97 5.33 3.29 100.55 110285.18 1136.96 0.36 7.43 92 113 6.20 3.87 117.76 110448.39 977.42 0.39 6.87 93 129 6.96 4.42 132.93 112318.97 870.69 0.45 6.92 93 145 7.86 5.01 151.02 111793.89 770.99 0.50 6.76 93 179 9.71 6.16 185.62 111713.70 624.10 0.72 7.92 92 213 11.70 7.43 224.77 110323.08 517.95 0.73 6.69 93 247 13.45 8.48 259.19 111287.73 450.56 0.73 5.78 94
Max Jobs per Minute 112318.97