* [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth
@ 2006-05-29 14:35 Jan Beulich
2006-05-29 14:59 ` Keir Fraser
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2006-05-29 14:35 UTC (permalink / raw)
To: xen-devel
[-- 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth
2006-05-29 14:35 [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth Jan Beulich
@ 2006-05-29 14:59 ` Keir Fraser
2006-05-30 8:11 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2006-05-29 14:59 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth
2006-05-29 14:59 ` Keir Fraser
@ 2006-05-30 8:11 ` Jan Beulich
2006-05-30 8:49 ` Keir Fraser
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2006-05-30 8:11 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel
[-- 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth
2006-05-30 8:11 ` Jan Beulich
@ 2006-05-30 8:49 ` Keir Fraser
2006-05-30 9:21 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2006-05-30 8:49 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth
2006-05-30 8:49 ` Keir Fraser
@ 2006-05-30 9:21 ` Jan Beulich
2006-05-30 9:30 ` Keir Fraser
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2006-05-30 9:21 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel
>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth
2006-05-30 9:21 ` Jan Beulich
@ 2006-05-30 9:30 ` Keir Fraser
0 siblings, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2006-05-30 9:30 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-05-30 9:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-29 14:35 [PATCH] x86 linux: prevent halted VCPUs from eating up CPU bandwidth Jan Beulich
2006-05-29 14:59 ` Keir Fraser
2006-05-30 8:11 ` Jan Beulich
2006-05-30 8:49 ` Keir Fraser
2006-05-30 9:21 ` Jan Beulich
2006-05-30 9:30 ` Keir Fraser
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.