kuznet@ms2.inr.ac.ru wrote:
>
> > +static struct timer_list *running_timer;
>
> I see, I see. Seems, this should work. But this implementation looks
> utterly ugly (not a big problem in practice)
You ain't seen ugly yet.
> and difficult to generalize.
Not so difficult :)
Turn running_timer into a per-CPU array and place the current CPU
identifier into the timer_list struct when the handler is running.
Attached here is a version of this patch which does support concurrent
handlers. This is just for reference purposes, closure and all that.
This is simply too Byzantine.
--- linux-2.4.0test1-ac5/kernel/timer.c Mon May 15 21:25:15 2000
+++ linux-akpm/kernel/timer.c Sat Jun 3 15:16:51 2000
@@ -13,6 +13,8 @@
* serialize accesses to xtime/lost_ticks).
* Copyright (C) 1998 Andrea Arcangeli
* 1999-03-10 Improved NTP compatibility by Ulrich Windl
+ * 2000-05-27 Make del_timer synchronous, add del_timer_async - Andrew Morton.
+ * http://kernelnotes.org/lnxlists/linux-kernel/lk_0005_04/msg00672.html
*/
#include <linux/config.h>
@@ -72,6 +74,24 @@
unsigned long prof_len;
unsigned long prof_shift;
+#ifdef CONFIG_SMP
+/*
+ * A non-zero entry here indicates a currently-running timer on
+ * the CPU identified by running_timer_cpu.
+ */
+struct running_timer_t {
+ struct timer_list *t;
+} ____cacheline_aligned running_timers[NR_CPUS];
+
+/* FIXME: delete this stuff */
+#ifdef CONFIG_X86
+extern void show_stack(unsigned long* esp); /* Why doesn't this have a prototype? */
+#define x86_show_stack() show_stack(0)
+#else
+#define x86_show_stack() do { } while (0)
+#endif
+#endif /* CONFIG_SMP */
+
/*
* Event timer code
*/
@@ -201,6 +221,7 @@
return ret;
}
+#ifndef CONFIG_SMP
int del_timer(struct timer_list * timer)
{
int ret;
@@ -212,39 +233,94 @@
spin_unlock_irqrestore(&timerlist_lock, flags);
return ret;
}
+#endif
#ifdef CONFIG_SMP
-/*
- * SMP specific function to delete periodic timer.
- * Caller must disable by some means restarting the timer
- * for new. Upon exit the timer is not queued and handler is not running
- * on any CPU. It returns number of times, which timer was deleted
- * (for reference counting).
- */
+
+int del_timer_async(struct timer_list * timer)
+{
+ int ret;
+ unsigned long flags;
+
+ spin_lock_irqsave(&timerlist_lock, flags);
+ ret = detach_timer(timer);
+ timer->list.next = timer->list.prev = NULL;
+ spin_unlock_irqrestore(&timerlist_lock, flags);
+ return ret;
+}
int del_timer_sync(struct timer_list * timer)
{
- int ret = 0;
+ int ret;
+ unsigned long flags;
- for (;;) {
- unsigned long flags;
- int running;
+ spin_lock_irqsave(&timerlist_lock, flags);
+ ret = detach_timer(timer);
+ timer->list.next = timer->list.prev = 0;
- spin_lock_irqsave(&timerlist_lock, flags);
- ret += detach_timer(timer);
- timer->list.next = timer->list.prev = 0;
- running = timer->running;
+ { /* I hate C */
+ const int timer_cpu = timer->cpu;
+ if (running_timers[timer_cpu].t == timer) { /* This timer's handler is presently running */
+ int j;
+ struct timer_list * volatile *vtl;
+
+ if (timer_cpu == smp_processor_id()) {
+ /*
+ * Either the handler is deleting the timer (which is pointless) or
+ * a hard IRQ is calling del_timer_sync, which is a bug
+ */
+ printk( "del_timer_sync: Deleting timer %p%s\n", timer,
+ in_irq() ? " in hardware interrupt"
+ : (in_interrupt() ? " in handler" : " insane"));
+ goto out;
+ }
+
+ /* FIXME: temp */
+ printk( "del_timer_sync(%p) during handler execution! Called from %p\n"
+ "See http://www.uow.edu.au/~andrewm/linux/del_timer.html\n",
+ timer, __builtin_return_address(0));
+ x86_show_stack();
+re_del_timer:
+ /* Unlock the timer list and spin until handler completion. */
spin_unlock_irqrestore(&timerlist_lock, flags);
+ vtl = (struct timer_list * volatile *)&(running_timers[timer_cpu].t);
+ for (j = 50 * 1000 * 1000; j != 0; --j) {
+ if (*vtl != timer)
+ break;
+ }
+ spin_lock_irqsave(&timerlist_lock, flags);
- if (!running)
- return ret;
- timer_synchronize(timer);
+ if (j == 0) {
+ printk( "del_timer_sync(%p): deadlock! Called from %p\n",
+ timer, __builtin_return_address(0));
+ printk("See http://www.uow.edu.au/~andrewm/linux/deadlock.html\n");
+ x86_show_stack();
+ } else {
+ if (running_timers[timer_cpu].t == timer) {
+ /* There's a tiny chance that the same timer handler has started to run again */
+ printk("del_timer_sync(%p): the timer started running again!\n", timer);
+ x86_show_stack();
+ goto re_del_timer;
+ }
+ /*
+ * OK, we have the lock and the timer handler is definitely not running. But the
+ * handler may have re-added the timer, and it may have deleted the timer.
+ * But we assume that the handler has NOT kfree'ed the timer. We can assume this
+ * because calling del_timer() on a timer which is freed in the handler is a BUG.
+ */
+ if (timer_pending(timer)) {
+ printk("del_timer_sync(): timer was re-added\n");
+ ret += detach_timer(timer);
+ timer->list.next = timer->list.prev = 0;
+ }
+ }
}
-
+ } /* I like C++ */
+out:
+ spin_unlock_irqrestore(&timerlist_lock, flags);
return ret;
}
-#endif
-
+#endif /* CONFIG_SMP */
static inline void cascade_timers(struct timer_vec *tv)
{
@@ -272,6 +348,9 @@
static inline void run_timer_list(void)
{
+#ifdef CONFIG_SMP
+ const int this_cpu = smp_processor_id();
+#endif
spin_lock_irq(&timerlist_lock);
while ((long)(jiffies - timer_jiffies) >= 0) {
struct list_head *head, *curr;
@@ -295,10 +374,20 @@
detach_timer(timer);
timer->list.next = timer->list.prev = NULL;
- timer_set_running(timer);
+#ifdef CONFIG_SMP
+ if (running_timers[this_cpu].t != 0)
+ BUG();
+ running_timers[this_cpu].t = timer;
+ timer->cpu = this_cpu;
+#endif
spin_unlock_irq(&timerlist_lock);
fn(data);
spin_lock_irq(&timerlist_lock);
+#ifdef CONFIG_SMP
+ if (running_timers[this_cpu].t != timer)
+ BUG();
+ running_timers[this_cpu].t = 0;
+#endif
goto repeat;
}
++timer_jiffies;
--- linux-2.4.0test1-ac5/include/linux/timer.h Tue May 30 18:45:52 2000
+++ linux-akpm/include/linux/timer.h Sat Jun 3 15:12:55 2000
@@ -54,11 +54,50 @@
unsigned long expires;
unsigned long data;
void (*function)(unsigned long);
- volatile int running;
+#ifdef CONFIG_SMP
+ int cpu;
+#endif
};
extern void add_timer(struct timer_list * timer);
+#define timer_exit(t) (void)(t)
+
+#ifdef CONFIG_SMP
+extern int del_timer_async(struct timer_list * timer);
+extern int del_timer_sync(struct timer_list * timer);
+#ifndef del_timer
+#define del_timer del_timer_async
+#endif
+
+#define INIT_TIMER_FND(fn, d) { \
+ list: LIST_HEAD_INIT_NULL, \
+ expires: 0, \
+ data: d, \
+ function: fn, \
+ cpu: 0 \
+}
+
+#else /* CONFIG_SMP */
+
extern int del_timer(struct timer_list * timer);
+#ifndef del_timer_sync
+#define del_timer_sync del_timer
+#endif
+#ifndef del_timer_async
+#define del_timer_async del_timer
+#endif
+
+#define INIT_TIMER_FND(fn, d) { \
+ list: LIST_HEAD_INIT_NULL, \
+ expires: 0, \
+ data: d, \
+ function: fn, \
+}
+
+#endif /* CONFIG_SMP */
+
+#define INIT_TIMER_FN(fn) INIT_TIMER_FND(fn, 0)
+#define INIT_TIMER INIT_TIMER_FN(0)
/*
* mod_timer is a more efficient way to update the expire field of an
@@ -72,29 +111,13 @@
static inline void init_timer(struct timer_list * timer)
{
timer->list.next = timer->list.prev = NULL;
-#ifdef CONFIG_SMP
- timer->running = 0;
-#endif
+ timer->cpu = 0;
}
static inline int timer_pending (const struct timer_list * timer)
{
return timer->list.next != NULL;
}
-
-#ifdef CONFIG_SMP
-#define timer_exit(t) do { (t)->running = 0; mb(); } while (0)
-#define timer_set_running(t) do { (t)->running = 1; mb(); } while (0)
-#define timer_is_running(t) ((t)->running != 0)
-#define timer_synchronize(t) while (timer_is_running(t)) barrier()
-extern int del_timer_sync(struct timer_list * timer);
-#else
-#define timer_exit(t) (void)(t)
-#define timer_set_running(t) (void)(t)
-#define timer_is_running(t) (0)
-#define timer_synchronize(t) do { (void)(t); barrier(); } while(0)
-#define del_timer_sync(t) del_timer(t)
-#endif
/*
* These inlines deal with timer wrapping correctly. You are
--- linux-2.4.0test1-ac5/include/linux/list.h Wed Apr 26 22:52:03 2000
+++ linux-akpm/include/linux/list.h Sat May 27 22:26:43 2000
@@ -19,6 +19,8 @@
#define LIST_HEAD_INIT(name) { &(name), &(name) }
+#define LIST_HEAD_INIT_NULL { 0, 0 }
+
#define LIST_HEAD(name) \
struct list_head name = LIST_HEAD_INIT(name)
--- linux-2.4.0test1-ac5/kernel/ksyms.c Tue May 30 18:45:52 2000
+++ linux-akpm/kernel/ksyms.c Tue May 30 18:51:13 2000
@@ -341,9 +341,16 @@
EXPORT_SYMBOL(proc_doulongvec_ms_jiffies_minmax);
EXPORT_SYMBOL(proc_doulongvec_minmax);
-/* interrupt handling */
+/* kernel timers */
EXPORT_SYMBOL(add_timer);
+#ifdef CONFIG_SMP
+EXPORT_SYMBOL(del_timer_async);
+EXPORT_SYMBOL(del_timer_sync);
+#else
EXPORT_SYMBOL(del_timer);
+#endif
+
+/* interrupt handling */
EXPORT_SYMBOL(request_irq);
EXPORT_SYMBOL(free_irq);
@@ -356,9 +363,6 @@
EXPORT_SYMBOL(autoirq_report);
#endif
-#ifdef CONFIG_SMP
-EXPORT_SYMBOL(del_timer_sync);
-#endif
EXPORT_SYMBOL(mod_timer);
EXPORT_SYMBOL(tq_timer);
EXPORT_SYMBOL(tq_immediate);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Jun 07 2000 - 21:00:16 EST