Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1241787 > unrolled thread

[RFC PATCH] Use vAPIC when doing IPI for PVHVM guests.

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH] Use vAPIC when doing IPI for PVHVM guests. Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2015-10-07 22:30 +0200
    [RFC PATCH v1 2/2] xen_nopv: Combine a bunch of the PV features that can be disabled Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2015-10-07 22:30 +0200
      Re: [RFC PATCH v1 2/2] xen_nopv: Combine a bunch of the PV features  that can be disabled Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2015-10-07 23:40 +0200
    Re: [RFC PATCH] Use vAPIC when doing IPI for PVHVM guests. Juergen Gross <jgross@suse.com> - 2015-10-08 07:10 +0200
      Re: [Xen-devel] [RFC PATCH] Use vAPIC when doing IPI for PVHVM guests. Andrew Cooper <andrew.cooper3@citrix.com> - 2015-10-08 11:40 +0200

#1241787 — [RFC PATCH] Use vAPIC when doing IPI for PVHVM guests.

Hey,

I was running some tools in which we would heavily do rescheduling
of events - and realized to my surprise that the event channels (and
the hypercall) would slow things down. If I used the vAPIC with its
IPI support (so no VMEXIT) I got much much better performance.

Now this is an RFC because:
 1). I hadn't verified from the xentrace  how much less VMEXITS we get.
    But I remember Boris's patches and they gave at least 10%.
    I think this will get the same performance or even better.

 2). I don't know what to do with migration. That is if the guest
    migrates to older hardware it needs to recheck this I presume?

 3). Should this be enabled by default? I did get better performance
   but that was synthetic.

Thoughts?


 Documentation/kernel-parameters.txt | 15 +++++------
 arch/x86/xen/enlighten.c            | 50 +++++++++++++++++++++++++++++++++----
 arch/x86/xen/smp.c                  | 22 ++++++++++++++--
 arch/x86/xen/spinlock.c             | 18 ++++---------
 arch/x86/xen/xen-ops.h              |  4 +++
 5 files changed, 82 insertions(+), 27 deletions(-)

Konrad Rzeszutek Wilk (2):
      xen/apic: Use vAPIC for IPI if the hardware supports it.
      xen_nopv: Combine a bunch of the PV features that can be disabled

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1241789 — [RFC PATCH v1 2/2] xen_nopv: Combine a bunch of the PV features that can be disabled

under one parameter. Removes the xen_nopvspin parameter and
makes it part of the xen_nopv.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 Documentation/kernel-parameters.txt | 15 +++++------
 arch/x86/xen/enlighten.c            | 50 +++++++++++++++++++++++++++++++++----
 arch/x86/xen/spinlock.c             | 18 ++++---------
 arch/x86/xen/xen-ops.h              |  4 +++
 4 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 22a4b68..73cd745 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -4125,13 +4125,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 				the unplug protocol
 			never -- do not unplug even if version check succeeds
 
-	xen_nopvspin	[X86,XEN]
-			Disables the ticketlock slowpath using Xen PV
-			optimizations.
-
-	xen_nopv	[X86]
-			Disables the PV optimizations forcing the HVM guest to
-			run as generic HVM guest with no PV drivers.
+	xen_nopv=	[X86,XEN]
+			Disables various (or all) PV optimizations forcing the
+			HVM (or PV) guest to run without them.
+			Format: [off0,][off]
+			all -- every PV feature on HVM.
+			spin -- Disables the ticketlock slowpath using Xen PV
+                                optimizations (PV and HVM).
+			ipi - Disable PV event channel IPI (on HVM).
 
 	xirc2ps_cs=	[NET,PCMCIA]
 			Format:
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 30d12af..c644d2c 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1829,17 +1829,57 @@ static void __init xen_hvm_guest_init(void)
 }
 #endif
 
-static bool xen_nopv = false;
+static unsigned int xen_nopv_feat;
+#define XEN_NOPV_SPIN	1<<1
+#define XEN_NOPV_IPI	1<<2
+#define XEN_NOPV_ALL	(XEN_NOPV_SPIN | XEN_NOPV_IPI)
+
+bool xen_no_pvspin(void)
+{
+	return xen_nopv_feat & XEN_NOPV_SPIN;
+}
+
+bool xen_no_pv(void)
+{
+	return xen_nopv_feat & XEN_NOPV_ALL;
+}
+
+bool xen_no_pvipi(void)
+{
+	return xen_nopv_feat & XEN_NOPV_IPI;
+}
+
 static __init int xen_parse_nopv(char *arg)
 {
-       xen_nopv = true;
-       return 0;
+	char *p, *q;
+	int l;
+
+	for (p = arg; p; p = q) {
+		q = strchr(p, ',');
+		if (q) {
+			l = q - p;
+			q++;
+		} else {
+			l = strlen(p);
+		}
+		if (!strncmp(p, "spin", l))
+			xen_nopv_feat |= XEN_NOPV_SPIN;
+		else if (!strncmp(p, "all", l))
+			xen_nopv_feat = XEN_NOPV_ALL;
+		else if (!strncmp(p, "ipi", l))
+			xen_nopv_feat |= XEN_NOPV_IPI;
+		else
+			printk(KERN_WARNING "unrecognised option '%s' "
+				 "in parameter 'xen_nopv'\n", p);
+	}
+	printk(KERN_DEBUG "xen_nopv_feat = 0x%x\n", xen_nopv_feat);
+	return 0;
 }
 early_param("xen_nopv", xen_parse_nopv);
 
 static uint32_t __init xen_platform(void)
 {
-	if (xen_nopv)
+	if (xen_no_pv())
 		return 0;
 
 	return xen_cpuid_base();
@@ -1847,7 +1887,7 @@ static uint32_t __init xen_platform(void)
 
 bool xen_hvm_need_lapic(void)
 {
-	if (xen_nopv)
+	if (xen_no_pv())
 		return false;
 	if (xen_pv_domain())
 		return false;
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 9e2ba5c..31b91e9 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -19,7 +19,6 @@
 
 static DEFINE_PER_CPU(int, lock_kicker_irq) = -1;
 static DEFINE_PER_CPU(char *, irq_name);
-static bool xen_pvspin = true;
 
 #ifdef CONFIG_QUEUED_SPINLOCKS
 
@@ -277,7 +276,7 @@ void xen_init_lock_cpu(int cpu)
 	int irq;
 	char *name;
 
-	if (!xen_pvspin)
+	if (!xen_no_pvspin())
 		return;
 
 	WARN(per_cpu(lock_kicker_irq, cpu) >= 0, "spinlock on CPU%d exists on IRQ%d!\n",
@@ -302,7 +301,7 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
-	if (!xen_pvspin)
+	if (!xen_no_pvspin())
 		return;
 
 	unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
@@ -323,7 +322,7 @@ void xen_uninit_lock_cpu(int cpu)
 void __init xen_init_spinlocks(void)
 {
 
-	if (!xen_pvspin) {
+	if (!xen_no_pvspin()) {
 		printk(KERN_DEBUG "xen: PV spinlocks disabled\n");
 		return;
 	}
@@ -348,7 +347,7 @@ void __init xen_init_spinlocks(void)
  */
 static __init int xen_init_spinlocks_jump(void)
 {
-	if (!xen_pvspin)
+	if (!xen_no_pvspin())
 		return 0;
 
 	if (!xen_domain())
@@ -359,13 +358,6 @@ static __init int xen_init_spinlocks_jump(void)
 }
 early_initcall(xen_init_spinlocks_jump);
 
-static __init int xen_parse_nopvspin(char *arg)
-{
-	xen_pvspin = false;
-	return 0;
-}
-early_param("xen_nopvspin", xen_parse_nopvspin);
-
 #if defined(CONFIG_XEN_DEBUG_FS) && !defined(CONFIG_QUEUED_SPINLOCKS)
 
 static struct dentry *d_spin_debug;
@@ -377,7 +369,7 @@ static int __init xen_spinlock_debugfs(void)
 	if (d_xen == NULL)
 		return -ENOMEM;
 
-	if (!xen_pvspin)
+	if (xen_no_pvspin())
 		return 0;
 
 	d_spin_debug = debugfs_create_dir("spinlocks", d_xen);
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 1399423..3652d38 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -149,4 +149,8 @@ __visible void xen_adjust_exception_frame(void);
 extern int xen_panic_handler_init(void);
 
 void xen_pvh_secondary_vcpu_init(int cpu);
+
+bool xen_no_pvspin(void);
+bool xen_no_pvipi(void);
+bool xen_no_pv(void);
 #endif /* XEN_OPS_H */
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241813 — Re: [RFC PATCH v1 2/2] xen_nopv: Combine a bunch of the PV features that can be disabled

On 10/07/2015 04:21 PM, Konrad Rzeszutek Wilk wrote:
> under one parameter. Removes the xen_nopvspin parameter and
> makes it part of the xen_nopv.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>   Documentation/kernel-parameters.txt | 15 +++++------
>   arch/x86/xen/enlighten.c            | 50 +++++++++++++++++++++++++++++++++----
>   arch/x86/xen/spinlock.c             | 18 ++++---------
>   arch/x86/xen/xen-ops.h              |  4 +++
>   4 files changed, 62 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 22a4b68..73cd745 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -4125,13 +4125,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>   				the unplug protocol
>   			never -- do not unplug even if version check succeeds
>   
> -	xen_nopvspin	[X86,XEN]
> -			Disables the ticketlock slowpath using Xen PV
> -			optimizations.
> -
> -	xen_nopv	[X86]
> -			Disables the PV optimizations forcing the HVM guest to
> -			run as generic HVM guest with no PV drivers.
> +	xen_nopv=	[X86,XEN]
> +			Disables various (or all) PV optimizations forcing the
> +			HVM (or PV) guest to run without them.
> +			Format: [off0,][off]

{ [spin][,ipi] | all }


> +			all -- every PV feature on HVM.
> +			spin -- Disables the ticketlock slowpath using Xen PV
> +                                optimizations (PV and HVM).
> +			ipi - Disable PV event channel IPI (on HVM).
>   
>   	xirc2ps_cs=	[NET,PCMCIA]
>   			Format:
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 30d12af..c644d2c 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1829,17 +1829,57 @@ static void __init xen_hvm_guest_init(void)
>   }
>   #endif
>   
> -static bool xen_nopv = false;
> +static unsigned int xen_nopv_feat;
> +#define XEN_NOPV_SPIN	1<<1
> +#define XEN_NOPV_IPI	1<<2
> +#define XEN_NOPV_ALL	(XEN_NOPV_SPIN | XEN_NOPV_IPI)
> +
> +bool xen_no_pvspin(void)
> +{
> +	return xen_nopv_feat & XEN_NOPV_SPIN;
> +}
> +
> +bool xen_no_pv(void)
> +{
> +	return xen_nopv_feat & XEN_NOPV_ALL;
> +}

Should XEN_NOPV_ALL be the 'OR' of the other two? I think it implies 
them but is broader than that. We may decide to have NOPV_IPI and 
NOPV_SPIN but still run a Xen-aware guest (i.e. we want to return 'true' 
from xen_platform()).

(Besides, this will return true if only one of the two bits is set.)

> +
> +bool xen_no_pvipi(void)
> +{
> +	return xen_nopv_feat & XEN_NOPV_IPI;
> +}

I don't see anyone using this.

Also, I think these should be inlines.


-boris

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241961

On 10/07/2015 10:21 PM, Konrad Rzeszutek Wilk wrote:
> Hey,
>
> I was running some tools in which we would heavily do rescheduling
> of events - and realized to my surprise that the event channels (and
> the hypercall) would slow things down. If I used the vAPIC with its
> IPI support (so no VMEXIT) I got much much better performance.
>
> Now this is an RFC because:
>   1). I hadn't verified from the xentrace  how much less VMEXITS we get.
>      But I remember Boris's patches and they gave at least 10%.
>      I think this will get the same performance or even better.
>
>   2). I don't know what to do with migration. That is if the guest
>      migrates to older hardware it needs to recheck this I presume?

Same problem applies to many other features. In case you want to
migrate to a machine with less features you'd have to mask those
features in the cpuid data of the domain.

>   3). Should this be enabled by default? I did get better performance
>     but that was synthetic.

Having some benchmark results would help to decide this. :-)

I'd be especially interested in checking "no vcpu over-commitment"
and "heavy vcpu over-commitment" scenarios regarding the effect of
the feature.

>
> Thoughts?

I like the idea.


Juergen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242117 — Re: [Xen-devel] [RFC PATCH] Use vAPIC when doing IPI for PVHVM guests.

On 08/10/15 06:05, Juergen Gross wrote:
> On 10/07/2015 10:21 PM, Konrad Rzeszutek Wilk wrote:
>> Hey,
>>
>> I was running some tools in which we would heavily do rescheduling
>> of events - and realized to my surprise that the event channels (and
>> the hypercall) would slow things down. If I used the vAPIC with its
>> IPI support (so no VMEXIT) I got much much better performance.
>>
>> Now this is an RFC because:
>>   1). I hadn't verified from the xentrace  how much less VMEXITS we get.
>>      But I remember Boris's patches and they gave at least 10%.
>>      I think this will get the same performance or even better.
>>
>>   2). I don't know what to do with migration. That is if the guest
>>      migrates to older hardware it needs to recheck this I presume?
>
> Same problem applies to many other features. In case you want to
> migrate to a machine with less features you'd have to mask those
> features in the cpuid data of the domain.

Those leaves in particular are from the HV set rather than the plain
featureset.  One way or another there will be an APIC to use, but those
features are expected to appear/disappear across migrate to indicate
whether hardware assistance is in use or not.

Therefore, they should be resampled and re-acted-upon in the resume path.

~Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web