[PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth

[PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 130 bytes --]

Since there was no reply to an earlier inquiry regarding this, here's a patch.

Signed-off-by: Jan Beulich <jbeulich@novell.com>


[-- Attachment #2: xenlinux-x86-better-halt.patch --]
[-- Type: text/plain, Size: 1832 bytes --]

Index: head-2006-05-26/include/asm-i386/bugs.h
===================================================================
--- head-2006-05-26.orig/include/asm-i386/bugs.h	2006-03-20 06:53:29.000000000 +0100
+++ head-2006-05-26/include/asm-i386/bugs.h	2006-05-26 13:55:43.000000000 +0200
@@ -92,6 +92,7 @@ static void __init check_fpu(void)
 
 static void __init check_hlt(void)
 {
+#ifndef CONFIG_XEN
 	printk(KERN_INFO "Checking 'hlt' instruction... ");
 	if (!boot_cpu_data.hlt_works_ok) {
 		printk("disabled\n");
@@ -102,6 +103,7 @@ static void __init check_hlt(void)
 	halt();
 	halt();
 	printk("OK.\n");
+#endif
 }
 
 /*
Index: head-2006-05-26/include/asm-i386/mach-xen/asm/system.h
===================================================================
--- head-2006-05-26.orig/include/asm-i386/mach-xen/asm/system.h	2006-04-24 11:43:43.000000000 +0200
+++ head-2006-05-26/include/asm-i386/mach-xen/asm/system.h	2006-05-26 12:26:22.000000000 +0200
@@ -625,8 +625,8 @@ do {									\
 		preempt_enable_no_resched();				\
 } while (0)
 
-#define safe_halt()		((void)0)
-#define halt()			((void)0)
+#define safe_halt()		({ __sti(); halt(); })
+#define halt()			((void)HYPERVISOR_block())
 
 #define __save_and_cli(x)						\
 do {									\
Index: head-2006-05-26/include/asm-x86_64/mach-xen/asm/system.h
===================================================================
--- head-2006-05-26.orig/include/asm-x86_64/mach-xen/asm/system.h	2006-04-24 11:43:43.000000000 +0200
+++ head-2006-05-26/include/asm-x86_64/mach-xen/asm/system.h	2006-05-26 12:26:08.000000000 +0200
@@ -424,8 +424,8 @@ do {									\
 	preempt_enable_no_resched();					\
 	___x; })
 
-#define safe_halt()		((void)0)
-#define halt()			((void)0)
+#define safe_halt()		({ __sti(); halt(); })
+#define halt()			((void)HYPERVISOR_block())
 
 void cpu_idle_wait(void);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth

[Keir Fraser]

On 29 May 2006, at 15:35, Jan Beulich wrote:

> Since there was no reply to an earlier inquiry regarding this, here's 
> a patch.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

safe_halt() should simply be HYPERVISOR_block() -- the hypercall 
implicitly reenables interrupt delivery for the calling VCPU, so no 
need to call __sti() explicitly.

halt() is a bit trickier -- given that it is executed (as far as I can 
tell) only in contexts where the CPU has reached "end of life" (it's 
crashed, or offlined, or shut down for some other reason) it might make 
sense to define halt() as VCPUOP_down. Or, just in case it is used in 
places with interrupts still enabled, where the CPU may have a future:
#define halt() (irqs_disabled() ? HYPERVISOR_vcpu_op(VCPUOP_down) : 
HYPERVISOR_block())

  -- Keir

Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 29.05.06 16:59 >>>
>
>On 29 May 2006, at 15:35, Jan Beulich wrote:
>
>> Since there was no reply to an earlier inquiry regarding this, here's 
>> a patch.
>>
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
>safe_halt() should simply be HYPERVISOR_block() -- the hypercall 
>implicitly reenables interrupt delivery for the calling VCPU, so no 
>need to call __sti() explicitly.
>
>halt() is a bit trickier -- given that it is executed (as far as I can 
>tell) only in contexts where the CPU has reached "end of life" (it's 
>crashed, or offlined, or shut down for some other reason) it might make 
>sense to define halt() as VCPUOP_down. Or, just in case it is used in 
>places with interrupts still enabled, where the CPU may have a future:
>#define halt() (irqs_disabled() ? HYPERVISOR_vcpu_op(VCPUOP_down) : 
>HYPERVISOR_block())

Indeed, this was the behavior I intended, but somehow I failed to find the VCPUOP_down operation that I was looking
for. Here's an updated patch.

Jan

[-- Attachment #2: xenlinux-x86-better-halt.patch --]
[-- Type: text/plain, Size: 2700 bytes --]

Index: head-2006-05-26/include/asm-i386/bugs.h
===================================================================
--- head-2006-05-26.orig/include/asm-i386/bugs.h	2006-03-20 06:53:29.000000000 +0100
+++ head-2006-05-26/include/asm-i386/bugs.h	2006-05-26 13:55:43.000000000 +0200
@@ -92,6 +92,7 @@ static void __init check_fpu(void)
 
 static void __init check_hlt(void)
 {
+#ifndef CONFIG_XEN
 	printk(KERN_INFO "Checking 'hlt' instruction... ");
 	if (!boot_cpu_data.hlt_works_ok) {
 		printk("disabled\n");
@@ -102,6 +103,7 @@ static void __init check_hlt(void)
 	halt();
 	halt();
 	printk("OK.\n");
+#endif
 }
 
 /*
Index: head-2006-05-26/include/asm-i386/mach-xen/asm/hypervisor.h
===================================================================
--- head-2006-05-26.orig/include/asm-i386/mach-xen/asm/hypervisor.h	2006-04-24 11:43:43.000000000 +0200
+++ head-2006-05-26/include/asm-i386/mach-xen/asm/hypervisor.h	2006-05-29 17:37:01.000000000 +0200
@@ -43,6 +43,7 @@
 #include <xen/interface/event_channel.h>
 #include <xen/interface/physdev.h>
 #include <xen/interface/sched.h>
+#include <xen/interface/vcpu.h>
 #include <xen/interface/nmi.h>
 #include <asm/ptrace.h>
 #include <asm/page.h>
Index: head-2006-05-26/include/asm-i386/mach-xen/asm/system.h
===================================================================
--- head-2006-05-26.orig/include/asm-i386/mach-xen/asm/system.h	2006-05-29 17:21:52.000000000 +0200
+++ head-2006-05-26/include/asm-i386/mach-xen/asm/system.h	2006-05-29 17:37:31.000000000 +0200
@@ -625,8 +625,11 @@ do {									\
 		preempt_enable_no_resched();				\
 } while (0)
 
-#define safe_halt()		((void)0)
-#define halt()			((void)0)
+#define safe_halt()		((void)HYPERVISOR_block())
+#define halt()			((void)					\
+	(HYPERVISOR_shared_info->vcpu_info[__vcpu_id].evtchn_upcall_mask \
+	 ? HYPERVISOR_vcpu_op(VCPUOP_down, __vcpu_id, NULL)		\
+	 : HYPERVISOR_block()))
 
 #define __save_and_cli(x)						\
 do {									\
Index: head-2006-05-26/include/asm-x86_64/mach-xen/asm/system.h
===================================================================
--- head-2006-05-26.orig/include/asm-x86_64/mach-xen/asm/system.h	2006-05-29 17:21:52.000000000 +0200
+++ head-2006-05-26/include/asm-x86_64/mach-xen/asm/system.h	2006-05-29 17:37:46.000000000 +0200
@@ -424,8 +424,11 @@ do {									\
 	preempt_enable_no_resched();					\
 	___x; })
 
-#define safe_halt()		((void)0)
-#define halt()			((void)0)
+#define safe_halt()		((void)HYPERVISOR_block())
+#define halt()			((void)					\
+	(HYPERVISOR_shared_info->vcpu_info[__vcpu_id].evtchn_upcall_mask \
+	 ? HYPERVISOR_vcpu_op(VCPUOP_down, __vcpu_id, NULL)		\
+	 : HYPERVISOR_block()))
 
 void cpu_idle_wait(void);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth

[Keir Fraser]

On 30 May 2006, at 09:11, Jan Beulich wrote:

> Indeed, this was the behavior I intended, but somehow I failed to find 
> the VCPUOP_down operation that I was looking
> for. Here's an updated patch.

Given that you went for the more complicated definition of halt() that 
calls block when event delivery is enabled, is it actually necessary to 
not compile the halt-checking routine? If event delivery is enabled in 
that function, shouldn't we return from each invocation of halt() on 
(at least) each timer interrupt? If not, I wonder why our behaviour 
there differs from native?

  -- Keir

Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth

[Jan Beulich]

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 30.05.06 10:49 >>>
>
>On 30 May 2006, at 09:11, Jan Beulich wrote:
>
>> Indeed, this was the behavior I intended, but somehow I failed to find 
>> the VCPUOP_down operation that I was looking
>> for. Here's an updated patch.
>
>Given that you went for the more complicated definition of halt() that 
>calls block when event delivery is enabled, is it actually necessary to 
>not compile the halt-checking routine? If event delivery is enabled in 

What halt-checking routine do you mean here?

>that function, shouldn't we return from each invocation of halt() on 
>(at least) each timer interrupt? If not, I wonder why our behaviour 
>there differs from native?

Yes, I think the net effect should be similar behavior to native.

Jan

Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth

[Keir Fraser]

On 30 May 2006, at 10:21, Jan Beulich wrote:

>> Given that you went for the more complicated definition of halt() that
>> calls block when event delivery is enabled, is it actually necessary 
>> to
>> not compile the halt-checking routine? If event delivery is enabled in
>
> What halt-checking routine do you mean here?

The one called check_hlt() that you modify in your patch to not build 
on Xen! I don't see why that part of the patch is necessary.

  -- Keir