There are cases folks want to disable spinlock optimization for
debug/test purpose. Xen and hyperv already have parameters "xen_nopvspin"
and "hv_nopvspin" to support that, but kvm doesn't.
The first patch adds that feature to KVM guest with "nopvspin".
I also changed print code in first patch, I'm not sure if I should
pick that part into a separate patch, just let me know if it's
preferred.
For compatibility reason original parameters "xen_nopvspin" and
"hv_nopvspin" are retained and marked obsolete.
Thanks
Zhenzhong
Included asm/hypervisor.h in order to reference x86_hyper_type.
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: Jonathan Corbet <[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: "H. Peter Anvin" <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 6 +++++-
arch/x86/hyperv/hv_spinlock.c | 9 +++++----
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1f0a62f..43f922c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1436,6 +1436,10 @@
hv_nopvspin [X86,HYPER_V] Disables the paravirt spinlock optimizations
which allow the hypervisor to 'idle' the
guest on lock contention.
+ This parameter is obsoleted by "nopvspin"
+ parameter, which has equivalent effect for
+ HYPER_V platform.
+
keep_bootcon [KNL]
Do not unregister boot console at start. This is only
@@ -5331,7 +5335,7 @@
as generic guest with no PV drivers. Currently support
XEN HVM, KVM, HYPER_V and VMWARE guest.
- nopvspin [X86,XEN,KVM] Disables the qspinlock slow path
+ nopvspin [X86,XEN,KVM,HYPER_V] Disables the qspinlock slow path
using PV optimizations which allow the hypervisor to
'idle' the guest on lock contention.
diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c
index 07f21a0..00c2902 100644
--- a/arch/x86/hyperv/hv_spinlock.c
+++ b/arch/x86/hyperv/hv_spinlock.c
@@ -12,12 +12,11 @@
#include <linux/spinlock.h>
+#include <asm/hypervisor.h>
#include <asm/mshyperv.h>
#include <asm/paravirt.h>
#include <asm/apic.h>
-static bool __initdata hv_pvspin = true;
-
static void hv_qlock_kick(int cpu)
{
apic->send_IPI(cpu, X86_PLATFORM_IPI_VECTOR);
@@ -64,7 +63,7 @@ __visible bool hv_vcpu_is_preempted(int vcpu)
void __init hv_init_spinlocks(void)
{
- if (!hv_pvspin || !apic ||
+ if (!pvspin || !apic ||
!(ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) ||
!(ms_hyperv.features & HV_X64_MSR_GUEST_IDLE_AVAILABLE)) {
pr_info("PV spinlocks disabled\n");
@@ -82,7 +81,9 @@ void __init hv_init_spinlocks(void)
static __init int hv_parse_nopvspin(char *arg)
{
- hv_pvspin = false;
+ pr_notice("\"hv_nopvspin\" is deprecated, please use \"nopvspin\" instead\n");
+ if (x86_hyper_type != X86_HYPER_MS_HYPERV)
+ pvspin = false;
return 0;
}
early_param("hv_nopvspin", hv_parse_nopvspin);
--
1.8.3.1
There are cases where a guest tries to switch spinlocks to bare metal
behavior (e.g. by setting "xen_nopvspin" on XEN platform and
"hv_nopvspin" on HYPER_V).
That feature is missed on KVM, add a new parameter "nopvspin" to disable
PV spinlocks for KVM guest.
This new parameter is also intended to replace "xen_nopvspin" and
"hv_nopvspin" in the future.
The global variable pvspin isn't defined as __initdata as it's used at
runtime by XEN guest.
Refactor the print stuff with pr_* which is preferred.
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krcmar <[email protected]>
Cc: Sean Christopherson <[email protected]>
Cc: Vitaly Kuznetsov <[email protected]>
Cc: Wanpeng Li <[email protected]>
Cc: Jim Mattson <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 4 ++++
arch/x86/include/asm/qspinlock.h | 1 +
arch/x86/kernel/kvm.c | 27 ++++++++++++++++---------
kernel/locking/qspinlock.c | 7 +++++++
4 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index c7ac2f3..4b956d8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5330,6 +5330,10 @@
as generic guest with no PV drivers. Currently support
XEN HVM, KVM, HYPER_V and VMWARE guest.
+ nopvspin [X86,KVM] Disables the qspinlock slow path
+ using PV optimizations which allow the hypervisor to
+ 'idle' the guest on lock contention.
+
xirc2ps_cs= [NET,PCMCIA]
Format:
<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index 444d6fd..34a4484 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -32,6 +32,7 @@ static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lo
extern void __pv_init_lock_hash(void);
extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
extern void __raw_callee_save___pv_queued_spin_unlock(struct qspinlock *lock);
+extern bool pvspin;
#define queued_spin_unlock queued_spin_unlock
/**
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index e820568..7b8cf0d 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -7,6 +7,8 @@
* Authors: Anthony Liguori <[email protected]>
*/
+#define pr_fmt(fmt) "KVM: " fmt
+
#include <linux/context_tracking.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -286,7 +288,7 @@ static void kvm_register_steal_time(void)
return;
wrmsrl(MSR_KVM_STEAL_TIME, (slow_virt_to_phys(st) | KVM_MSR_ENABLED));
- pr_info("kvm-stealtime: cpu %d, msr %llx\n",
+ pr_info("stealtime: cpu %d, msr %llx\n",
cpu, (unsigned long long) slow_virt_to_phys(st));
}
@@ -321,7 +323,7 @@ static void kvm_guest_cpu_init(void)
wrmsrl(MSR_KVM_ASYNC_PF_EN, pa);
__this_cpu_write(apf_reason.enabled, 1);
- printk(KERN_INFO"KVM setup async PF for cpu %d\n",
+ pr_info("setup async PF for cpu %d\n",
smp_processor_id());
}
@@ -347,7 +349,7 @@ static void kvm_pv_disable_apf(void)
wrmsrl(MSR_KVM_ASYNC_PF_EN, 0);
__this_cpu_write(apf_reason.enabled, 0);
- printk(KERN_INFO"Unregister pv shared memory for cpu %d\n",
+ pr_info("Unregister pv shared memory for cpu %d\n",
smp_processor_id());
}
@@ -509,7 +511,7 @@ static void kvm_setup_pv_ipi(void)
{
apic->send_IPI_mask = kvm_send_ipi_mask;
apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
- pr_info("KVM setup pv IPIs\n");
+ pr_info("setup pv IPIs\n");
}
static void kvm_smp_send_call_func_ipi(const struct cpumask *mask)
@@ -639,11 +641,11 @@ static void __init kvm_guest_init(void)
!kvm_para_has_hint(KVM_HINTS_REALTIME) &&
kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
smp_ops.send_call_func_ipi = kvm_smp_send_call_func_ipi;
- pr_info("KVM setup pv sched yield\n");
+ pr_info("setup pv sched yield\n");
}
if (cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "x86/kvm:online",
kvm_cpu_online, kvm_cpu_down_prepare) < 0)
- pr_err("kvm_guest: Failed to install cpu hotplug callbacks\n");
+ pr_err("failed to install cpu hotplug callbacks\n");
#else
sev_map_percpu_data();
kvm_guest_cpu_init();
@@ -746,7 +748,7 @@ static __init int kvm_setup_pv_tlb_flush(void)
zalloc_cpumask_var_node(per_cpu_ptr(&__pv_tlb_mask, cpu),
GFP_KERNEL, cpu_to_node(cpu));
}
- pr_info("KVM setup pv remote TLB flush\n");
+ pr_info("setup pv remote TLB flush\n");
}
return 0;
@@ -842,6 +844,13 @@ void __init kvm_spinlock_init(void)
if (num_possible_cpus() == 1)
return;
+ if (!pvspin) {
+ pr_info("PV spinlocks disabled\n");
+ static_branch_disable(&virt_spin_lock_key);
+ return;
+ }
+ pr_info("PV spinlocks enabled\n");
+
__pv_init_lock_hash();
pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
pv_ops.lock.queued_spin_unlock =
@@ -872,8 +881,8 @@ static void kvm_enable_host_haltpoll(void *i)
void arch_haltpoll_enable(unsigned int cpu)
{
if (!kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL)) {
- pr_err_once("kvm: host does not support poll control\n");
- pr_err_once("kvm: host upgrade recommended\n");
+ pr_err_once("host does not support poll control\n");
+ pr_err_once("host upgrade recommended\n");
return;
}
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index 2473f10..945b510 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -580,4 +580,11 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
#include "qspinlock_paravirt.h"
#include "qspinlock.c"
+bool pvspin = true;
+static __init int parse_nopvspin(char *arg)
+{
+ pvspin = false;
+ return 0;
+}
+early_param("nopvspin", parse_nopvspin);
#endif
--
1.8.3.1
Fix stale description of "xen_nopvspin" as we use qspinlock now.
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Stefano Stabellini <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 7 ++++---
arch/x86/xen/spinlock.c | 13 +++++++------
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4b956d8..1f0a62f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5303,8 +5303,9 @@
never -- do not unplug even if version check succeeds
xen_nopvspin [X86,XEN]
- Disables the ticketlock slowpath using Xen PV
- optimizations.
+ Disables the qspinlock slowpath using Xen PV optimizations.
+ This parameter is obsoleted by "nopvspin" parameter, which
+ has equivalent effect for XEN platform.
xen_nopv [X86]
Disables the PV optimizations forcing the HVM guest to
@@ -5330,7 +5331,7 @@
as generic guest with no PV drivers. Currently support
XEN HVM, KVM, HYPER_V and VMWARE guest.
- nopvspin [X86,KVM] Disables the qspinlock slow path
+ nopvspin [X86,XEN,KVM] Disables the qspinlock slow path
using PV optimizations which allow the hypervisor to
'idle' the guest on lock contention.
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 6deb490..092a53f 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -18,7 +18,6 @@
static DEFINE_PER_CPU(int, lock_kicker_irq) = -1;
static DEFINE_PER_CPU(char *, irq_name);
static DEFINE_PER_CPU(atomic_t, xen_qlock_wait_nest);
-static bool xen_pvspin = true;
static void xen_qlock_kick(int cpu)
{
@@ -68,7 +67,7 @@ void xen_init_lock_cpu(int cpu)
int irq;
char *name;
- if (!xen_pvspin)
+ if (!pvspin)
return;
WARN(per_cpu(lock_kicker_irq, cpu) >= 0, "spinlock on CPU%d exists on IRQ%d!\n",
@@ -93,7 +92,7 @@ void xen_init_lock_cpu(int cpu)
void xen_uninit_lock_cpu(int cpu)
{
- if (!xen_pvspin)
+ if (!pvspin)
return;
unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
@@ -117,9 +116,9 @@ void __init xen_init_spinlocks(void)
/* Don't need to use pvqspinlock code if there is only 1 vCPU. */
if (num_possible_cpus() == 1)
- xen_pvspin = false;
+ pvspin = false;
- if (!xen_pvspin) {
+ if (!pvspin) {
printk(KERN_DEBUG "xen: PV spinlocks disabled\n");
static_branch_disable(&virt_spin_lock_key);
return;
@@ -137,7 +136,9 @@ void __init xen_init_spinlocks(void)
static __init int xen_parse_nopvspin(char *arg)
{
- xen_pvspin = false;
+ pr_notice("\"xen_nopvspin\" is deprecated, please use \"nopvspin\" instead\n");
+ if (xen_domain())
+ pvspin = false;
return 0;
}
early_param("xen_nopvspin", xen_parse_nopvspin);
--
1.8.3.1
On 29.09.19 14:21, Zhenzhong Duan wrote:
> Fix stale description of "xen_nopvspin" as we use qspinlock now.
>
> Signed-off-by: Zhenzhong Duan <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Boris Ostrovsky <[email protected]>
> Cc: Juergen Gross <[email protected]>
> Cc: Stefano Stabellini <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
Reviewed-by: Juergen Gross <[email protected]>
Juergen
Zhenzhong Duan <[email protected]> writes:
> There are cases where a guest tries to switch spinlocks to bare metal
> behavior (e.g. by setting "xen_nopvspin" on XEN platform and
> "hv_nopvspin" on HYPER_V).
>
> That feature is missed on KVM, add a new parameter "nopvspin" to disable
> PV spinlocks for KVM guest.
>
> This new parameter is also intended to replace "xen_nopvspin" and
> "hv_nopvspin" in the future.
Any reason to not do it right now? We will probably need to have compat
code to support xen_nopvspin/hv_nopvspin too but emit a 'is deprecated'
warning.
>
> The global variable pvspin isn't defined as __initdata as it's used at
> runtime by XEN guest.
>
> Refactor the print stuff with pr_* which is preferred.
Please do it in a separate patch.
>
> Signed-off-by: Zhenzhong Duan <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Radim Krcmar <[email protected]>
> Cc: Sean Christopherson <[email protected]>
> Cc: Vitaly Kuznetsov <[email protected]>
> Cc: Wanpeng Li <[email protected]>
> Cc: Jim Mattson <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 4 ++++
> arch/x86/include/asm/qspinlock.h | 1 +
> arch/x86/kernel/kvm.c | 27 ++++++++++++++++---------
> kernel/locking/qspinlock.c | 7 +++++++
> 4 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index c7ac2f3..4b956d8 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5330,6 +5330,10 @@
> as generic guest with no PV drivers. Currently support
> XEN HVM, KVM, HYPER_V and VMWARE guest.
>
> + nopvspin [X86,KVM] Disables the qspinlock slow path
> + using PV optimizations which allow the hypervisor to
> + 'idle' the guest on lock contention.
> +
> xirc2ps_cs= [NET,PCMCIA]
> Format:
> <irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
> diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
> index 444d6fd..34a4484 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -32,6 +32,7 @@ static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lo
> extern void __pv_init_lock_hash(void);
> extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
> extern void __raw_callee_save___pv_queued_spin_unlock(struct qspinlock *lock);
> +extern bool pvspin;
>
> #define queued_spin_unlock queued_spin_unlock
> /**
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index e820568..7b8cf0d 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -7,6 +7,8 @@
> * Authors: Anthony Liguori <[email protected]>
> */
>
> +#define pr_fmt(fmt) "KVM: " fmt
> +
> #include <linux/context_tracking.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> @@ -286,7 +288,7 @@ static void kvm_register_steal_time(void)
> return;
>
> wrmsrl(MSR_KVM_STEAL_TIME, (slow_virt_to_phys(st) | KVM_MSR_ENABLED));
> - pr_info("kvm-stealtime: cpu %d, msr %llx\n",
> + pr_info("stealtime: cpu %d, msr %llx\n",
> cpu, (unsigned long long) slow_virt_to_phys(st));
> }
>
> @@ -321,7 +323,7 @@ static void kvm_guest_cpu_init(void)
>
> wrmsrl(MSR_KVM_ASYNC_PF_EN, pa);
> __this_cpu_write(apf_reason.enabled, 1);
> - printk(KERN_INFO"KVM setup async PF for cpu %d\n",
> + pr_info("setup async PF for cpu %d\n",
> smp_processor_id());
> }
>
> @@ -347,7 +349,7 @@ static void kvm_pv_disable_apf(void)
> wrmsrl(MSR_KVM_ASYNC_PF_EN, 0);
> __this_cpu_write(apf_reason.enabled, 0);
>
> - printk(KERN_INFO"Unregister pv shared memory for cpu %d\n",
> + pr_info("Unregister pv shared memory for cpu %d\n",
> smp_processor_id());
> }
>
> @@ -509,7 +511,7 @@ static void kvm_setup_pv_ipi(void)
> {
> apic->send_IPI_mask = kvm_send_ipi_mask;
> apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
> - pr_info("KVM setup pv IPIs\n");
> + pr_info("setup pv IPIs\n");
> }
>
> static void kvm_smp_send_call_func_ipi(const struct cpumask *mask)
> @@ -639,11 +641,11 @@ static void __init kvm_guest_init(void)
> !kvm_para_has_hint(KVM_HINTS_REALTIME) &&
> kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
> smp_ops.send_call_func_ipi = kvm_smp_send_call_func_ipi;
> - pr_info("KVM setup pv sched yield\n");
> + pr_info("setup pv sched yield\n");
> }
> if (cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "x86/kvm:online",
> kvm_cpu_online, kvm_cpu_down_prepare) < 0)
> - pr_err("kvm_guest: Failed to install cpu hotplug callbacks\n");
> + pr_err("failed to install cpu hotplug callbacks\n");
> #else
> sev_map_percpu_data();
> kvm_guest_cpu_init();
> @@ -746,7 +748,7 @@ static __init int kvm_setup_pv_tlb_flush(void)
> zalloc_cpumask_var_node(per_cpu_ptr(&__pv_tlb_mask, cpu),
> GFP_KERNEL, cpu_to_node(cpu));
> }
> - pr_info("KVM setup pv remote TLB flush\n");
> + pr_info("setup pv remote TLB flush\n");
> }
>
> return 0;
> @@ -842,6 +844,13 @@ void __init kvm_spinlock_init(void)
> if (num_possible_cpus() == 1)
> return;
>
> + if (!pvspin) {
> + pr_info("PV spinlocks disabled\n");
> + static_branch_disable(&virt_spin_lock_key);
> + return;
> + }
> + pr_info("PV spinlocks enabled\n");
> +
> __pv_init_lock_hash();
> pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
> pv_ops.lock.queued_spin_unlock =
> @@ -872,8 +881,8 @@ static void kvm_enable_host_haltpoll(void *i)
> void arch_haltpoll_enable(unsigned int cpu)
> {
> if (!kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL)) {
> - pr_err_once("kvm: host does not support poll control\n");
> - pr_err_once("kvm: host upgrade recommended\n");
> + pr_err_once("host does not support poll control\n");
> + pr_err_once("host upgrade recommended\n");
> return;
> }
>
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index 2473f10..945b510 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -580,4 +580,11 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
> #include "qspinlock_paravirt.h"
> #include "qspinlock.c"
>
> +bool pvspin = true;
> +static __init int parse_nopvspin(char *arg)
> +{
> + pvspin = false;
> + return 0;
> +}
> +early_param("nopvspin", parse_nopvspin);
> #endif
--
Vitaly
On 2019/9/30 23:41, Vitaly Kuznetsov wrote:
> Zhenzhong Duan<[email protected]> writes:
>
>> There are cases where a guest tries to switch spinlocks to bare metal
>> behavior (e.g. by setting "xen_nopvspin" on XEN platform and
>> "hv_nopvspin" on HYPER_V).
>>
>> That feature is missed on KVM, add a new parameter "nopvspin" to disable
>> PV spinlocks for KVM guest.
>>
>> This new parameter is also intended to replace "xen_nopvspin" and
>> "hv_nopvspin" in the future.
> Any reason to not do it right now? We will probably need to have compat
> code to support xen_nopvspin/hv_nopvspin too but emit a 'is deprecated'
> warning.
Sorry the description isn't clear, I'll fix it.
I did the compat work in the other two patches.
[PATCH 2/3] xen: Mark "xen_nopvspin" parameter obsolete and map it to "nopvspin"
[PATCH 3/3] x86/hyperv: Mark "hv_nopvspin" parameter obsolete and map it to "nopvspin"
>
>> The global variable pvspin isn't defined as __initdata as it's used at
>> runtime by XEN guest.
>>
>> Refactor the print stuff with pr_* which is preferred.
> Please do it in a separate patch.
Ok, I'll do that in v2. Thanks for review.
Zhenzhong
Zhenzhong Duan <[email protected]> writes:
> On 2019/9/30 23:41, Vitaly Kuznetsov wrote:
>> Zhenzhong Duan<[email protected]> writes:
>>
>>> There are cases where a guest tries to switch spinlocks to bare metal
>>> behavior (e.g. by setting "xen_nopvspin" on XEN platform and
>>> "hv_nopvspin" on HYPER_V).
>>>
>>> That feature is missed on KVM, add a new parameter "nopvspin" to disable
>>> PV spinlocks for KVM guest.
>>>
>>> This new parameter is also intended to replace "xen_nopvspin" and
>>> "hv_nopvspin" in the future.
>> Any reason to not do it right now? We will probably need to have compat
>> code to support xen_nopvspin/hv_nopvspin too but emit a 'is deprecated'
>> warning.
>
> Sorry the description isn't clear, I'll fix it.
>
> I did the compat work in the other two patches.
> [PATCH 2/3] xen: Mark "xen_nopvspin" parameter obsolete and map it to "nopvspin"
> [PATCH 3/3] x86/hyperv: Mark "hv_nopvspin" parameter obsolete and map it to "nopvspin"
>
For some reason I got CCed only on the first one and moreover, I don't
see e.g PATCH3 on 'linux-hyperv' mailing list.
>>
>>> The global variable pvspin isn't defined as __initdata as it's used at
>>> runtime by XEN guest.
>>>
>>> Refactor the print stuff with pr_* which is preferred.
>> Please do it in a separate patch.
>
> Ok, I'll do that in v2. Thanks for review.
Thanks!
--
Vitaly
On 2019/10/1 16:39, Vitaly Kuznetsov wrote:
> Zhenzhong Duan<[email protected]> writes:
>
>> On 2019/9/30 23:41, Vitaly Kuznetsov wrote:
>>> Zhenzhong Duan<[email protected]> writes:
>>>
>>>> There are cases where a guest tries to switch spinlocks to bare metal
>>>> behavior (e.g. by setting "xen_nopvspin" on XEN platform and
>>>> "hv_nopvspin" on HYPER_V).
>>>>
>>>> That feature is missed on KVM, add a new parameter "nopvspin" to disable
>>>> PV spinlocks for KVM guest.
>>>>
>>>> This new parameter is also intended to replace "xen_nopvspin" and
>>>> "hv_nopvspin" in the future.
>>> Any reason to not do it right now? We will probably need to have compat
>>> code to support xen_nopvspin/hv_nopvspin too but emit a 'is deprecated'
>>> warning.
>> Sorry the description isn't clear, I'll fix it.
>>
>> I did the compat work in the other two patches.
>> [PATCH 2/3] xen: Mark "xen_nopvspin" parameter obsolete and map it to "nopvspin"
>> [PATCH 3/3] x86/hyperv: Mark "hv_nopvspin" parameter obsolete and map it to "nopvspin"
>>
> For some reason I got CCed only on the first one and moreover,
The three patches have different maintainers/reviewers by get_maintainer.pl, I added
"Cc: maintainers/reviewers" to each patch then git-sendemail picked them automaticly.
I meaned to not disturb maintainers with the field they aren't in charge of. It looks
I'm wrong.
So what's the correct way dealing with this? Should I send the whole patchset to all
the maintainers/reviewers related to all the patches?
> I don't see e.g PATCH3 on 'linux-hyperv' mailing list.
Thanks for point out, I'll add it.
Zhenzhong
On Tue, Oct 01, 2019 at 05:47:00PM +0800, Zhenzhong Duan wrote:
>
> On 2019/10/1 16:39, Vitaly Kuznetsov wrote:
> >Zhenzhong Duan<[email protected]> writes:
> >
> >>On 2019/9/30 23:41, Vitaly Kuznetsov wrote:
> >>>Zhenzhong Duan<[email protected]> writes:
> >>>
> >>>>There are cases where a guest tries to switch spinlocks to bare metal
> >>>>behavior (e.g. by setting "xen_nopvspin" on XEN platform and
> >>>>"hv_nopvspin" on HYPER_V).
> >>>>
> >>>>That feature is missed on KVM, add a new parameter "nopvspin" to disable
> >>>>PV spinlocks for KVM guest.
> >>>>
> >>>>This new parameter is also intended to replace "xen_nopvspin" and
> >>>>"hv_nopvspin" in the future.
> >>>Any reason to not do it right now? We will probably need to have compat
> >>>code to support xen_nopvspin/hv_nopvspin too but emit a 'is deprecated'
> >>>warning.
> >>Sorry the description isn't clear, I'll fix it.
> >>
> >>I did the compat work in the other two patches.
> >>[PATCH 2/3] xen: Mark "xen_nopvspin" parameter obsolete and map it to "nopvspin"
> >>[PATCH 3/3] x86/hyperv: Mark "hv_nopvspin" parameter obsolete and map it to "nopvspin"
> >>
> >For some reason I got CCed only on the first one and moreover,
>
> The three patches have different maintainers/reviewers by get_maintainer.pl, I added
> "Cc: maintainers/reviewers" to each patch then git-sendemail picked them automaticly.
> I meaned to not disturb maintainers with the field they aren't in charge of. It looks
> I'm wrong.
>
> So what's the correct way dealing with this? Should I send the whole patchset to all
> the maintainers/reviewers related to all the patches?
There's no one right answer to that question, folks have different
preferences. My general rule of thumb is to cc everyone on all patches
unless the series is obnoxiously large *and* isolated to a specific part
of the kernel. The idea being that people are more likely to be annoyed
if they can't find all patches in a relatively small series (this case)
than they are about receiving a mail or two that they don't care about.
At a minimum I would cc everyone involved on the cover letter, and cc the
relevant mailing lists on all patches. Sending everyone the cover letter
provides people a quick overview of the patches they didn't receive, as
well as a starting point if they want to find those patches. Cc'ing the
mailing list(s) can make it even easier to find the patches. The cover
letter is also a good place to explain why you didn't cc everyone on all
patches (or vice versa).
Also, the cover letter should have the shortlog and overall diffstats.
'git format-patch --cover-letter' will do the work for you.
On 2019/10/3 0:47, Sean Christopherson wrote:
> On Tue, Oct 01, 2019 at 05:47:00PM +0800, Zhenzhong Duan wrote:
>> On 2019/10/1 16:39, Vitaly Kuznetsov wrote:
>>> Zhenzhong Duan<[email protected]> writes:
>>>
>>>> On 2019/9/30 23:41, Vitaly Kuznetsov wrote:
>>>>> Zhenzhong Duan<[email protected]> writes:
>>>>>
>>>>>> There are cases where a guest tries to switch spinlocks to bare metal
>>>>>> behavior (e.g. by setting "xen_nopvspin" on XEN platform and
>>>>>> "hv_nopvspin" on HYPER_V).
>>>>>>
>>>>>> That feature is missed on KVM, add a new parameter "nopvspin" to disable
>>>>>> PV spinlocks for KVM guest.
>>>>>>
>>>>>> This new parameter is also intended to replace "xen_nopvspin" and
>>>>>> "hv_nopvspin" in the future.
>>>>> Any reason to not do it right now? We will probably need to have compat
>>>>> code to support xen_nopvspin/hv_nopvspin too but emit a 'is deprecated'
>>>>> warning.
>>>> Sorry the description isn't clear, I'll fix it.
>>>>
>>>> I did the compat work in the other two patches.
>>>> [PATCH 2/3] xen: Mark "xen_nopvspin" parameter obsolete and map it to "nopvspin"
>>>> [PATCH 3/3] x86/hyperv: Mark "hv_nopvspin" parameter obsolete and map it to "nopvspin"
>>>>
>>> For some reason I got CCed only on the first one and moreover,
>> The three patches have different maintainers/reviewers by get_maintainer.pl, I added
>> "Cc: maintainers/reviewers" to each patch then git-sendemail picked them automaticly.
>> I meaned to not disturb maintainers with the field they aren't in charge of. It looks
>> I'm wrong.
>>
>> So what's the correct way dealing with this? Should I send the whole patchset to all
>> the maintainers/reviewers related to all the patches?
> There's no one right answer to that question, folks have different
> preferences. My general rule of thumb is to cc everyone on all patches
> unless the series is obnoxiously large *and* isolated to a specific part
> of the kernel. The idea being that people are more likely to be annoyed
> if they can't find all patches in a relatively small series (this case)
> than they are about receiving a mail or two that they don't care about.
>
> At a minimum I would cc everyone involved on the cover letter, and cc the
> relevant mailing lists on all patches. Sending everyone the cover letter
> provides people a quick overview of the patches they didn't receive, as
> well as a starting point if they want to find those patches. Cc'ing the
> mailing list(s) can make it even easier to find the patches. The cover
> letter is also a good place to explain why you didn't cc everyone on all
> patches (or vice versa).
>
> Also, the cover letter should have the shortlog and overall diffstats.
> 'git format-patch --cover-letter' will do the work for you.
Thanks for your detailed reply, I's clear to me what to do now.
Zhenzhong