Re: [PATCH] remove i386 dynamic ticks ifdefs

Re: [PATCH] remove i386 dynamic ticks ifdefs

[Pavel Machek]

Hi!

> I assume you're maintaining the dyn tick patches for i386 posted on the muru 
> website as your email is listed there. I thought you might be interested in 
> this patch for dyn-ticks which removes most of the #ifdefs out of common code 
> paths as per linux kernel style and moves more code into dyn-tick.c. Most of 
> it is straight forward code reorganisation, but to keep do_timer_interrupt 
> inlined I'd have to move it's code around somewhat. That may be a better 
> option but I've tried to fiddle with the mainline code as little as possible.
> 
> Patch applies to 2.6.12 with patch-dynamic-tick-2.6.12-rc6-050610-1 applied
> 
> cc'ed lkml just for public record of the patch.

Please inline patches...

You broke indentation in one of first hunks, and you probably
want empty functions to be static inline, so that we do not eat
function call overhead.
				Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

[PATCH] remove i386 dynamic ticks ifdefs

[Con Kolivas]

[-- Attachment #1.1: Type: text/plain, Size: 704 bytes --]

Hi Tony

I assume you're maintaining the dyn tick patches for i386 posted on the muru 
website as your email is listed there. I thought you might be interested in 
this patch for dyn-ticks which removes most of the #ifdefs out of common code 
paths as per linux kernel style and moves more code into dyn-tick.c. Most of 
it is straight forward code reorganisation, but to keep do_timer_interrupt 
inlined I'd have to move it's code around somewhat. That may be a better 
option but I've tried to fiddle with the mainline code as little as possible.

Patch applies to 2.6.12 with patch-dynamic-tick-2.6.12-rc6-050610-1 applied

cc'ed lkml just for public record of the patch.

Cheers,
Con

[-- Attachment #1.2: dt-tweaks.patch --]
[-- Type: text/x-diff, Size: 14117 bytes --]

Move most of the dynamic ticks code that is #ifdef'd out of code paths and
put it into dyn-tick.c

Signed-off-by: Con Kolivas <kernel@kolivas.org>

 arch/i386/kernel/Makefile   |    2
 arch/i386/kernel/apic.c     |   36 ----------
 arch/i386/kernel/dyn-tick.c |  146 ++++++++++++++++++++++++++++++++++++++++++++
 arch/i386/kernel/irq.c      |    5 -
 arch/i386/kernel/time.c     |   72 ---------------------
 include/asm/dyn-tick.h      |    4 -
 include/linux/dyn-tick.h    |   12 +++
 7 files changed, 165 insertions(+), 112 deletions(-)

Index: linux-2.6.12-dt/arch/i386/kernel/apic.c
===================================================================
--- linux-2.6.12-dt.orig/arch/i386/kernel/apic.c	2005-07-29 01:43:15.000000000 +1000
+++ linux-2.6.12-dt/arch/i386/kernel/apic.c	2005-07-29 01:49:05.000000000 +1000
@@ -932,11 +932,8 @@ static void __setup_APIC_LVTT(unsigned i
 
 	apic_timer_val = clocks/APIC_DIVISOR;
 
-#ifdef CONFIG_NO_IDLE_HZ
-	/* Local APIC timer is 24-bit */
 	if (apic_timer_val)
-		dyn_tick->max_skip = 0xffffff / apic_timer_val;
-#endif
+		set_dyn_tick_max_skip(apic_timer_val);
 
 	apic_write_around(APIC_TMICT, apic_timer_val);
 }
@@ -1051,12 +1048,7 @@ void __init setup_boot_APIC_clock(void)
 	 */
 	setup_APIC_timer(calibration_result);
 
-#ifdef CONFIG_NO_IDLE_HZ
-	if (calibration_result)
-		dyn_tick->state |= DYN_TICK_USE_APIC;
-	else
-		printk(KERN_INFO "dyn-tick: Cannot use local APIC\n");
-#endif
+	setup_dyn_tick_use_apic(calibration_result);
 
 	local_irq_enable();
 }
@@ -1086,18 +1078,6 @@ void enable_APIC_timer(void)
 	}
 }
 
-#if defined(CONFIG_NO_IDLE_HZ)
-void reprogram_apic_timer(unsigned int count)
-{
-	unsigned long flags;
-
-	count *= apic_timer_val;
-	local_irq_save(flags);
-	apic_write_around(APIC_TMICT, count);
-	local_irq_restore(flags);
-}
-#endif
-
 /*
  * the frequency of the profiling timer can be changed
  * by writing a multiplier value into /proc/profile.
@@ -1210,21 +1190,11 @@ fastcall void smp_apic_timer_interrupt(s
 	 */
 	irq_enter();
 
-#ifdef CONFIG_NO_IDLE_HZ
 	/*
 	 * Check if we need to wake up PIT interrupt handler.
 	 * Otherwise just wake up local APIC timer.
 	 */
-	do {
-		seq = read_seqbegin(&xtime_lock);
-		if (dyn_tick->state & (DYN_TICK_ENABLED | DYN_TICK_SKIPPING)) {
-			if (dyn_tick->skip_cpu == cpu && dyn_tick->skip > DYN_TICK_MIN_SKIP)
-				dyn_tick->interrupt(99, NULL, regs);
-			else
-				reprogram_apic_timer(1);
-		}
-	} while (read_seqretry(&xtime_lock, seq));
-#endif
+	wakeup_pit_or_apic(seq, cpu, &regs);
 
 	smp_local_timer_interrupt(regs);
 	irq_exit();
Index: linux-2.6.12-dt/arch/i386/kernel/dyn-tick.c
===================================================================
--- linux-2.6.12-dt.orig/arch/i386/kernel/dyn-tick.c	2005-07-29 01:43:15.000000000 +1000
+++ linux-2.6.12-dt/arch/i386/kernel/dyn-tick.c	2005-07-29 11:46:48.000000000 +1000
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/dyn-tick.h>
 
+#ifdef CONFIG_NO_IDLE_HZ
 void arch_reprogram_timer(void)
 {
 	if (cpu_has_local_apic()) {
@@ -43,3 +44,148 @@ int __init dyn_tick_init(void)
 	return 0;
 }
 arch_initcall(dyn_tick_init);
+
+static unsigned long long last_tick;
+
+/*
+ * This interrupt handler updates the time based on number of jiffies skipped
+ * It would be somewhat more optimized to have a customa handler in each timer
+ * using hardware ticks instead of nanoseconds. Note that CONFIG_NO_IDLE_HZ
+ * currently disables timer fallback on skipped jiffies.
+ */
+irqreturn_t dyn_tick_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+	unsigned long flags;
+	volatile unsigned long long now;
+	unsigned int skipped = 0;
+
+	write_seqlock_irqsave(&xtime_lock, flags);
+	now = cur_timer->monotonic_clock();
+	while (now - last_tick >= NS_TICK_LEN) {
+		last_tick += NS_TICK_LEN;
+		cur_timer->mark_offset();
+		do_timer_interrupt(irq, NULL, regs);
+		skipped++;
+	}
+	if (dyn_tick->state & (DYN_TICK_ENABLED | DYN_TICK_SKIPPING)) {
+		dyn_tick->skip = 1;
+		if (cpu_has_local_apic())
+			reprogram_apic_timer(dyn_tick->skip);
+		reprogram_pit_timer(dyn_tick->skip);
+		dyn_tick->state |= DYN_TICK_ENABLED;
+		dyn_tick->state &= ~DYN_TICK_SKIPPING;
+	}
+	write_sequnlock_irqrestore(&xtime_lock, flags);
+
+	return IRQ_HANDLED;
+}
+
+int __init dyn_tick_arch_init(void)
+{
+	unsigned long flags;
+
+	write_seqlock_irqsave(&xtime_lock, flags);
+	last_tick = cur_timer->monotonic_clock();
+	dyn_tick->skip = 1;
+	if (!(dyn_tick->state & DYN_TICK_USE_APIC) || !cpu_has_local_apic())
+		dyn_tick->max_skip = 0xffff/LATCH;	/* PIT timer length */
+	printk(KERN_INFO "dyn-tick: Maximum ticks to skip limited to %i\n",
+	       dyn_tick->max_skip);
+	write_sequnlock_irqrestore(&xtime_lock, flags);
+
+	dyn_tick->interrupt = dyn_tick_timer_interrupt;
+	replace_timer_interrupt(dyn_tick->interrupt);
+
+	return 0;
+}
+
+/* Reproduced functions below here */
+inline void set_dyn_tick_max_skip(u32 apic_timer_val)
+{
+	dyn_tick->max_skip = 0xffffff / apic_timer_val;
+}
+
+inline void setup_dyn_tick_use_apic(unsigned int calibration_result)
+{
+	if (calibration_result)
+		dyn_tick->state |= DYN_TICK_USE_APIC;
+	else
+		printk(KERN_INFO "dyn-tick: Cannot use local APIC\n");
+}
+
+void wakeup_pit_or_apic(unsigned long seq, int cpu, struct pt_regs *regs)
+{
+	do {
+		seq = read_seqbegin(&xtime_lock);
+		if (dyn_tick->state & (DYN_TICK_ENABLED | DYN_TICK_SKIPPING)) {
+			if (dyn_tick->skip_cpu == cpu && dyn_tick->skip > DYN_TICK_MIN_SKIP)
+				dyn_tick->interrupt(99, NULL, regs);
+			else
+				reprogram_apic_timer(1);
+		}
+	} while (read_seqretry(&xtime_lock, seq));
+}
+
+void dyn_tick_interrupt(int irq, struct pt_regs *regs)
+{
+	if (dyn_tick->state & (DYN_TICK_ENABLED | DYN_TICK_SKIPPING) && irq != 0)
+		dyn_tick->interrupt(irq, NULL, regs);
+}
+
+void dyn_tick_time_init(struct timer_opts *cur_timer)
+{
+	if (strncmp(cur_timer->name, "tsc", 3) == 0 ||
+	    strncmp(cur_timer->name, "pmtmr", 3) == 0) {
+		dyn_tick->state |= DYN_TICK_SUITABLE;
+		printk(KERN_INFO "dyn-tick: Found suitable timer: %s\n",
+		       cur_timer->name);
+	} else
+		printk(KERN_ERR "dyn-tick: Cannot use timer %s\n",
+		       cur_timer->name);
+}
+
+#ifdef CONFIG_X86_LOCAL_APIC
+void reprogram_apic_timer(unsigned int count)
+{
+	unsigned long flags;
+
+	count *= apic_timer_val;
+	local_irq_save(flags);
+	apic_write_around(APIC_TMICT, count);
+	local_irq_restore(flags);
+}
+#else
+void reprogram_apic_timer(unsigned int count)
+{
+}
+
+#endif
+
+#else
+inline void set_dyn_tick_max_skip(u32 apic_timer_val)
+{
+}
+
+void reprogram_apic_timer(unsigned int count)
+{
+}
+
+inline void setup_dyn_tick_use_apic(unsigned int calibration_result)
+{
+}
+
+void wakeup_pit_or_apic(unsigned long seq, int cpu, struct pt_regs *regs)
+{
+}
+
+void dyn_tick_interrupt(int irq, struct pt_regs *regs)
+{
+}
+
+void dyn_tick_time_init(struct timer_opts *cur_timer)
+{
+}
+
+#endif
+
+
Index: linux-2.6.12-dt/arch/i386/kernel/irq.c
===================================================================
--- linux-2.6.12-dt.orig/arch/i386/kernel/irq.c	2005-07-29 01:43:15.000000000 +1000
+++ linux-2.6.12-dt/arch/i386/kernel/irq.c	2005-07-29 02:03:33.000000000 +1000
@@ -74,10 +74,7 @@ fastcall unsigned int do_IRQ(struct pt_r
 	}
 #endif
 
-#ifdef CONFIG_NO_IDLE_HZ
-	if (dyn_tick->state & (DYN_TICK_ENABLED | DYN_TICK_SKIPPING) && irq != 0)
-		dyn_tick->interrupt(irq, NULL, regs);
-#endif
+	dyn_tick_interrupt(irq, regs);
 
 #ifdef CONFIG_4KSTACKS
 
Index: linux-2.6.12-dt/arch/i386/kernel/Makefile
===================================================================
--- linux-2.6.12-dt.orig/arch/i386/kernel/Makefile	2005-07-29 00:06:26.000000000 +1000
+++ linux-2.6.12-dt/arch/i386/kernel/Makefile	2005-07-29 02:05:54.000000000 +1000
@@ -31,7 +31,7 @@ obj-$(CONFIG_MODULES)		+= module.o
 obj-y				+= sysenter.o vsyscall.o
 obj-$(CONFIG_ACPI_SRAT) 	+= srat.o
 obj-$(CONFIG_HPET_TIMER) 	+= time_hpet.o
-obj-$(CONFIG_NO_IDLE_HZ) 	+= dyn-tick.o
+obj-y			 	+= dyn-tick.o
 obj-$(CONFIG_EFI) 		+= efi.o efi_stub.o
 obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
 
Index: linux-2.6.12-dt/arch/i386/kernel/time.c
===================================================================
--- linux-2.6.12-dt.orig/arch/i386/kernel/time.c	2005-07-29 01:43:15.000000000 +1000
+++ linux-2.6.12-dt/arch/i386/kernel/time.c	2005-07-29 11:53:52.000000000 +1000
@@ -248,7 +248,7 @@ EXPORT_SYMBOL(profile_pc);
  * timer_interrupt() needs to keep up the real-time clock,
  * as well as call the "do_timer()" routine every clocktick
  */
-static inline void do_timer_interrupt(int irq, void *dev_id,
+void do_timer_interrupt(int irq, void *dev_id,
 					struct pt_regs *regs)
 {
 #ifdef CONFIG_X86_IO_APIC
@@ -309,43 +309,6 @@ irqreturn_t timer_interrupt(int irq, voi
 	return IRQ_HANDLED;
 }
 
-#ifdef CONFIG_NO_IDLE_HZ
-static unsigned long long last_tick;
-
-/*
- * This interrupt handler updates the time based on number of jiffies skipped
- * It would be somewhat more optimized to have a customa handler in each timer
- * using hardware ticks instead of nanoseconds. Note that CONFIG_NO_IDLE_HZ
- * currently disables timer fallback on skipped jiffies.
- */
-irqreturn_t dyn_tick_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
-{
-	unsigned long flags;
-	volatile unsigned long long now;
-	unsigned int skipped = 0;
-
-	write_seqlock_irqsave(&xtime_lock, flags);
-	now = cur_timer->monotonic_clock();
-	while (now - last_tick >= NS_TICK_LEN) {
-		last_tick += NS_TICK_LEN;
-		cur_timer->mark_offset();
-		do_timer_interrupt(irq, NULL, regs);
-		skipped++;
-	}
-	if (dyn_tick->state & (DYN_TICK_ENABLED | DYN_TICK_SKIPPING)) {
-		dyn_tick->skip = 1;
-		if (cpu_has_local_apic())
-			reprogram_apic_timer(dyn_tick->skip);
-		reprogram_pit_timer(dyn_tick->skip);
-		dyn_tick->state |= DYN_TICK_ENABLED;
-		dyn_tick->state &= ~DYN_TICK_SKIPPING;
-	}
-	write_sequnlock_irqrestore(&xtime_lock, flags);
-
-	return IRQ_HANDLED;
-}
-#endif	/* CONFIG_NO_IDLE_HZ */
-
 /* not static: needed by APM */
 unsigned long get_cmos_time(void)
 {
@@ -490,28 +453,6 @@ static void __init hpet_time_init(void)
 }
 #endif
 
-#ifdef CONFIG_NO_IDLE_HZ
-
-int __init dyn_tick_arch_init(void)
-{
-	unsigned long flags;
-
-	write_seqlock_irqsave(&xtime_lock, flags);
-	last_tick = cur_timer->monotonic_clock();
-	dyn_tick->skip = 1;
-	if (!(dyn_tick->state & DYN_TICK_USE_APIC) || !cpu_has_local_apic())
-		dyn_tick->max_skip = 0xffff/LATCH;	/* PIT timer length */
-	printk(KERN_INFO "dyn-tick: Maximum ticks to skip limited to %i\n",
-	       dyn_tick->max_skip);
-	write_sequnlock_irqrestore(&xtime_lock, flags);
-
-	dyn_tick->interrupt = dyn_tick_timer_interrupt;
-	replace_timer_interrupt(dyn_tick->interrupt);
-
-	return 0;
-}
-#endif	/* CONFIG_NO_IDLE_HZ */
-
 void __init time_init(void)
 {
 #ifdef CONFIG_HPET_TIMER
@@ -532,16 +473,7 @@ void __init time_init(void)
 	cur_timer = select_timer();
 	printk(KERN_INFO "Using %s for high-res timesource\n",cur_timer->name);
 
-#ifdef CONFIG_NO_IDLE_HZ
-	if (strncmp(cur_timer->name, "tsc", 3) == 0 ||
-	    strncmp(cur_timer->name, "pmtmr", 3) == 0) {
-		dyn_tick->state |= DYN_TICK_SUITABLE;
-		printk(KERN_INFO "dyn-tick: Found suitable timer: %s\n",
-		       cur_timer->name);
-	} else
-		printk(KERN_ERR "dyn-tick: Cannot use timer %s\n",
-		       cur_timer->name);
-#endif
+	dyn_tick_time_init(cur_timer);
 
 	time_init_hook();
 }
Index: linux-2.6.12-dt/include/asm/dyn-tick.h
===================================================================
--- linux-2.6.12-dt.orig/include/asm/dyn-tick.h	2005-07-29 01:43:15.000000000 +1000
+++ linux-2.6.12-dt/include/asm/dyn-tick.h	2005-07-29 01:58:54.000000000 +1000
@@ -19,11 +19,7 @@ extern void reprogram_pit_timer(int jiff
 extern void reprogram_apic_timer(unsigned int count);
 extern void replace_timer_interrupt(void * new_handler);
 
-#if defined(CONFIG_NO_IDLE_HZ) && defined(CONFIG_X86_LOCAL_APIC)
 extern void reprogram_apic_timer(unsigned int count);
-#else
-#define reprogram_apic_timer(x)	do {} while (0)
-#endif
 
 #if defined(CONFIG_DYN_TICK_USE_APIC) && (defined(CONFIG_SMP) || defined(CONFIG_X86_UP_APIC))
 #define cpu_has_local_apic()	(dyn_tick->state & DYN_TICK_USE_APIC)
Index: linux-2.6.12-dt/include/linux/dyn-tick.h
===================================================================
--- linux-2.6.12-dt.orig/include/linux/dyn-tick.h	2005-07-29 01:43:15.000000000 +1000
+++ linux-2.6.12-dt/include/linux/dyn-tick.h	2005-07-29 11:41:48.000000000 +1000
@@ -14,6 +14,7 @@
 #define _DYN_TICK_TIMER_H
 
 #include <linux/interrupt.h>
+#include <asm/timer.h>
 
 #define DYN_TICK_TIMER_INT	(1 << 4)
 #define DYN_TICK_USE_APIC	(1 << 3)
@@ -39,6 +40,14 @@ struct dyn_tick_timer {
 
 extern struct dyn_tick_state * dyn_tick;
 extern void dyn_tick_register(struct dyn_tick_timer * new_timer);
+extern inline void set_dyn_tick_max_skip(u32 apic_timer_val);
+extern void reprogram_apic_timer(unsigned int count);
+extern inline void setup_dyn_tick_use_apic(unsigned int calibration_result);
+extern void wakeup_pit_or_apic(unsigned long seq, int cpu, struct pt_regs *regs);
+extern void dyn_tick_interrupt(int irq, struct pt_regs *regs);
+extern void dyn_tick_time_init(struct timer_opts *cur_timer);
+extern void do_timer_interrupt(int irq, void *dev_id,
+					struct pt_regs *regs);
 
 #define NS_TICK_LEN		((1 * 1000000000)/HZ)
 #define DYN_TICK_MIN_SKIP	2
@@ -47,6 +56,9 @@ extern void dyn_tick_register(struct dyn
 #ifdef CONFIG_NO_IDLE_HZ
 
 extern unsigned long dyn_tick_reprogram_timer(void);
+extern irqreturn_t dyn_tick_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+extern int __init dyn_tick_arch_init(void);
+
 #define dyn_tick_enabled()		(dyn_tick->state & DYN_TICK_ENABLED)
 
 #else

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] remove i386 dynamic ticks ifdefs

[Con Kolivas]

On Tue, 1 Jan 2002 21:33, Pavel Machek wrote:
> Hi!
>
> > I assume you're maintaining the dyn tick patches for i386 posted on the
> > muru website as your email is listed there. I thought you might be
> > interested in this patch for dyn-ticks which removes most of the #ifdefs
> > out of common code paths as per linux kernel style and moves more code
> > into dyn-tick.c. Most of it is straight forward code reorganisation, but
> > to keep do_timer_interrupt inlined I'd have to move it's code around
> > somewhat. That may be a better option but I've tried to fiddle with the
> > mainline code as little as possible.
> >
> > Patch applies to 2.6.12 with patch-dynamic-tick-2.6.12-rc6-050610-1
> > applied
> >
> > cc'ed lkml just for public record of the patch.
>
> Please inline patches...

Not everyone uses console email clients :| It was an inlined attachment rather 
than an ordinary attachment but clearly that doesn't suit those with console 
clients. I'm sorry.

>
> You broke indentation in one of first hunks, and you probably
> want empty functions to be static inline, so that we do not eat
> function call overhead.

Will examine. Thanks very much for your code comments!

Cheers,
Con