Dynamic tick for x86 version 050602-1

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Jonathan Corbet
Sent: Tuesday, June 07, 2005 1:36 PM
To: Tony Lindgren
Cc: [email protected]
Subject: Re: [PATCH] Dynamic tick for x86 version 050602-1

Quoted 1 line · Tony Lindgren

--- linux-dev.orig/arch/i386/kernel/irq.c 2005-06-01

17:51:36.000000000 -0700

Quoted 1 line

+++ linux-dev/arch/i386/kernel/irq.c 2005-06-01

17:54:32.000000000 -0700

Review fastcall unsigned int do_IRQ(struct pt_r · line 108

Quoted 8 lines

[...]

(quoted diff) +3 -0

@@ -102,6 +103,12 @@ fastcall unsigned int do_IRQ(struct pt_r
 		);
 	} else
 #endif
+
+#ifdef CONFIG_NO_IDLE_HZ
+	if (dyn_tick->state & (DYN_TICK_ENABLED | 

DYN_TICK_SKIPPING) && irq != 0)

Review fastcall unsigned int do_IRQ(struct pt_r · line 112

Quoted 4 lines

(quoted diff) +3 -0

+		dyn_tick->interrupt(irq, NULL, regs);
+#endif
+
 		__do_IRQ(irq, regs);

Forgive me if I'm being obtuse (again...), but this hunk doesn't look like it would work well in the 4K stacks case. When 4K stacks are being
used, dyn_tick->interrupt() will only get called in the nested interrupt
case, when the interrupt stack is already in use. This change also pushes the non-assembly __do_IRQ() call out of the else branch, meaning that, when the switch is made to the interrupt stack (most of the time),
__do_IRQ() will be called twice for the same interrupt.

It looks to me like you want to put your #ifdef chunk *after* the call to __do_IRQ(), unless you have some reason for needing it to happen before the regular interrupt handler is invoked.

Review fastcall unsigned int do_IRQ(struct pt_r

Quoted 3 lines

(quoted diff) +1 -0

 	} else
 #endif
+

Review

Quoted 2 lines

(quoted diff) +2 -0

+#ifdef CONFIG_NO_IDLE_HZ
+	if (dyn_tick->state & (DYN_TICK_ENABLED | 

DYN_TICK_SKIPPING) && irq != 0)

Review

Quoted 4 lines

(quoted diff) +3 -0

+		dyn_tick->interrupt(irq, NULL, regs);
+#endif
+
 		__do_IRQ(irq, regs);

Quoted 41 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Jonathan Corbet
Sent: Tuesday, June 07, 2005 1:36 PM
To: Tony Lindgren
Cc: [email protected]
Subject: Re: [PATCH] Dynamic tick for x86 version 050602-1

Quoted 1 line · Tony Lindgren

--- linux-dev.orig/arch/i386/kernel/irq.c 2005-06-01

17:51:36.000000000 -0700

Quoted 1 line

+++ linux-dev/arch/i386/kernel/irq.c 2005-06-01

17:54:32.000000000 -0700

Review fastcall unsigned int do_IRQ(struct pt_r · line 108

Quoted 8 lines

[...]

(quoted diff) +3 -0

@@ -102,6 +103,12 @@ fastcall unsigned int do_IRQ(struct pt_r
 		);
 	} else
 #endif
+
+#ifdef CONFIG_NO_IDLE_HZ
+	if (dyn_tick->state & (DYN_TICK_ENABLED | 

DYN_TICK_SKIPPING) && irq != 0)

Review fastcall unsigned int do_IRQ(struct pt_r · line 112

Quoted 4 lines

(quoted diff) +3 -0

+		dyn_tick->interrupt(irq, NULL, regs);
+#endif
+
 		__do_IRQ(irq, regs);

Forgive me if I'm being obtuse (again...), but this hunk doesn't look like it would work well in the 4K stacks case. When 4K stacks are being
used, dyn_tick->interrupt() will only get called in the nested interrupt
case, when the interrupt stack is already in use. This change also pushes the non-assembly __do_IRQ() call out of the else branch, meaning that, when the switch is made to the interrupt stack (most of the time),
__do_IRQ() will be called twice for the same interrupt.

Quoted 4 lines

It looks to me like you want to put your #ifdef chunk *after* the call to __do_IRQ(), unless you have some reason for needing it to happen before the regular interrupt handler is invoked.

Good catch. This indeed looks like a bug.
With 050602-1 version I am seeing double the number of calls to timer_interrupt routine than expected. Say, when all CPUs are fully busy,
I see 2*HZ timer interrupt count in /proc/interrupts

And things look normal once I change this hunk as below

Quoted 3 lines

Review fastcall unsigned int do_IRQ(struct pt_r

Quoted 3 lines

(quoted diff) +1 -0

 	} else
 #endif
+

+ {

Quoted 7 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+#ifdef CONFIG_NO_IDLE_HZ
+	if (dyn_tick->state & (DYN_TICK_ENABLED | 

DYN_TICK_SKIPPING) && irq != 0)

Review

Quoted 4 lines

(quoted diff) +3 -0

+		dyn_tick->interrupt(irq, NULL, regs);
+#endif
+
 		__do_IRQ(irq, regs);

+ }

kernel/dyn-tick.c new file +16 -0

+/*
+ * ---------------------------------------------------------------------------
+ * Command line options
+ * ---------------------------------------------------------------------------
+ */
+static int __initdata dyntick_autoenable = 0;
+static int __initdata dyntick_useapic = 0;
+
+/*
+ * dyntick=[enable|disable],[forceapic]
+ */ 
+static int __init dyntick_setup(char *options)
+{
+	if (!options)
+		return 0;
+

kernel/dyn-tick.c new file +6 -0

+/*
+ * ---------------------------------------------------------------------------
+ * Sysfs interface
+ * ---------------------------------------------------------------------------
+ */
+

Hi!

Some more nitpicking...

Quoted 22 lines

kernel/dyn-tick.c new file +22 -0

+/*
+ * ---------------------------------------------------------------------------
+ * Command line options
+ * ---------------------------------------------------------------------------
+ */
+static int __initdata dyntick_autoenable = 0;
+static int __initdata dyntick_useapic = 0;
+
+/*
+ * dyntick=[enable|disable],[forceapic]
+ */ 
+static int __init dyntick_setup(char *options)
+{
+	if (!options)
+		return 0;
+
+	if (strstr(options, "enable"))
+		dyntick_autoenable = 1;
+
+	if (strstr(options, "forceapic"))
+		dyntick_useapic = 1;
+

kernel/dyn-tick.c line 118

Quoted 4 lines

kernel/dyn-tick.c new file +4 -0

+	return 0;
+}
+
+__setup("dyntick=", dyntick_setup);

Well, your parsing is little too simplistic. If I pass dyntick=do_not_dare_to_enable_it, it still enables :-).

Quoted 6 lines

kernel/dyn-tick.c new file +6 -0

+/*
+ * ---------------------------------------------------------------------------
+ * Sysfs interface
+ * ---------------------------------------------------------------------------
+ */
+

kernel/dyn-tick.c line 135

Quoted 10 lines

kernel/dyn-tick.c new file +10 -0

+extern struct sys_device device_timer;
+
+static ssize_t show_dyn_tick_state(struct sys_device *dev, char *buf)
+{
+	return sprintf(buf, "suitable:\t%i\n"
+		       "enabled:\t%i\n"
+		       "using APIC:\t%i\n",
+		       dyn_tick->state & DYN_TICK_SUITABLE,
+		       (dyn_tick->state & DYN_TICK_ENABLED) >> 1,
+		       (dyn_tick->state & DYN_TICK_USE_APIC) >> 3);

You basically hardcode values of DYN_TICK_* here. Why not use !!() and loose dependency?

Pavel

Thanks for all the comments. Here's an updated dyntick patch.

Quoted 1 line · Tony Lindgren

Thanks for all the comments. Here's an updated dyntick patch.

Patches with 3 minor rejects against -rc6-mm1, boots, and seems to work well on my Dell Latitude C840 laptop - although running at full load with seti@home causes the expected 250 timer ticks/sec, running a mostly-idle X session only gets about 117, and having xmms and a few other things running it hits about 170 tics/sec. I've had the CPU speed bounce between 1.2G and 1.6G a few times and it didn't seem to blink either. Even NTP is happy with what it sees.. ;)

Need to rebuild with CONFIG_HZ=1000 and see what it does, and see what it does to actual power consumption.

Minor nit: The implementation of /sys/devices/system/timer/timer0/dyn_tick_state violates the one-value-per-file rule for sysfs. I suspect this needs to become a directory with 3-4 files in it, each containing one value.

You may also want to check out the patch by Thomas Renninger for ACPI C-states. I've added a link to it at:

http://muru.com/dyntick/

Quoted 4 lines · Tony Lindgren

You may also want to check out the patch by Thomas Renninger for ACPI C-states. I've added a link to it at:

http://muru.com/dyntick/

I think that's muru.com/linux/dyntick ?

I'm not sure what Thomas's patch will do for me

% cat /proc/acpi/processor/CPU0/power

active state:            C2
max_cstate:              C8
bus master activity:     00000000
states:
    C1:                  type[C1] promotion[C2] demotion[--] latency[000] usage[00000010]

*C2: type[C2] promotion[--] demotion[C1] latency[050] usage[01314979]

Near as I can tell, we start off in C1, drop into C2, and stay there no matter what happens - we never move back up to C1, and there's no C3 to drop into....

Should there be a C3/C4? Is my laptop just plain borked? :)

Quoted 8 lines · [email protected] wrote:

Quoted 4 lines · Tony Lindgren

You may also want to check out the patch by Thomas Renninger for ACPI C-states. I've added a link to it at:

http://muru.com/dyntick/

I think that's muru.com/linux/dyntick ?

Quoted 1 line

I'm not sure what Thomas's patch will do for me

Not much.
The one measures how long your machine really stays in each C-state (Tony's pmstats should be sufficient for you).

The other one tried to calc the next C-state to go, based on statistics of bus master activity and idleness of the machine.
But it is *wrong*.
Tony could you please remove the link to:
ftp://ftp.suse.com/pub/people/trenn/dyn_tick_c_states/dynamic_tick_cstate_patch.diff Therefore we also will never get such good results as stated in:
ftp://ftp.suse.com/pub/people/trenn/dyn_tick_c_states/measures_C4_machine

The problem is that if there is bus master activity, a certain amount of time has to be waited (nobody could tell me how long this must be, currently
it's 40 ms. Then it's assumed bm transfers have been finished) before C3/C4 can be called -> -> bm activity is not interrupt driven -> this needs ticks to be enabled.

Therefore a final patch could look like:
Let ticks be enabled (maybe reduced?) as long as machine is still in C1/C2 and only disable them for deeper sleeping states (C3/C4).

- here's what I currently have:

Quoted 7 lines

% cat /proc/acpi/processor/CPU0/power

active state:            C2
max_cstate:              C8
bus master activity:     00000000
states:
    C1:                  type[C1] promotion[C2] demotion[--] latency[000] usage[00000010]

Quoted 7 lines

*C2: type[C2] promotion[--] demotion[C1] latency[050] usage[01314979]

Near as I can tell, we start off in C1, drop into C2, and stay there no matter what happens - we never move back up to C1, and there's no C3 to drop into....

Should there be a C3/C4? Is my laptop just plain borked? :)

Depends on your machine and BIOS, whether it's supported -> seems as if it's not.

You could verify by having a deeper look in your FADT/DSDT.
You need the acpi tools from Len Brown (acpidmp/acpixtract) and the iasl Intel ACPI compiler.
AFAIK checking for C-support is rather robust in recent kernels as long as you don't have a broken DSDT table.
Maybe you find a newer BIOS supporting C3?

To be honest, I doubt you save much power even with dyn tick enabled if you only have support for C1 and C2. The pmstats tool from Tony (see link above)
could tell you nicely whether you gain anything.

Quoted 1 line

Should there be a C3/C4? Is my laptop just plain borked? :)

Depends on your machine and BIOS, whether it's supported -> seems as if it's not.

You could verify by having a deeper look in your FADT/DSDT.
You need the acpi tools from Len Brown (acpidmp/acpixtract) and the iasl Intel ACPI compiler.
AFAIK checking for C-support is rather robust in recent kernels as long as you don't have a broken DSDT table.

Maybe you find a newer BIOS supporting C3?

To be honest, I doubt you save much power even with dyn tick enabled if you only have support for C1 and C2. The pmstats tool from Tony (see link above)
could tell you nicely whether you gain anything.

Quoted 3 lines · Thomas Renninger

Quoted 1 line

Should there be a C3/C4? Is my laptop just plain borked? :)

Depends on your machine and BIOS, whether it's supported -> seems as if it's not.

Quoted 5 lines

You could verify by having a deeper look in your FADT/DSDT.
You need the acpi tools from Len Brown (acpidmp/acpixtract) and the iasl Intel ACPI compiler.
AFAIK checking for C-support is rather robust in recent kernels as long as you don't have a broken DSDT table.

OK, found acpidmp and iasl, now have a decompiled DSDT - now to figure out if it's busticated or not.... :)

Quoted 2 lines

Maybe you find a newer BIOS supporting C3?

Nope, I just checked, and the A13 BIOS from 02/06/2004 is the latest that Dell has released for the C840. Not much hope there unless there's some special secret site that even newer BIOS updates hide until they escape.. ;)

Quoted 3 lines

To be honest, I doubt you save much power even with dyn tick enabled if you only have support for C1 and C2. The pmstats tool from Tony (see link above)
could tell you nicely whether you gain anything.

Well, it's a start, anyhow. :)

There are two ways C-state addresses are exported to OS.

- Some flags in the FADT (-> ACPI spec) -> this gives you two C-states maximum, AFAIK.

- Through the _CST function in your DSDT (-> ACPI spec, sorry). If you have have a look in dsdt.dsl at the _CST function there are that much packages returned as your BIOS claims to support. Hmm, _CST code is often in the SSDT an extention of the DSDT code. If you have one: acpidmp > acpidmp; acpixtract ssdt acpidmp >my_ssdt; iasl -d my_ssdt.

2. reprogram_apic_timer seems to reprogram the count-down APIC timer (APIC_TMICT) with an integral number of apic_timer_val. How accurate will this be? Shouldnt this take into account that we may not be reprogramming the timer on exactly "jiffy" boundary?

3. Is there any strong reason why you reprogram timers only when _all_ CPUs are idle?

Quoted 6 lines · Srivatsa Vaddagiri

2. reprogram_apic_timer seems to reprogram the count-down APIC timer (APIC_TMICT) with an integral number of apic_timer_val. How accurate will this be? Shouldnt this take into account that we may not be reprogramming the timer on exactly "jiffy" boundary?

All PIT based reprogramming schemes will lose time.

Only with HPET you can do better (but even there it is difficult to do properly)

Quoted 2 lines

3. Is there any strong reason why you reprogram timers only when _all_ CPUs are idle?

There is none imho - my x86-64 no idle tick patch doesn't do it.

Actually there is a small reason - RCU currently does not get updated by a fully idle CPU and can stall other CPUs. But that is in practice not too big an issue yet because so many subsystems cause ticks now and then, so the CPUs tend to wake up often enough to not stall the rest of the system too badly.

Quoted 8 lines · Andi Kleen

Quoted 6 lines · Srivatsa Vaddagiri

2. reprogram_apic_timer seems to reprogram the count-down APIC timer (APIC_TMICT) with an integral number of apic_timer_val. How accurate will this be? Shouldnt this take into account that we may not be reprogramming the timer on exactly "jiffy" boundary?

Quoted 1 line

All PIT based reprogramming schemes will lose time.

Not true if the timesource is different from interrupt source.

Consider PM timer for timesource, and PIT for interrupt source. Reprogamming PIT should not affect PM timer. Time is always updated from PM timer.

Quoted 6 lines

Actually there is a small reason - RCU currently does not get updated by a fully idle CPU and can stall other CPUs. But that is in practice not too big an issue yet because so many subsystems cause ticks now and then, so the CPUs tend to wake up often enough to not stall the rest of the system too badly.

I guess it should be safe to reprogram timer even if other CPUs are not idle, assuming the busy CPUs reprogramming timer will also wake up the idle CPUs.

There's one thing that should be considered though; Reprogamming timers should be avoided if the system is busy as it causes performance issues. Especially reprogramming PIT.

Quoted 11 lines · Tony Lindgren

Quoted 9 lines · Andi Kleen

Quoted 6 lines · Srivatsa Vaddagiri

2. reprogram_apic_timer seems to reprogram the count-down APIC timer (APIC_TMICT) with an integral number of apic_timer_val. How accurate will this be? Shouldnt this take into account that we may not be reprogramming the timer on exactly "jiffy" boundary?

All PIT based reprogramming schemes will lose time.

Quoted 4 lines

Not true if the timesource is different from interrupt source.

Consider PM timer for timesource, and PIT for interrupt source. Reprogamming PIT should not affect PM timer. Time is always updated from PM timer.

PM timer is not really suitable for this because it overflows too quickly (several times a second).

Also you still lose time in timers (e.g. your internal timers slowly drift) unless you regularly sync with the time source, but that has other drawbacks.

Quoted 11 lines

Quoted 6 lines

Actually there is a small reason - RCU currently does not get updated by a fully idle CPU and can stall other CPUs. But that is in practice not too big an issue yet because so many subsystems cause ticks now and then, so the CPUs tend to wake up often enough to not stall the rest of the system too badly.

I guess it should be safe to reprogram timer even if other CPUs are not idle, assuming the busy CPUs reprogramming timer will also wake up the idle CPUs.

Quoted 3 lines

There's one thing that should be considered though; Reprogamming timers should be avoided if the system is busy as it causes performance issues. Especially reprogramming PIT.

Just forget about reprogramming with PIT. IMHO that should be never used in production. The right way for this is HPET.

The main issue with HPET is that many BIOS even though the chipsets have it don't set up the HPET table because Windows doesn't use it right now. However that can be avoided with some chipset specific code.

The main issue with HPET is that many BIOS even though the chipsets have it don't set up the HPET table because Windows doesn't use it right now. However that can be avoided with some chipset specific code.

Hi Tony,
I went through the dynamic-tick patch on your website
(patch-dynamic-tick-2.6.12-rc6-050610-1) and was having some questions about it:

1. dyn_tick->skip is set to the number of ticks that have to be skipped. This is set on the CPU which is the last (in online_map) to go idle and is based on when that CPU's next timer is set to expire.

Other CPUs also seem to use the same interval
to skip ticks. Shouldnt other CPU check their nearest timer rather than blindly skipping dyn_tick->skip number of ticks?

2. reprogram_apic_timer seems to reprogram the count-down APIC timer (APIC_TMICT) with an integral number of apic_timer_val. How accurate will this be? Shouldnt this take into account that we may not be reprogramming the timer on exactly "jiffy" boundary?

3. Is there any strong reason why you reprogram timers only when _all_ CPUs are idle?

4. In what aspects you think does your patch differ from VST (other than not relying on HRT!)?