lkml.org 
[lkml]   [2000]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] Minor timer.c cleanup test3-pre2
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
\
 
 \ /
  Last update: 2005-03-22 13:57    [W:0.104 / U:0.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site