linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 6/11] Vmi timer no idle hz fixes.patch
@ 2007-02-06  3:53 Zachary Amsden
  2007-02-06 12:29 ` Andi Kleen
  0 siblings, 1 reply; 4+ messages in thread
From: Zachary Amsden @ 2007-02-06  3:53 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Andrew Morton, Andi Kleen,
	Rusty Russell, Jeremy Fitzhardinge, Chris Wright, Zachary Amsden

Fix the VMI-Timer no-idle-hz code.

Do not setup a one shot alarm if we are keeping the periodic alarm
armed.  Additionally, since the periodic alarm can be run at a lower
rate than HZ, let's fixup the guard to the no-idle-hz mode appropriately.
This fixes the bug where the no-idle-hz mode might have a higher interrupt
rate than the non-idle case.

Signed-off-by: Dan Hecht <dhecht@vmware.com>

diff -r 9d107b81bb7d arch/i386/kernel/vmitime.c
--- a/arch/i386/kernel/vmitime.c	Thu Feb 01 23:43:37 2007 -0800
+++ b/arch/i386/kernel/vmitime.c	Thu Feb 01 23:52:59 2007 -0800
@@ -374,7 +374,6 @@ int vmi_stop_hz_timer(void)
 	unsigned long seq, next;
 	unsigned long long real_cycles_expiry;
 	int cpu = smp_processor_id();
-	int idle;
 
 	BUG_ON(!irqs_disabled());
 	if (sysctl_hz_timer != 0)
@@ -382,13 +381,13 @@ int vmi_stop_hz_timer(void)
 
 	cpu_set(cpu, nohz_cpu_mask);
 	smp_mb();
+
 	if (rcu_needs_cpu(cpu) || local_softirq_pending() ||
-	    (next = next_timer_interrupt(), time_before_eq(next, jiffies))) {
+	    (next = next_timer_interrupt(), 
+	     time_before_eq(next, jiffies + HZ/CONFIG_VMI_ALARM_HZ))) {
 		cpu_clear(cpu, nohz_cpu_mask);
-		next = jiffies;
-		idle = 0;
-	} else
-		idle = 1;
+		return 0;
+	}
 
 	/* Convert jiffies to the real cycle counter. */
 	do {
@@ -398,17 +397,13 @@ int vmi_stop_hz_timer(void)
 	} while (read_seqretry(&xtime_lock, seq));
 
 	/* This cpu is going idle. Disable the periodic alarm. */
-	if (idle) {
-		vmi_timer_ops.cancel_alarm(VMI_CYCLES_AVAILABLE);
-		per_cpu(idle_start_jiffies, cpu) = jiffies;
-	}
-
+	vmi_timer_ops.cancel_alarm(VMI_CYCLES_AVAILABLE);
+	per_cpu(idle_start_jiffies, cpu) = jiffies;
 	/* Set the real time alarm to expire at the next event. */
 	vmi_timer_ops.set_alarm(
-		      VMI_ALARM_WIRING | VMI_ALARM_IS_ONESHOT | VMI_CYCLES_REAL,
-		      real_cycles_expiry, 0);
-
-	return idle;
+		VMI_ALARM_WIRING | VMI_ALARM_IS_ONESHOT | VMI_CYCLES_REAL,
+		real_cycles_expiry, 0);
+	return 1;
 }
 
 static void vmi_reenable_hz_timer(int cpu)
diff -r 9d107b81bb7d kernel/sysctl.c
--- a/kernel/sysctl.c	Thu Feb 01 23:43:37 2007 -0800
+++ b/kernel/sysctl.c	Thu Feb 01 23:52:59 2007 -0800
@@ -440,17 +440,7 @@ static ctl_table kern_table[] = {
 		.extra1		= &minolduid,
 		.extra2		= &maxolduid,
 	},
-#ifdef CONFIG_S390
-#ifdef CONFIG_MATHEMU
-	{
-		.ctl_name	= KERN_IEEE_EMULATION_WARNINGS,
-		.procname	= "ieee_emulation_warnings",
-		.data		= &sysctl_ieee_emulation_warnings,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= &proc_dointvec,
-	},
-#endif
+#if defined(CONFIG_S390) || defined(CONFIG_VMI)
 #ifdef CONFIG_NO_IDLE_HZ
 	{
 		.ctl_name       = KERN_HZ_TIMER,
@@ -459,6 +449,18 @@ static ctl_table kern_table[] = {
 		.maxlen         = sizeof(int),
 		.mode           = 0644,
 		.proc_handler   = &proc_dointvec,
+	},
+#endif
+#endif
+#ifdef CONFIG_S390
+#ifdef CONFIG_MATHEMU
+	{
+		.ctl_name	= KERN_IEEE_EMULATION_WARNINGS,
+		.procname	= "ieee_emulation_warnings",
+		.data		= &sysctl_ieee_emulation_warnings,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
 	},
 #endif
 	{

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 6/11] Vmi timer no idle hz fixes.patch
  2007-02-06  3:53 [PATCH 6/11] Vmi timer no idle hz fixes.patch Zachary Amsden
@ 2007-02-06 12:29 ` Andi Kleen
  2007-02-07  0:22   ` Zachary Amsden
  2007-02-07  1:05   ` Zachary Amsden
  0 siblings, 2 replies; 4+ messages in thread
From: Andi Kleen @ 2007-02-06 12:29 UTC (permalink / raw)
  To: Zachary Amsden
  Cc: Linux Kernel Mailing List, Andrew Morton, Rusty Russell,
	Jeremy Fitzhardinge, Chris Wright

> -#ifdef CONFIG_S390
> -#ifdef CONFIG_MATHEMU
> -	{
> -		.ctl_name	= KERN_IEEE_EMULATION_WARNINGS,
> -		.procname	= "ieee_emulation_warnings",
> -		.data		= &sysctl_ieee_emulation_warnings,
> -		.maxlen		= sizeof(int),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_dointvec,
> -	},

I think there needs to be a way to have this syctl only
when it's really implemented for the paravirt ops.

I suppose you should move it to dynamic registration on demand.

-Andi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 6/11] Vmi timer no idle hz fixes.patch
  2007-02-06 12:29 ` Andi Kleen
@ 2007-02-07  0:22   ` Zachary Amsden
  2007-02-07  1:05   ` Zachary Amsden
  1 sibling, 0 replies; 4+ messages in thread
From: Zachary Amsden @ 2007-02-07  0:22 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Linux Kernel Mailing List, Andrew Morton, Rusty Russell,
	Jeremy Fitzhardinge, Chris Wright

Andi Kleen wrote:
>> -#ifdef CONFIG_S390
>> -#ifdef CONFIG_MATHEMU
>> -	{
>> -		.ctl_name	= KERN_IEEE_EMULATION_WARNINGS,
>> -		.procname	= "ieee_emulation_warnings",
>> -		.data		= &sysctl_ieee_emulation_warnings,
>> -		.maxlen		= sizeof(int),
>> -		.mode		= 0644,
>> -		.proc_handler	= &proc_dointvec,
>> -	},
>>     
>
> I think there needs to be a way to have this syctl only
> when it's really implemented for the paravirt ops.
>
> I suppose you should move it to dynamic registration on demand.
>   

Hmm, is there a way to do that and share the sysctl with Xen?  Yes, I 
think if we move the data to paravirt.c and call 
paravirt_start_no_idle_hz (or something_with_fewer_underbars), then we 
can both share this call site instead of silly duplicating the .ctl_name ...

Will code it up.

Zach

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 6/11] Vmi timer no idle hz fixes.patch
  2007-02-06 12:29 ` Andi Kleen
  2007-02-07  0:22   ` Zachary Amsden
@ 2007-02-07  1:05   ` Zachary Amsden
  1 sibling, 0 replies; 4+ messages in thread
From: Zachary Amsden @ 2007-02-07  1:05 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Linux Kernel Mailing List, Andrew Morton, Rusty Russell,
	Jeremy Fitzhardinge, Chris Wright

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

Andi Kleen wrote:
>> -#ifdef CONFIG_S390
>> -#ifdef CONFIG_MATHEMU
>> -	{
>> -		.ctl_name	= KERN_IEEE_EMULATION_WARNINGS,
>> -		.procname	= "ieee_emulation_warnings",
>> -		.data		= &sysctl_ieee_emulation_warnings,
>> -		.maxlen		= sizeof(int),
>> -		.mode		= 0644,
>> -		.proc_handler	= &proc_dointvec,
>> -	},
>>     
>
> I think there needs to be a way to have this syctl only
> when it's really implemented for the paravirt ops.
>   

Alright, lets drop that part of the patch for now.  Here's an update.  
Please apply.

> I suppose you should move it to dynamic registration on demand.
>   

And start working on this.

Zach

[-- Attachment #2: vmi-timer-no-idle-hz-fixes.patch --]
[-- Type: text/plain, Size: 2064 bytes --]

Fix the VMI-Timer no-idle-hz code.

Do not setup a one shot alarm if we are keeping the periodic alarm
armed.  Additionally, since the periodic alarm can be run at a lower
rate than HZ, let's fixup the guard to the no-idle-hz mode appropriately.
This fixes the bug where the no-idle-hz mode might have a higher interrupt
rate than the non-idle case.

Signed-off-by: Dan Hecht <dhecht@vmware.com>
Signed-off-by: Zachary Amsden <zach@vmware.com>

diff -r 9d107b81bb7d arch/i386/kernel/vmitime.c
--- a/arch/i386/kernel/vmitime.c	Thu Feb 01 23:43:37 2007 -0800
+++ b/arch/i386/kernel/vmitime.c	Thu Feb 01 23:52:59 2007 -0800
@@ -374,7 +374,6 @@ int vmi_stop_hz_timer(void)
 	unsigned long seq, next;
 	unsigned long long real_cycles_expiry;
 	int cpu = smp_processor_id();
-	int idle;
 
 	BUG_ON(!irqs_disabled());
 	if (sysctl_hz_timer != 0)
@@ -382,13 +381,13 @@ int vmi_stop_hz_timer(void)
 
 	cpu_set(cpu, nohz_cpu_mask);
 	smp_mb();
+
 	if (rcu_needs_cpu(cpu) || local_softirq_pending() ||
-	    (next = next_timer_interrupt(), time_before_eq(next, jiffies))) {
+	    (next = next_timer_interrupt(), 
+	     time_before_eq(next, jiffies + HZ/CONFIG_VMI_ALARM_HZ))) {
 		cpu_clear(cpu, nohz_cpu_mask);
-		next = jiffies;
-		idle = 0;
-	} else
-		idle = 1;
+		return 0;
+	}
 
 	/* Convert jiffies to the real cycle counter. */
 	do {
@@ -398,17 +397,13 @@ int vmi_stop_hz_timer(void)
 	} while (read_seqretry(&xtime_lock, seq));
 
 	/* This cpu is going idle. Disable the periodic alarm. */
-	if (idle) {
-		vmi_timer_ops.cancel_alarm(VMI_CYCLES_AVAILABLE);
-		per_cpu(idle_start_jiffies, cpu) = jiffies;
-	}
-
+	vmi_timer_ops.cancel_alarm(VMI_CYCLES_AVAILABLE);
+	per_cpu(idle_start_jiffies, cpu) = jiffies;
 	/* Set the real time alarm to expire at the next event. */
 	vmi_timer_ops.set_alarm(
-		      VMI_ALARM_WIRING | VMI_ALARM_IS_ONESHOT | VMI_CYCLES_REAL,
-		      real_cycles_expiry, 0);
-
-	return idle;
+		VMI_ALARM_WIRING | VMI_ALARM_IS_ONESHOT | VMI_CYCLES_REAL,
+		real_cycles_expiry, 0);
+	return 1;
 }
 
 static void vmi_reenable_hz_timer(int cpu)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-02-07  1:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-06  3:53 [PATCH 6/11] Vmi timer no idle hz fixes.patch Zachary Amsden
2007-02-06 12:29 ` Andi Kleen
2007-02-07  0:22   ` Zachary Amsden
2007-02-07  1:05   ` Zachary Amsden

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).