Rusty Russell wrote:
>
> In message <39654DE5.389BEFB@uow.edu.au> you write:
> > > sync_timers() -> finish all timers currently running (this can be
> > > trivially implemented as spin_lock_wait(&global_bh_lock).
> >
> > Where does the requirement for sync_timers() appear? How would it be used?
>
> module unload, where module count is decremented by timer; timer must
> be finished. Also: `if (!del_timer(timer)) sync_timers();' ==
> del_timer_sync() for self-destructing timers.
OK, that makes sense. It's one of the few sane ways of doing this.
How does the attached hastily-thrown-together-not-doing-timers-at-present
patch look?
Notes:
- We really should get rid of the eighteen trillion RUN_AT macros
and turn them into real functions - add_timer_at(), mod_timer_at().
Do the addition to `jiffies' in a single place.
- The test3-pre4 implementation has a potential bug:
volatile struct timer_list * running_timer;
should have been
struct timer_list * volatile running_timer;
Fortunately gcc throws its hands up and runs away as
soon as it gets a whiff of `volatile'.
- The following architectures don't provide spin_unlock_wait:
* asm-arm
* asm-m68k
* asm-mips
* asm-sh
And hence won't compile with the attached patch and CONFIG_SMP. It
appears that these architectures don't support SMP anyway.
BTW: how do we make decisions about inlining things? I often see
what appear to be dubious inlining decisions. For example,
internal_add_timer(). That's a BIG function. Is there a net
benefit in having up to three copies sitting in the icache?
--- linux-2.4.0-test3-pre4/include/linux/timer.h Thu Jul 6 20:23:47 2000
+++ linux-akpm/include/linux/timer.h Fri Jul 7 22:25:48 2000
@@ -24,14 +24,23 @@
void (*function)(unsigned long);
};
-extern volatile struct timer_list *running_timer;
extern void add_timer(struct timer_list * timer);
extern int del_timer(struct timer_list * timer);
+#ifdef CONFIG_SMP
+extern int del_timer_sync(struct timer_list * timer);
+extern void sync_timers(void);
+#else
+#define del_timer_sync(t) del_timer(t)
+#define sync_timers() do { } while (0)
+#endif
+
/*
* mod_timer is a more efficient way to update the expire field of an
* active timer (if the timer is inactive it will be activated)
- * mod_timer(a,b) is equivalent to del_timer(a); a->expires = b; add_timer(a)
+ * mod_timer(a,b) is equivalent to del_timer(a); a->expires = b; add_timer(a).
+ * If the timer is known to be not pending (ie, in the handler), mod_timer
+ * is less efficient than a->expires = b; add_timer(a).
*/
int mod_timer(struct timer_list *timer, unsigned long expires);
@@ -46,20 +55,6 @@
{
return timer->list.next != NULL;
}
-
-#ifdef CONFIG_SMP
-#define timer_enter(t) do { running_timer = t; mb(); } while (0)
-#define timer_exit() do { running_timer = NULL; } 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
-#define timer_enter(t) do { } while (0)
-#define timer_exit() do { } while (0)
-#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.0-test3-pre4/kernel/timer.c Thu Jul 6 20:23:47 2000
+++ linux-akpm/kernel/timer.c Fri Jul 7 22:36:59 2000
@@ -162,7 +162,19 @@
/* Initialize both explicitly - let's try to have them in the same cache line */
spinlock_t timerlist_lock = SPIN_LOCK_UNLOCKED;
-volatile struct timer_list *running_timer = NULL;
+volatile struct timer_list * volatile running_timer = NULL;
+
+#ifdef CONFIG_SMP
+#define timer_enter(t) do { running_timer = t; mb(); } while (0)
+#define timer_exit() do { running_timer = NULL; } while (0)
+#define timer_is_running(t) (running_timer == t)
+#define timer_synchronize(t) while (timer_is_running(t)) barrier()
+#else
+#define timer_enter(t) do { } while (0)
+#define timer_exit() do { } while (0)
+#define timer_is_running(t) (0)
+#define timer_synchronize(t) do { (void)(t); barrier(); } while(0)
+#endif
void add_timer(struct timer_list *timer)
{
@@ -216,6 +228,11 @@
}
#ifdef CONFIG_SMP
+void sync_timers(void)
+{
+ spin_unlock_wait(&global_bh_lock);
+}
+
/*
* SMP specific function to delete periodic timer.
* Caller must disable by some means restarting the timer
-
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:21 EST