Skip to contentSkip to conversation

TIMER_BH-less smptimers

RFC PATCH In review Dipankar Sarma 3 participants 8 messages May 16, 2002 – May 21, 2002

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 TIMER_BH-less smptimers

I have been experimenting with Ingo's smptimers and I ended up extending it a little bit. I would really appreciate comments on whether these things make sense or not.

The major changes are -

1. I removed TIMER_BH. The vector is defined but not used.

2. Not having the TIMER_BH requires additional work for serializing old protocol code (deliver_to_old_ones()) to work with new timers. The protocol code is already serialized using net_bh_lock. So, I used it to serialize old protocol code with new timers.

3. Ingo's run_local_timers() had to do all sorts of locking to keep legacy serialization happy. I just added net_bh_lock to that list. This makes sure that timers don't run with old protocol code.

4. When legacy locking fails in run_local_timers(), I use a per-cpu tasklet to run the timers for that cpu. This tasklet also uses the same locking scheme to support legacy stuff.

5. I expanded it to all smp architectures that I could see, but no testing has been done yet for non-i386. As and when I can boot 2.5 kernels on ia64 and ppc, I will test it. I am reasonably sure I didn't get mips right, if not others.

The patch has been tested with *2.5.14* (applies cleanly) and ltp in a 4CPU i386. Unfortunately, the legacy code still requires us to do serialize timers with global locks.

Thanks

Signature · 2 lines
Dipankar Sarma  <[email protected]> http://lse.sourceforge.net
Linux Technology Center, IBM Software Lab, Bangalore, India.
arch/alpha/kernel/smp.c +2 -0
diff -urN linux-2.5.12-base/arch/alpha/kernel/smp.c linux-2.5.12-smptimers/arch/alpha/kernel/smp.c
@@ -24,6 +24,7 @@
 #include <linux/spinlock.h>
 #include <linux/irq.h>
 #include <linux/cache.h>
+#include <linux/timer.h>
 
 #include <asm/hwrpb.h>
 #include <asm/ptrace.h>
@@ -657,6 +658,7 @@
 		if (softirq_pending(cpu))
 			do_softirq();
 	}
+	run_local_timers();
 }
 
 int __init
arch/i386/kernel/apic.c +2 -1
diff -urN linux-2.5.12-base/arch/i386/kernel/apic.c linux-2.5.12-smptimers/arch/i386/kernel/apic.c
@@ -23,6 +23,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <linux/kernel_stat.h>
+#include <linux/timer.h>
 
 #include <asm/atomic.h>
 #include <asm/smp.h>
@@ -1087,7 +1088,7 @@
 	irq_enter(cpu, 0);
 	smp_local_timer_interrupt(&regs);
 	irq_exit(cpu, 0);
-
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 }
arch/i386/mm/fault.c +1 -5
diff -urN linux-2.5.12-base/arch/i386/mm/fault.c linux-2.5.12-smptimers/arch/i386/mm/fault.c
@@ -94,16 +94,12 @@
 	goto bad_area;
 }
 
-extern spinlock_t timerlist_lock;
-
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 #ifdef CONFIG_SMP
arch/ia64/kernel/smp.c +2 -0
diff -urN linux-2.5.12-base/arch/ia64/kernel/smp.c linux-2.5.12-smptimers/arch/ia64/kernel/smp.c
@@ -32,6 +32,7 @@
 #include <linux/cache.h>
 #include <linux/delay.h>
 #include <linux/cache.h>
+#include <linux/timer.h>
 
 #include <asm/atomic.h>
 #include <asm/bitops.h>
@@ -330,6 +331,7 @@
 		local_cpu_data->prof_counter = local_cpu_data->prof_multiplier;
 		update_process_times(user);
 	}
+	run_local_timers();
 }
 
 /*
arch/ia64/kernel/traps.c +1 -3
diff -urN linux-2.5.12-base/arch/ia64/kernel/traps.c linux-2.5.12-smptimers/arch/ia64/kernel/traps.c
@@ -42,7 +42,6 @@
 
 #include <asm/fpswa.h>
 
-extern spinlock_t timerlist_lock;
 
 static fpswa_interface_t *fpswa_interface;
 
@@ -56,13 +55,12 @@
 }
 
 /*
- * Unlock any spinlocks which will prevent us from getting the message out (timerlist_lock
+ * Unlock any spinlocks which will prevent us from getting the message out 
  * is acquired through the console unblank code)
  */
 void
 bust_spinlocks (int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 #ifdef CONFIG_SMP
arch/mips64/mm/fault.c +1 -4
diff -urN linux-2.5.12-base/arch/mips64/mm/fault.c linux-2.5.12-smptimers/arch/mips64/mm/fault.c
@@ -58,16 +58,13 @@
 	printk("Got exception 0x%lx at 0x%lx\n", retaddr, regs.cp0_epc);
 }
 
-extern spinlock_t timerlist_lock;
 
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out 
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
arch/mips64/sgi-ip27/ip27-timer.c +2 -0
diff -urN linux-2.5.12-base/arch/mips64/sgi-ip27/ip27-timer.c linux-2.5.12-smptimers/arch/mips64/sgi-ip27/ip27-timer.c
@@ -11,6 +11,7 @@
 #include <linux/param.h>
 #include <linux/timex.h>
 #include <linux/mm.h>		
+#include <linux/timer.h>
 
 #include <asm/pgtable.h>
 #include <asm/sgialib.h>
@@ -123,6 +124,7 @@
 		irq_exit(cpu, 0);
 	}
 #endif /* CONFIG_SMP */
+	run_local_timers();
 	
 	/*
 	 * If we have an externally synchronized Linux clock, then update
arch/ppc/kernel/time.c +2 -0
diff -urN linux-2.5.12-base/arch/ppc/kernel/time.c linux-2.5.12-smptimers/arch/ppc/kernel/time.c
@@ -59,6 +59,7 @@
 #include <linux/mc146818rtc.h>
 #include <linux/time.h>
 #include <linux/init.h>
+#include <linux/timer.h>
 
 #include <asm/segment.h>
 #include <asm/io.h>
@@ -213,6 +214,7 @@
 
 	hardirq_exit(cpu);
 
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 
arch/ppc64/kernel/time.c +2 -0
diff -urN linux-2.5.12-base/arch/ppc64/kernel/time.c linux-2.5.12-smptimers/arch/ppc64/kernel/time.c
@@ -46,6 +46,7 @@
 #include <linux/mc146818rtc.h>
 #include <linux/time.h>
 #include <linux/init.h>
+#include <linux/timer.h>
 
 #include <asm/segment.h>
 #include <asm/io.h>
@@ -292,6 +293,7 @@
 
 	irq_exit(cpu);
 
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 	
arch/s390/kernel/time.c +2 -0
diff -urN linux-2.5.12-base/arch/s390/kernel/time.c linux-2.5.12-smptimers/arch/s390/kernel/time.c
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/types.h>
+#include <linux/timer.h>
 
 #include <asm/uaccess.h>
 #include <asm/delay.h>
@@ -196,6 +197,7 @@
 	}
 
 	irq_exit(cpu, 0);
+	run_local_timers();
 }
 
 /*
arch/s390/mm/fault.c +1 -4
diff -urN linux-2.5.12-base/arch/s390/mm/fault.c linux-2.5.12-smptimers/arch/s390/mm/fault.c
@@ -38,16 +38,13 @@
 extern void die(const char *,struct pt_regs *,long);
 static void force_sigsegv(struct task_struct *tsk, int code, void *address);
 
-extern spinlock_t timerlist_lock;
 
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out 
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
arch/s390x/kernel/time.c +2 -0
diff -urN linux-2.5.12-base/arch/s390x/kernel/time.c linux-2.5.12-smptimers/arch/s390x/kernel/time.c
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/types.h>
+#include <linux/timer.h>
 
 #include <asm/uaccess.h>
 #include <asm/delay.h>
@@ -202,6 +203,7 @@
 	}
 
 	irq_exit(cpu, 0);
+	run_local_timers();
 }
 
 /*
arch/s390x/mm/fault.c +1 -4
diff -urN linux-2.5.12-base/arch/s390x/mm/fault.c linux-2.5.12-smptimers/arch/s390x/mm/fault.c
@@ -37,16 +37,13 @@
 extern void die(const char *,struct pt_regs *,long);
 static void force_sigsegv(struct task_struct *tsk, int code, void *address);
 
-extern spinlock_t timerlist_lock;
 
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out 
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
arch/sparc/kernel/irq.c +1 -1
diff -urN linux-2.5.12-base/arch/sparc/kernel/irq.c linux-2.5.12-smptimers/arch/sparc/kernel/irq.c
@@ -73,7 +73,7 @@
     prom_halt();
 }
 
-void (*init_timers)(void (*)(int, void *,struct pt_regs *)) =
+void (*sparc_init_timers)(void (*)(int, void *,struct pt_regs *)) =
     (void (*)(void (*)(int, void *,struct pt_regs *))) irq_panic;
 
 /*
arch/sparc/kernel/sun4c_irq.c +2 -2
diff -urN linux-2.5.12-base/arch/sparc/kernel/sun4c_irq.c linux-2.5.12-smptimers/arch/sparc/kernel/sun4c_irq.c
@@ -143,7 +143,7 @@
 	/* Errm.. not sure how to do this.. */
 }
 
-static void __init sun4c_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
+static void __init sun4c_sparc_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
 {
 	int irq;
 
@@ -221,7 +221,7 @@
 	BTFIXUPSET_CALL(clear_profile_irq, sun4c_clear_profile_irq, BTFIXUPCALL_NOP);
 	BTFIXUPSET_CALL(load_profile_irq, sun4c_load_profile_irq, BTFIXUPCALL_NOP);
 	BTFIXUPSET_CALL(__irq_itoa, sun4m_irq_itoa, BTFIXUPCALL_NORM);
-	init_timers = sun4c_init_timers;
+	sparc_init_timers = sun4c_sparc_init_timers;
 #ifdef CONFIG_SMP
 	BTFIXUPSET_CALL(set_cpu_int, sun4c_nop, BTFIXUPCALL_NOP);
 	BTFIXUPSET_CALL(clear_cpu_int, sun4c_nop, BTFIXUPCALL_NOP);
arch/sparc/kernel/sun4d_irq.c +2 -2
diff -urN linux-2.5.12-base/arch/sparc/kernel/sun4d_irq.c linux-2.5.12-smptimers/arch/sparc/kernel/sun4d_irq.c
@@ -436,7 +436,7 @@
 	bw_set_prof_limit(cpu, limit);
 }
 
-static void __init sun4d_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
+static void __init sun4d_sparc_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
 {
 	int irq;
 	extern struct prom_cpuinfo linux_cpus[NR_CPUS];
@@ -547,7 +547,7 @@
 	BTFIXUPSET_CALL(clear_profile_irq, sun4d_clear_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(load_profile_irq, sun4d_load_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(__irq_itoa, sun4d_irq_itoa, BTFIXUPCALL_NORM);
-	init_timers = sun4d_init_timers;
+	sparc_init_timers = sun4d_sparc_init_timers;
 #ifdef CONFIG_SMP
 	BTFIXUPSET_CALL(set_cpu_int, sun4d_set_cpu_int, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(clear_cpu_int, sun4d_clear_ipi, BTFIXUPCALL_NOP);
arch/sparc/kernel/sun4d_smp.c +2 -0
diff -urN linux-2.5.12-base/arch/sparc/kernel/sun4d_smp.c linux-2.5.12-smptimers/arch/sparc/kernel/sun4d_smp.c
@@ -18,6 +18,7 @@
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/mm.h>
+#include <linux/timer.h>
 
 #include <asm/ptrace.h>
 #include <asm/atomic.h>
@@ -467,6 +468,7 @@
 
 		prof_counter[cpu] = prof_multiplier[cpu];
 	}
+	run_local_timers();
 }
 
 extern unsigned int lvl14_resolution;
arch/sparc/kernel/sun4m_irq.c +2 -2
diff -urN linux-2.5.12-base/arch/sparc/kernel/sun4m_irq.c linux-2.5.12-smptimers/arch/sparc/kernel/sun4m_irq.c
@@ -223,7 +223,7 @@
 	return buff;
 }
 
-static void __init sun4m_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
+static void __init sun4m_sparc_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
 {
 	int reg_count, irq, cpu;
 	struct linux_prom_registers cnt_regs[PROMREG_MAX];
@@ -374,7 +374,7 @@
 	BTFIXUPSET_CALL(clear_profile_irq, sun4m_clear_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(load_profile_irq, sun4m_load_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(__irq_itoa, sun4m_irq_itoa, BTFIXUPCALL_NORM);
-	init_timers = sun4m_init_timers;
+	sparc_init_timers = sun4m_sparc_init_timers;
 #ifdef CONFIG_SMP
 	BTFIXUPSET_CALL(set_cpu_int, sun4m_send_ipi, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(clear_cpu_int, sun4m_clear_ipi, BTFIXUPCALL_NORM);
arch/sparc/kernel/sun4m_smp.c +2 -0
diff -urN linux-2.5.12-base/arch/sparc/kernel/sun4m_smp.c linux-2.5.12-smptimers/arch/sparc/kernel/sun4m_smp.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/mm.h>
+#include <linux/timer.h>
 
 #include <asm/ptrace.h>
 #include <asm/atomic.h>
@@ -454,6 +455,7 @@
 
 		prof_counter[cpu] = prof_multiplier[cpu];
 	}
+	run_local_timers();
 }
 
 extern unsigned int lvl14_resolution;
arch/sparc/kernel/time.c +1 -1
diff -urN linux-2.5.12-base/arch/sparc/kernel/time.c linux-2.5.12-smptimers/arch/sparc/kernel/time.c
@@ -379,7 +379,7 @@
 	else
 		clock_probe();
 
-	init_timers(timer_interrupt);
+	sparc_init_timers(timer_interrupt);
 	
 #ifdef CONFIG_SUN4
 	if(idprom->id_machtype == (SM_SUN4 | SM_4_330)) {
arch/sparc64/kernel/irq.c +1 -1
diff -urN linux-2.5.12-base/arch/sparc64/kernel/irq.c linux-2.5.12-smptimers/arch/sparc64/kernel/irq.c
@@ -1027,7 +1027,7 @@
 }
 
 /* This is gets the master TICK_INT timer going. */
-void init_timers(void (*cfunc)(int, void *, struct pt_regs *),
+void sparc_init_timers(void (*cfunc)(int, void *, struct pt_regs *),
 		 unsigned long *clock)
 {
 	unsigned long pstate;
arch/sparc64/kernel/smp.c +1 -0
diff -urN linux-2.5.12-base/arch/sparc64/kernel/smp.c linux-2.5.12-smptimers/arch/sparc64/kernel/smp.c
@@ -1027,6 +1027,7 @@
 
 			prof_counter(cpu) = prof_multiplier(cpu);
 		}
+		run_local_timers();
 
 		/* Guarentee that the following sequences execute
 		 * uninterrupted.
arch/sparc64/kernel/time.c +2 -2
diff -urN linux-2.5.12-base/arch/sparc64/kernel/time.c linux-2.5.12-smptimers/arch/sparc64/kernel/time.c
@@ -611,7 +611,7 @@
 	__restore_flags(flags);
 }
 
-extern void init_timers(void (*func)(int, void *, struct pt_regs *),
+extern void sparc_init_timers(void (*func)(int, void *, struct pt_regs *),
 			unsigned long *);
 
 void __init time_init(void)
@@ -622,7 +622,7 @@
 	 */
 	unsigned long clock;
 
-	init_timers(timer_interrupt, &clock);
+	sparc_init_timers(timer_interrupt, &clock);
 	timer_ticks_per_usec_quotient = ((1UL<<32) / (clock / 1000020));
 }
 
arch/x86_64/kernel/apic.c +2 -0
diff -urN linux-2.5.12-base/arch/x86_64/kernel/apic.c linux-2.5.12-smptimers/arch/x86_64/kernel/apic.c
@@ -23,6 +23,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <linux/kernel_stat.h>
+#include <linux/timer.h>
 
 #include <asm/atomic.h>
 #include <asm/smp.h>
@@ -1069,6 +1070,7 @@
 	smp_local_timer_interrupt(regs);
 	irq_exit(cpu, 0);
 
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 }
arch/x86_64/mm/fault.c +1 -2
diff -urN linux-2.5.12-base/arch/x86_64/mm/fault.c linux-2.5.12-smptimers/arch/x86_64/mm/fault.c
@@ -32,11 +32,10 @@
 
 extern void die(const char *,struct pt_regs *,long);
 
-extern spinlock_t console_lock, timerlist_lock;
+extern spinlock_t console_lock;
 
 void bust_spinlocks(int yes)
 {
- 	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 #ifdef CONFIG_SMP
drivers/net/eepro100.c +0 -3
diff -urN linux-2.5.12-base/drivers/net/eepro100.c linux-2.5.12-smptimers/drivers/net/eepro100.c
@@ -1171,9 +1171,6 @@
 	/* We must continue to monitor the media. */
 	sp->timer.expires = RUN_AT(2*HZ); 			/* 2.0 sec. */
 	add_timer(&sp->timer);
-#if defined(timer_exit)
-	timer_exit(&sp->timer);
-#endif
 }
 
 static void speedo_show_state(struct net_device *dev)
include/asm-sparc/irq.h +1 -1
diff -urN linux-2.5.12-base/include/asm-sparc/irq.h linux-2.5.12-smptimers/include/asm-sparc/irq.h
@@ -45,7 +45,7 @@
 #define clear_profile_irq(cpu) BTFIXUP_CALL(clear_profile_irq)(cpu)
 #define load_profile_irq(cpu,limit) BTFIXUP_CALL(load_profile_irq)(cpu,limit)
 
-extern void (*init_timers)(void (*lvl10_irq)(int, void *, struct pt_regs *));
+extern void (*sparc_init_timers)(void (*lvl10_irq)(int, void *, struct pt_regs *));
 extern void claim_ticker14(void (*irq_handler)(int, void *, struct pt_regs *),
 			   int irq,
 			   unsigned int timeout);
include/asm-sparc64/irq.h +1 -1
diff -urN linux-2.5.12-base/include/asm-sparc64/irq.h linux-2.5.12-smptimers/include/asm-sparc64/irq.h
@@ -114,7 +114,7 @@
 extern void disable_irq(unsigned int);
 #define disable_irq_nosync disable_irq
 extern void enable_irq(unsigned int);
-extern void init_timers(void (*lvl10_irq)(int, void *, struct pt_regs *),
+extern void sparc_init_timers(void (*lvl10_irq)(int, void *, struct pt_regs *),
 			unsigned long *);
 extern unsigned int build_irq(int pil, int inofixup, unsigned long iclr, unsigned long imap);
 extern unsigned int sbus_build_irq(void *sbus, unsigned int ino);
include/linux/interrupt.h +1 -0
diff -urN linux-2.5.12-base/include/linux/interrupt.h linux-2.5.12-smptimers/include/linux/interrupt.h
@@ -225,6 +225,7 @@
 
 /* It is exported _ONLY_ for wait_on_irq(). */
 extern spinlock_t global_bh_lock;
+extern spinlock_t net_bh_lock;
 
 static inline void mark_bh(int nr)
 {
include/linux/smp.h +2 -1
diff -urN linux-2.5.12-base/include/linux/smp.h linux-2.5.12-smptimers/include/linux/smp.h
@@ -78,7 +78,8 @@
 /*
  *	These macros fold the SMP functionality into a single CPU system
  */
- 
+
+#define NR_CPUS					1 
 #define smp_num_cpus				1
 #define smp_processor_id()			0
 #define hard_smp_processor_id()			0
include/linux/timer.h +85 -9

133 lines not on this page · raw message

kernel/ksyms.c +1 -0
diff -urN linux-2.5.12-base/kernel/ksyms.c linux-2.5.12-smptimers/kernel/ksyms.c
@@ -402,6 +402,7 @@
 EXPORT_SYMBOL(del_timer_sync);
 #endif
 EXPORT_SYMBOL(mod_timer);
+EXPORT_SYMBOL(tvec_bases);
 EXPORT_SYMBOL(tq_timer);
 EXPORT_SYMBOL(tq_immediate);
 
kernel/sched.c +2 -3
diff -urN linux-2.5.12-base/kernel/sched.c linux-2.5.12-smptimers/kernel/sched.c
@@ -1573,7 +1573,7 @@
 	idle->thread_info->preempt_count = (idle->lock_depth >= 0);
 }
 
-extern void init_timervecs(void);
+extern void init_timers(void);
 extern void timer_bh(void);
 extern void tqueue_bh(void);
 extern void immediate_bh(void);
@@ -1612,8 +1612,7 @@
 	rq->idle = current;
 	wake_up_process(current);
 
-	init_timervecs();
-	init_bh(TIMER_BH, timer_bh);
+	init_timers();
 	init_bh(TQUEUE_BH, tqueue_bh);
 	init_bh(IMMEDIATE_BH, immediate_bh);
 
kernel/softirq.c +6 -0
diff -urN linux-2.5.12-base/kernel/softirq.c linux-2.5.12-smptimers/kernel/softirq.c
@@ -43,6 +43,12 @@
 irq_cpustat_t irq_stat[NR_CPUS];
 
 static struct softirq_action softirq_vec[32] __cacheline_aligned_in_smp;
+/*
+ * This is required only to support single threaded network protocol
+ * code. (see net/core/dev.c) This should GO AWAY when those protocols
+ * are fixed.
+ */
+spinlock_t net_bh_lock = SPIN_LOCK_UNLOCKED;
 
 /*
  * we cannot loop indefinitely here to avoid userspace starvation,
kernel/timer.c +258 -129

557 lines not on this page · raw message

lib/bust_spinlocks.c +0 -2
diff -urN linux-2.5.12-base/lib/bust_spinlocks.c linux-2.5.12-smptimers/lib/bust_spinlocks.c
@@ -14,11 +14,9 @@
 #include <linux/wait.h>
 #include <linux/vt_kern.h>
 
-extern spinlock_t timerlist_lock;
 
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
net/core/dev.c +0 -7
diff -urN linux-2.5.12-base/net/core/dev.c linux-2.5.12-smptimers/net/core/dev.c
@@ -1291,7 +1291,6 @@
  */
 static int deliver_to_old_ones(struct packet_type *pt, struct sk_buff *skb, int last)
 {
-	static spinlock_t net_bh_lock = SPIN_LOCK_UNLOCKED;
 	int ret = NET_RX_DROP;
 
 
@@ -1311,13 +1310,7 @@
 
 	/* Emulate NET_BH with special spinlock */
 	spin_lock(&net_bh_lock);
-
-	/* Disable timers and wait for all timers completion */
-	tasklet_disable(bh_task_vec+TIMER_BH);
-
 	ret = pt->func(skb, skb->dev, pt);
-
-	tasklet_hi_enable(bh_task_vec+TIMER_BH);
 	spin_unlock(&net_bh_lock);
 	return ret;
 }
#1 lore raw

Hi Dipankar,

Quoted 3 lines

I have been experimenting with Ingo's smptimers and I ended up extending it a little bit. I would really appreciate comments on whether these things make sense or not.

I tried it out and found that we were context switching like crazy. It seems we were always running the timers out of a tasklet because we never unlocked the net_bh_lock.

Anton

kernel/timer.c +2 -0
diff -urN linux-2.5_smptimers/kernel/timer.c linux-2.5_tux/kernel/timer.c
@@ -694,6 +694,7 @@
 		__run_timers(base);
 
         hardirq_endlock(cpu);
+        spin_unlock(&net_bh_lock);
         spin_unlock(&global_bh_lock);
         return;
 resched_net:
@@ -733,6 +734,7 @@
 		__run_timers(base);
 
 	hardirq_endlock(cpu);
+	spin_unlock(&net_bh_lock);
 	spin_unlock(&global_bh_lock);
 	local_irq_enable();
 	local_bh_enable();
#2 lore raw
In reply to Dipankar Sarma
Quoted 10 lines · Anton Blanchard

Hi Dipankar,

Quoted 3 lines

I have been experimenting with Ingo's smptimers and I ended up extending it a little bit. I would really appreciate comments on whether these things make sense or not.

I tried it out and found that we were context switching like crazy. It seems we were always running the timers out of a tasklet because we never unlocked the net_bh_lock.

The tasklet code also needs fixing. It is a miracle that the kernel booted when I tested that code. Here is a fixed diff.

I am curious about performance of smptimers. It seems that webserver benchmark performance worsens with smptimers (Ingo version) contrary to our expectations. Do you see this ? If so, could this happen because -

1) Bouncing around of global_bh_lock cacheline by more cpus compared to earlier timer implemenation ?
2) All per-cpu timers invoked from timer_bh running in one cpu ?

Do you see any other side-effects of smptimers ?

Also, did my PPC changes for smptimers work or you had to fix it ?

Thanks

Signature · 51 lines
Dipankar Sarma  <[email protected]> http://lse.sourceforge.net
Linux Technology Center, IBM Software Lab, Bangalore, India.

--- linux-2.5.12-smptimers/kernel/timer.c	Tue May 14 13:21:26 2002
+++ linux-2.5.14-smptimers/kernel/timer.c	Mon May 20 15:46:29 2002
@@ -680,19 +680,14 @@
                 goto resched;
 
         if (!spin_trylock(&net_bh_lock))
-                goto resched_net;
-
-        if (!hardirq_trylock(cpu))
                 goto resched_unlock;
 
 	if ((long)(jiffies - base->timer_jiffies) >= 0)
 		__run_timers(base);
 
-        hardirq_endlock(cpu);
+        spin_unlock(&net_bh_lock);
         spin_unlock(&global_bh_lock);
         return;
-resched_net:
-        spin_unlock(&net_bh_lock);
 resched_unlock:
         spin_unlock(&global_bh_lock);
 resched:
@@ -719,20 +714,21 @@
 	if (!spin_trylock(&global_bh_lock))
 		goto out_enable_mark;
         if (!spin_trylock(&net_bh_lock))
-                goto out_unlock_net;
+                goto out_unlock_enable_mark;
 
 	if (!hardirq_trylock(cpu))
-		goto out_unlock_enable_mark;
+		goto out_unlock_enable_mark_net;
 
 	if ((long)(jiffies - base->timer_jiffies) >= 0)
 		__run_timers(base);
 
 	hardirq_endlock(cpu);
+	spin_unlock(&net_bh_lock);
 	spin_unlock(&global_bh_lock);
 	local_irq_enable();
 	local_bh_enable();
 	return;
-out_unlock_net:
+out_unlock_enable_mark_net:
 	spin_unlock(&net_bh_lock);
 out_unlock_enable_mark:
 	spin_unlock(&global_bh_lock);
#3 lore raw
In reply to Dipankar Sarma
Quoted 10 lines · Anton Blanchard

Hi Dipankar,

Quoted 3 lines

I have been experimenting with Ingo's smptimers and I ended up extending it a little bit. I would really appreciate comments on whether these things make sense or not.

I tried it out and found that we were context switching like crazy. It seems we were always running the timers out of a tasklet because we never unlocked the net_bh_lock.

Ok, here is the fixed smptimers patch (with the changes I mentioned in the earlier mail). Hopefully the goto lock unwinding logic in run_local_timer() and run_timer_tasklet() are not messed up this time around.

We need to change bust_spinlocks() to bust that CPUs timer base lock instead, though.

Thanks

Signature · 2 lines
Dipankar Sarma  <[email protected]> http://lse.sourceforge.net
Linux Technology Center, IBM Software Lab, Bangalore, India.
arch/alpha/kernel/smp.c +2 -0
diff -urN linux-2.5.14-base/arch/alpha/kernel/smp.c linux-2.5.14-smptimers/arch/alpha/kernel/smp.c
@@ -24,6 +24,7 @@
 #include <linux/spinlock.h>
 #include <linux/irq.h>
 #include <linux/cache.h>
+#include <linux/timer.h>
 
 #include <asm/hwrpb.h>
 #include <asm/ptrace.h>
@@ -657,6 +658,7 @@
 		if (softirq_pending(cpu))
 			do_softirq();
 	}
+	run_local_timers();
 }
 
 int __init
arch/i386/kernel/apic.c +2 -1
diff -urN linux-2.5.14-base/arch/i386/kernel/apic.c linux-2.5.14-smptimers/arch/i386/kernel/apic.c
@@ -23,6 +23,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <linux/kernel_stat.h>
+#include <linux/timer.h>
 
 #include <asm/atomic.h>
 #include <asm/smp.h>
@@ -1087,7 +1088,7 @@
 	irq_enter(cpu, 0);
 	smp_local_timer_interrupt(&regs);
 	irq_exit(cpu, 0);
-
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 }
arch/i386/mm/fault.c +1 -5
diff -urN linux-2.5.14-base/arch/i386/mm/fault.c linux-2.5.14-smptimers/arch/i386/mm/fault.c
@@ -94,16 +94,12 @@
 	goto bad_area;
 }
 
-extern spinlock_t timerlist_lock;
-
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 #ifdef CONFIG_SMP
arch/ia64/kernel/smp.c +2 -0
diff -urN linux-2.5.14-base/arch/ia64/kernel/smp.c linux-2.5.14-smptimers/arch/ia64/kernel/smp.c
@@ -32,6 +32,7 @@
 #include <linux/cache.h>
 #include <linux/delay.h>
 #include <linux/cache.h>
+#include <linux/timer.h>
 
 #include <asm/atomic.h>
 #include <asm/bitops.h>
@@ -330,6 +331,7 @@
 		local_cpu_data->prof_counter = local_cpu_data->prof_multiplier;
 		update_process_times(user);
 	}
+	run_local_timers();
 }
 
 /*
arch/ia64/kernel/traps.c +1 -3
diff -urN linux-2.5.14-base/arch/ia64/kernel/traps.c linux-2.5.14-smptimers/arch/ia64/kernel/traps.c
@@ -42,7 +42,6 @@
 
 #include <asm/fpswa.h>
 
-extern spinlock_t timerlist_lock;
 
 static fpswa_interface_t *fpswa_interface;
 
@@ -56,13 +55,12 @@
 }
 
 /*
- * Unlock any spinlocks which will prevent us from getting the message out (timerlist_lock
+ * Unlock any spinlocks which will prevent us from getting the message out 
  * is acquired through the console unblank code)
  */
 void
 bust_spinlocks (int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 #ifdef CONFIG_SMP
arch/mips64/mm/fault.c +1 -4
diff -urN linux-2.5.14-base/arch/mips64/mm/fault.c linux-2.5.14-smptimers/arch/mips64/mm/fault.c
@@ -58,16 +58,13 @@
 	printk("Got exception 0x%lx at 0x%lx\n", retaddr, regs.cp0_epc);
 }
 
-extern spinlock_t timerlist_lock;
 
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out 
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
arch/mips64/sgi-ip27/ip27-timer.c +2 -0
diff -urN linux-2.5.14-base/arch/mips64/sgi-ip27/ip27-timer.c linux-2.5.14-smptimers/arch/mips64/sgi-ip27/ip27-timer.c
@@ -11,6 +11,7 @@
 #include <linux/param.h>
 #include <linux/timex.h>
 #include <linux/mm.h>		
+#include <linux/timer.h>
 
 #include <asm/pgtable.h>
 #include <asm/sgialib.h>
@@ -123,6 +124,7 @@
 		irq_exit(cpu, 0);
 	}
 #endif /* CONFIG_SMP */
+	run_local_timers();
 	
 	/*
 	 * If we have an externally synchronized Linux clock, then update
arch/ppc/kernel/time.c +2 -0
diff -urN linux-2.5.14-base/arch/ppc/kernel/time.c linux-2.5.14-smptimers/arch/ppc/kernel/time.c
@@ -59,6 +59,7 @@
 #include <linux/mc146818rtc.h>
 #include <linux/time.h>
 #include <linux/init.h>
+#include <linux/timer.h>
 
 #include <asm/segment.h>
 #include <asm/io.h>
@@ -213,6 +214,7 @@
 
 	hardirq_exit(cpu);
 
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 
arch/ppc64/kernel/time.c +2 -0
diff -urN linux-2.5.14-base/arch/ppc64/kernel/time.c linux-2.5.14-smptimers/arch/ppc64/kernel/time.c
@@ -46,6 +46,7 @@
 #include <linux/mc146818rtc.h>
 #include <linux/time.h>
 #include <linux/init.h>
+#include <linux/timer.h>
 
 #include <asm/segment.h>
 #include <asm/io.h>
@@ -292,6 +293,7 @@
 
 	irq_exit(cpu);
 
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 	
arch/s390/kernel/time.c +2 -0
diff -urN linux-2.5.14-base/arch/s390/kernel/time.c linux-2.5.14-smptimers/arch/s390/kernel/time.c
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/types.h>
+#include <linux/timer.h>
 
 #include <asm/uaccess.h>
 #include <asm/delay.h>
@@ -196,6 +197,7 @@
 	}
 
 	irq_exit(cpu, 0);
+	run_local_timers();
 }
 
 /*
arch/s390/mm/fault.c +1 -4
diff -urN linux-2.5.14-base/arch/s390/mm/fault.c linux-2.5.14-smptimers/arch/s390/mm/fault.c
@@ -38,16 +38,13 @@
 extern void die(const char *,struct pt_regs *,long);
 static void force_sigsegv(struct task_struct *tsk, int code, void *address);
 
-extern spinlock_t timerlist_lock;
 
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out 
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
arch/s390x/kernel/time.c +2 -0
diff -urN linux-2.5.14-base/arch/s390x/kernel/time.c linux-2.5.14-smptimers/arch/s390x/kernel/time.c
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/types.h>
+#include <linux/timer.h>
 
 #include <asm/uaccess.h>
 #include <asm/delay.h>
@@ -202,6 +203,7 @@
 	}
 
 	irq_exit(cpu, 0);
+	run_local_timers();
 }
 
 /*
arch/s390x/mm/fault.c +1 -4
diff -urN linux-2.5.14-base/arch/s390x/mm/fault.c linux-2.5.14-smptimers/arch/s390x/mm/fault.c
@@ -37,16 +37,13 @@
 extern void die(const char *,struct pt_regs *,long);
 static void force_sigsegv(struct task_struct *tsk, int code, void *address);
 
-extern spinlock_t timerlist_lock;
 
 /*
  * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is acquired through the
- * console unblank code)
+ * message out 
  */
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
arch/sparc/kernel/irq.c +1 -1
diff -urN linux-2.5.14-base/arch/sparc/kernel/irq.c linux-2.5.14-smptimers/arch/sparc/kernel/irq.c
@@ -73,7 +73,7 @@
     prom_halt();
 }
 
-void (*init_timers)(void (*)(int, void *,struct pt_regs *)) =
+void (*sparc_init_timers)(void (*)(int, void *,struct pt_regs *)) =
     (void (*)(void (*)(int, void *,struct pt_regs *))) irq_panic;
 
 /*
arch/sparc/kernel/sun4c_irq.c +2 -2
diff -urN linux-2.5.14-base/arch/sparc/kernel/sun4c_irq.c linux-2.5.14-smptimers/arch/sparc/kernel/sun4c_irq.c
@@ -143,7 +143,7 @@
 	/* Errm.. not sure how to do this.. */
 }
 
-static void __init sun4c_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
+static void __init sun4c_sparc_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
 {
 	int irq;
 
@@ -221,7 +221,7 @@
 	BTFIXUPSET_CALL(clear_profile_irq, sun4c_clear_profile_irq, BTFIXUPCALL_NOP);
 	BTFIXUPSET_CALL(load_profile_irq, sun4c_load_profile_irq, BTFIXUPCALL_NOP);
 	BTFIXUPSET_CALL(__irq_itoa, sun4m_irq_itoa, BTFIXUPCALL_NORM);
-	init_timers = sun4c_init_timers;
+	sparc_init_timers = sun4c_sparc_init_timers;
 #ifdef CONFIG_SMP
 	BTFIXUPSET_CALL(set_cpu_int, sun4c_nop, BTFIXUPCALL_NOP);
 	BTFIXUPSET_CALL(clear_cpu_int, sun4c_nop, BTFIXUPCALL_NOP);
arch/sparc/kernel/sun4d_irq.c +2 -2
diff -urN linux-2.5.14-base/arch/sparc/kernel/sun4d_irq.c linux-2.5.14-smptimers/arch/sparc/kernel/sun4d_irq.c
@@ -436,7 +436,7 @@
 	bw_set_prof_limit(cpu, limit);
 }
 
-static void __init sun4d_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
+static void __init sun4d_sparc_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
 {
 	int irq;
 	extern struct prom_cpuinfo linux_cpus[NR_CPUS];
@@ -547,7 +547,7 @@
 	BTFIXUPSET_CALL(clear_profile_irq, sun4d_clear_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(load_profile_irq, sun4d_load_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(__irq_itoa, sun4d_irq_itoa, BTFIXUPCALL_NORM);
-	init_timers = sun4d_init_timers;
+	sparc_init_timers = sun4d_sparc_init_timers;
 #ifdef CONFIG_SMP
 	BTFIXUPSET_CALL(set_cpu_int, sun4d_set_cpu_int, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(clear_cpu_int, sun4d_clear_ipi, BTFIXUPCALL_NOP);
arch/sparc/kernel/sun4d_smp.c +2 -0
diff -urN linux-2.5.14-base/arch/sparc/kernel/sun4d_smp.c linux-2.5.14-smptimers/arch/sparc/kernel/sun4d_smp.c
@@ -18,6 +18,7 @@
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/mm.h>
+#include <linux/timer.h>
 
 #include <asm/ptrace.h>
 #include <asm/atomic.h>
@@ -465,6 +466,7 @@
 
 		prof_counter[cpu] = prof_multiplier[cpu];
 	}
+	run_local_timers();
 }
 
 extern unsigned int lvl14_resolution;
arch/sparc/kernel/sun4m_irq.c +2 -2
diff -urN linux-2.5.14-base/arch/sparc/kernel/sun4m_irq.c linux-2.5.14-smptimers/arch/sparc/kernel/sun4m_irq.c
@@ -223,7 +223,7 @@
 	return buff;
 }
 
-static void __init sun4m_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
+static void __init sun4m_sparc_init_timers(void (*counter_fn)(int, void *, struct pt_regs *))
 {
 	int reg_count, irq, cpu;
 	struct linux_prom_registers cnt_regs[PROMREG_MAX];
@@ -374,7 +374,7 @@
 	BTFIXUPSET_CALL(clear_profile_irq, sun4m_clear_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(load_profile_irq, sun4m_load_profile_irq, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(__irq_itoa, sun4m_irq_itoa, BTFIXUPCALL_NORM);
-	init_timers = sun4m_init_timers;
+	sparc_init_timers = sun4m_sparc_init_timers;
 #ifdef CONFIG_SMP
 	BTFIXUPSET_CALL(set_cpu_int, sun4m_send_ipi, BTFIXUPCALL_NORM);
 	BTFIXUPSET_CALL(clear_cpu_int, sun4m_clear_ipi, BTFIXUPCALL_NORM);
arch/sparc/kernel/sun4m_smp.c +2 -0
diff -urN linux-2.5.14-base/arch/sparc/kernel/sun4m_smp.c linux-2.5.14-smptimers/arch/sparc/kernel/sun4m_smp.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/mm.h>
+#include <linux/timer.h>
 
 #include <asm/ptrace.h>
 #include <asm/atomic.h>
@@ -452,6 +453,7 @@
 
 		prof_counter[cpu] = prof_multiplier[cpu];
 	}
+	run_local_timers();
 }
 
 extern unsigned int lvl14_resolution;
arch/sparc/kernel/time.c +1 -1
diff -urN linux-2.5.14-base/arch/sparc/kernel/time.c linux-2.5.14-smptimers/arch/sparc/kernel/time.c
@@ -379,7 +379,7 @@
 	else
 		clock_probe();
 
-	init_timers(timer_interrupt);
+	sparc_init_timers(timer_interrupt);
 	
 #ifdef CONFIG_SUN4
 	if(idprom->id_machtype == (SM_SUN4 | SM_4_330)) {
arch/sparc64/kernel/irq.c +1 -1
diff -urN linux-2.5.14-base/arch/sparc64/kernel/irq.c linux-2.5.14-smptimers/arch/sparc64/kernel/irq.c
@@ -1027,7 +1027,7 @@
 }
 
 /* This is gets the master TICK_INT timer going. */
-void init_timers(void (*cfunc)(int, void *, struct pt_regs *),
+void sparc_init_timers(void (*cfunc)(int, void *, struct pt_regs *),
 		 unsigned long *clock)
 {
 	unsigned long pstate;
arch/sparc64/kernel/smp.c +1 -0
diff -urN linux-2.5.14-base/arch/sparc64/kernel/smp.c linux-2.5.14-smptimers/arch/sparc64/kernel/smp.c
@@ -1041,6 +1041,7 @@
 
 			prof_counter(cpu) = prof_multiplier(cpu);
 		}
+		run_local_timers();
 
 		/* Guarentee that the following sequences execute
 		 * uninterrupted.
arch/sparc64/kernel/time.c +2 -2
diff -urN linux-2.5.14-base/arch/sparc64/kernel/time.c linux-2.5.14-smptimers/arch/sparc64/kernel/time.c
@@ -611,7 +611,7 @@
 	__restore_flags(flags);
 }
 
-extern void init_timers(void (*func)(int, void *, struct pt_regs *),
+extern void sparc_init_timers(void (*func)(int, void *, struct pt_regs *),
 			unsigned long *);
 
 void __init time_init(void)
@@ -622,7 +622,7 @@
 	 */
 	unsigned long clock;
 
-	init_timers(timer_interrupt, &clock);
+	sparc_init_timers(timer_interrupt, &clock);
 	timer_ticks_per_usec_quotient = ((1UL<<32) / (clock / 1000020));
 }
 
arch/x86_64/kernel/apic.c +2 -0
diff -urN linux-2.5.14-base/arch/x86_64/kernel/apic.c linux-2.5.14-smptimers/arch/x86_64/kernel/apic.c
@@ -23,6 +23,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <linux/kernel_stat.h>
+#include <linux/timer.h>
 
 #include <asm/atomic.h>
 #include <asm/smp.h>
@@ -1069,6 +1070,7 @@
 	smp_local_timer_interrupt(regs);
 	irq_exit(cpu, 0);
 
+	run_local_timers();
 	if (softirq_pending(cpu))
 		do_softirq();
 }
arch/x86_64/mm/fault.c +1 -2
diff -urN linux-2.5.14-base/arch/x86_64/mm/fault.c linux-2.5.14-smptimers/arch/x86_64/mm/fault.c
@@ -32,11 +32,10 @@
 
 extern void die(const char *,struct pt_regs *,long);
 
-extern spinlock_t console_lock, timerlist_lock;
+extern spinlock_t console_lock;
 
 void bust_spinlocks(int yes)
 {
- 	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 #ifdef CONFIG_SMP
drivers/net/eepro100.c +0 -3
diff -urN linux-2.5.14-base/drivers/net/eepro100.c linux-2.5.14-smptimers/drivers/net/eepro100.c
@@ -1171,9 +1171,6 @@
 	/* We must continue to monitor the media. */
 	sp->timer.expires = RUN_AT(2*HZ); 			/* 2.0 sec. */
 	add_timer(&sp->timer);
-#if defined(timer_exit)
-	timer_exit(&sp->timer);
-#endif
 }
 
 static void speedo_show_state(struct net_device *dev)
include/asm-sparc/irq.h +1 -1
diff -urN linux-2.5.14-base/include/asm-sparc/irq.h linux-2.5.14-smptimers/include/asm-sparc/irq.h
@@ -45,7 +45,7 @@
 #define clear_profile_irq(cpu) BTFIXUP_CALL(clear_profile_irq)(cpu)
 #define load_profile_irq(cpu,limit) BTFIXUP_CALL(load_profile_irq)(cpu,limit)
 
-extern void (*init_timers)(void (*lvl10_irq)(int, void *, struct pt_regs *));
+extern void (*sparc_init_timers)(void (*lvl10_irq)(int, void *, struct pt_regs *));
 extern void claim_ticker14(void (*irq_handler)(int, void *, struct pt_regs *),
 			   int irq,
 			   unsigned int timeout);
include/asm-sparc64/irq.h +1 -1
diff -urN linux-2.5.14-base/include/asm-sparc64/irq.h linux-2.5.14-smptimers/include/asm-sparc64/irq.h
@@ -114,7 +114,7 @@
 extern void disable_irq(unsigned int);
 #define disable_irq_nosync disable_irq
 extern void enable_irq(unsigned int);
-extern void init_timers(void (*lvl10_irq)(int, void *, struct pt_regs *),
+extern void sparc_init_timers(void (*lvl10_irq)(int, void *, struct pt_regs *),
 			unsigned long *);
 extern unsigned int build_irq(int pil, int inofixup, unsigned long iclr, unsigned long imap);
 extern unsigned int sbus_build_irq(void *sbus, unsigned int ino);
include/linux/interrupt.h +1 -0
diff -urN linux-2.5.14-base/include/linux/interrupt.h linux-2.5.14-smptimers/include/linux/interrupt.h
@@ -225,6 +225,7 @@
 
 /* It is exported _ONLY_ for wait_on_irq(). */
 extern spinlock_t global_bh_lock;
+extern spinlock_t net_bh_lock;
 
 static inline void mark_bh(int nr)
 {
include/linux/smp.h +2 -1
diff -urN linux-2.5.14-base/include/linux/smp.h linux-2.5.14-smptimers/include/linux/smp.h
@@ -78,7 +78,8 @@
 /*
  *	These macros fold the SMP functionality into a single CPU system
  */
- 
+
+#define NR_CPUS					1 
 #define smp_num_cpus				1
 #define smp_processor_id()			0
 #define hard_smp_processor_id()			0
include/linux/timer.h +85 -9

133 lines not on this page · raw message

kernel/ksyms.c +1 -0
diff -urN linux-2.5.14-base/kernel/ksyms.c linux-2.5.14-smptimers/kernel/ksyms.c
@@ -402,6 +402,7 @@
 EXPORT_SYMBOL(del_timer_sync);
 #endif
 EXPORT_SYMBOL(mod_timer);
+EXPORT_SYMBOL(tvec_bases);
 EXPORT_SYMBOL(tq_timer);
 EXPORT_SYMBOL(tq_immediate);
 
kernel/sched.c +2 -3
diff -urN linux-2.5.14-base/kernel/sched.c linux-2.5.14-smptimers/kernel/sched.c
@@ -1573,7 +1573,7 @@
 	idle->thread_info->preempt_count = (idle->lock_depth >= 0);
 }
 
-extern void init_timervecs(void);
+extern void init_timers(void);
 extern void timer_bh(void);
 extern void tqueue_bh(void);
 extern void immediate_bh(void);
@@ -1612,8 +1612,7 @@
 	rq->idle = current;
 	wake_up_process(current);
 
-	init_timervecs();
-	init_bh(TIMER_BH, timer_bh);
+	init_timers();
 	init_bh(TQUEUE_BH, tqueue_bh);
 	init_bh(IMMEDIATE_BH, immediate_bh);
 
kernel/softirq.c +6 -0
diff -urN linux-2.5.14-base/kernel/softirq.c linux-2.5.14-smptimers/kernel/softirq.c
@@ -43,6 +43,12 @@
 irq_cpustat_t irq_stat[NR_CPUS];
 
 static struct softirq_action softirq_vec[32] __cacheline_aligned_in_smp;
+/*
+ * This is required only to support single threaded network protocol
+ * code. (see net/core/dev.c) This should GO AWAY when those protocols
+ * are fixed.
+ */
+spinlock_t net_bh_lock = SPIN_LOCK_UNLOCKED;
 
 /*
  * we cannot loop indefinitely here to avoid userspace starvation,
kernel/timer.c +254 -129

553 lines not on this page · raw message

lib/bust_spinlocks.c +0 -2
diff -urN linux-2.5.14-base/lib/bust_spinlocks.c linux-2.5.14-smptimers/lib/bust_spinlocks.c
@@ -14,11 +14,9 @@
 #include <linux/wait.h>
 #include <linux/vt_kern.h>
 
-extern spinlock_t timerlist_lock;
 
 void bust_spinlocks(int yes)
 {
-	spin_lock_init(&timerlist_lock);
 	if (yes) {
 		oops_in_progress = 1;
 	} else {
net/core/dev.c +0 -7
diff -urN linux-2.5.14-base/net/core/dev.c linux-2.5.14-smptimers/net/core/dev.c
@@ -1291,7 +1291,6 @@
  */
 static int deliver_to_old_ones(struct packet_type *pt, struct sk_buff *skb, int last)
 {
-	static spinlock_t net_bh_lock = SPIN_LOCK_UNLOCKED;
 	int ret = NET_RX_DROP;
 
 
@@ -1311,13 +1310,7 @@
 
 	/* Emulate NET_BH with special spinlock */
 	spin_lock(&net_bh_lock);
-
-	/* Disable timers and wait for all timers completion */
-	tasklet_disable(bh_task_vec+TIMER_BH);
-
 	ret = pt->func(skb, skb->dev, pt);
-
-	tasklet_hi_enable(bh_task_vec+TIMER_BH);
 	spin_unlock(&net_bh_lock);
 	return ret;
 }
#4 lore raw
In reply to Dipankar Sarma

Hi Dipankar,

Quoted 2 lines

The tasklet code also needs fixing. It is a miracle that the kernel booted when I tested that code. Here is a fixed diff.

:) I was surprised it worked with the missing spin_unlock too. Im testing the fixed diff now, so far it looks good.

Quoted 10 lines

I am curious about performance of smptimers. It seems that webserver benchmark performance worsens with smptimers (Ingo version) contrary to our expectations. Do you see this ? If so, could this happen because -

1) Bouncing around of global_bh_lock cacheline by more cpus compared to earlier timer implemenation ?
2) All per-cpu timers invoked from timer_bh running in one cpu ?

Do you see any other side-effects of smptimers ?

We used to see bad behaviour. It turned out to be the per cpu timer interrupt firing at exactly the same time on all cpus. One cpu would successfully spin_trylock and the others would fail and postpone the work.

We now evenly space the per cpu interrupts. Does intel do the same?

Quoted 1 line

Also, did my PPC changes for smptimers work or you had to fix it ?

I tested ppc64 and it worked fine.

Anton

#5 lore raw
In reply to Dipankar Sarma
Quoted 11 lines · On 2002.05.20 Anton Blanchard wrote:

Hi Dipankar,

Quoted 3 lines

I have been experimenting with Ingo's smptimers and I ended up extending it a little bit. I would really appreciate comments on whether these things make sense or not.

I tried it out and found that we were context switching like crazy. It seems we were always running the timers out of a tasklet because we never unlocked the net_bh_lock.

The patch for 2.4 in

http://people.redhat.com/mingo/scalable-timers-patches/

does not acquire net_bh_lock. Then I suppose it does not apply to that ?

So this patch is a little outdated wrt the one for 2.5. Is there any updated version available for 2.4 ?
Can I try your patch for 2.5 on 2.4 or is there any infrastructure missing ?

TIA

Signature · 4 lines
J.A. Magallon                           #  Let the source be with you...        
mailto:[email protected]
Mandrake Linux release 8.3 (Cooker) for i586
Linux werewolf 2.4.19-pre8-jam3 #1 SMP dom may 19 21:07:40 CEST 2002 i686
#6 lore raw
In reply to Dipankar Sarma
Quoted 10 lines · J.A. Magallon
Quoted 9 lines
Quoted 4 lines

I have been experimenting with Ingo's smptimers and I ended up extending it a little bit. I would really appreciate comments on whether these things make sense or not.

I tried it out and found that we were context switching like crazy. It seems we were always running the timers out of a tasklet because we never unlocked the net_bh_lock.

Quoted 5 lines

The patch for 2.4 in

http://people.redhat.com/mingo/scalable-timers-patches/

does not acquire net_bh_lock. Then I suppose it does not apply to that ?

No. Ingo's smptimers doesn't have this problem. However I am not sure if this patch has timers completely serialized with respect to old protocol code that ran from NET_BH earlier. See deliver_to_old_ones() in net/core/dev.c. Unless I am missing something big, disabling TIMER_BH there doesn't stop timers from firing in run_local_timers().

Quoted 2 lines

Can I try your patch for 2.5 on 2.4 or is there any infrastructure missing ?

Only 2.5ish thing I use is the per-cpu area APIs from Rusty for the per-cpu tasklet in timer.c. You can replace it by an array of NR_CPUS tasklets each cache line aligned.

Thanks

Signature · 2 lines
Dipankar Sarma  <[email protected]> http://lse.sourceforge.net
Linux Technology Center, IBM Software Lab, Bangalore, India.
#7 lore raw
In reply to Dipankar Sarma
Quoted 2 lines · Anton Blanchard

:) I was surprised it worked with the missing spin_unlock too. Im testing the fixed diff now, so far it looks good.

I was positively invaded by space aliens while writing that code.

Quoted 9 lines
Quoted 9 lines

I am curious about performance of smptimers. It seems that webserver benchmark performance worsens with smptimers (Ingo version) contrary to our expectations. Do you see this ? If so, could this happen because -

1) Bouncing around of global_bh_lock cacheline by more cpus compared to earlier timer implemenation ?
2) All per-cpu timers invoked from timer_bh running in one cpu ?

Quoted 6 lines
Quoted 1 line

Do you see any other side-effects of smptimers ?

We used to see bad behaviour. It turned out to be the per cpu timer interrupt firing at exactly the same time on all cpus. One cpu would successfully spin_trylock and the others would fail and postpone the work.

This makes me wonder if using a per-cpu tasklet in my patch is a bad idea after all. Perhaps Ingo had figured it out already and used a single acquisition of global_bh_lock to fire timers for all CPUs when timers get deferred from the local timer interrupt.

In an ideal world where there are no BHs, we can parallelize the timers like I did. But with global_bh_lock serialization, I am not sure anymore if that is a good idea. I will try to get some benchmark measurements done with these two approaches.

Quoted 2 lines

We now evenly space the per cpu interrupts. Does intel do the same?

AFAICS, no. But then I am still learning.

Thanks

Signature · 2 lines
Dipankar Sarma  <[email protected]> http://lse.sourceforge.net
Linux Technology Center, IBM Software Lab, Bangalore, India.