Rusty Russell wrote:
>
> timer_enter() and timer_exit() are no longer for public consumption.
>
> Should probably get rid of `timer_is_running()' as well, as it is
> semantically void without the timer lock.
Ah timers, timers, timers...
Agree. We should also not export timer_synchronize(), as it is only
useful/meaningful/safe when used under timerlist_lock, or when used on a
non-reinstalling timer. The subtleties of this probably mean that if
someone tried to use timer_synchronize() they'd get it wrong :( I'd
like to see the following interface:
del_timer_async() -> delete a timer, may still be running
del_timer_sync() -> delete a timer, may deadlock
del_timer() -> Legacy, deprecated async deletion
and _nothing_ else.
The test3-pre2 timer code was written by Linus. I had a couple of
issues with it and sent him the attached patch which I believe improves
things - faster, smaller, cleaner.
The main issue I have with the sequence number algorithm is that many
timers in the kernel are _not_ initialised with init_timer() - they're
simply wiped to all zeroes. So if someone does a del_timer_sync() on
one of these when timer_sequence is equal to zero, the machine will
hang.
Also, I think timer_sequence (should it continue to exist) should be
moved inside CONFIG_SMP.
Anyway, I suggest we wait and see what test3-pre3 brings us, then seek a
narrower interface - move most of this stuff into timer.c
--- linux-2.4.0-test3-pre2/kernel/timer.c Fri Jun 30 18:05:27 2000
+++ linux-akpm/kernel/timer.c Fri Jun 30 19:12:44 2000
@@ -162,7 +162,7 @@
/* Initialize both explicitly - let's try to have them in the same cache line */
spinlock_t timerlist_lock = SPIN_LOCK_UNLOCKED;
-volatile unsigned long timer_sequence = 0xfee1bad;
+volatile struct timer_list *running_timer = 0;
void add_timer(struct timer_list *timer)
{
--- linux-2.4.0-test3-pre2/include/linux/timer.h Fri Jun 30 18:05:27 2000
+++ linux-akpm/include/linux/timer.h Fri Jun 30 19:12:44 2000
@@ -52,10 +52,9 @@
unsigned long expires;
unsigned long data;
void (*function)(unsigned long);
- unsigned long sequence;
};
-extern volatile unsigned long timer_sequence;
+extern volatile struct timer_list *running_timer;
extern void add_timer(struct timer_list * timer);
extern int del_timer(struct timer_list * timer);
@@ -71,9 +70,6 @@
static inline void init_timer(struct timer_list * timer)
{
timer->list.next = timer->list.prev = NULL;
-#ifdef CONFIG_SMP
- timer->sequence = timer_sequence-1;
-#endif
}
static inline int timer_pending (const struct timer_list * timer)
@@ -82,9 +78,9 @@
}
#ifdef CONFIG_SMP
-#define timer_enter(t) do { (t)->sequence = timer_sequence; mb(); } while (0)
-#define timer_exit() do { timer_sequence++; } while (0)
-#define timer_is_running(t) ((t)->sequence == timer_sequence)
+#define timer_enter(t) do { running_timer = t; mb(); } while (0)
+#define timer_exit() do { running_timer = 0; } while (0)
+#define timer_is_running(t) (running_timer == t)
#define timer_synchronize(t) while (timer_is_running(t)) barrier()
extern int del_timer_sync(struct timer_list * timer);
#else
-
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 : Fri Jul 07 2000 - 21:00:16 EST