Hi Andrew,
On Sun, May 28, 2000 at 12:11:36PM +1000, Andrew Morton wrote:
> Andrew Morton wrote:
> >
> > Patch attached for review/comments.
>
> "Obviously wrong". New version here.
The two versions look equivalent for me. But it doesn't matter.
The patch seems to be ok.
However, when we finish cleanup of all timer users, I would prefer to see
something very simple like
--- kernel/timer.c.timer Sun May 28 13:34:09 2000
+++ kernel/timer.c Sun May 28 13:34:09 2000
@@ -216,13 +216,10 @@
#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
+ * It returns number of times, which timer was deleted
* (for reference counting).
*/
-
-int del_timer_sync(struct timer_list * timer)
+int del_timer(struct timer_list * timer)
{
int ret = 0;
@@ -233,12 +230,14 @@
spin_lock_irqsave(&timerlist_lock, flags);
ret += detach_timer(timer);
timer->list.next = timer->list.prev = 0;
- running = timer->running;
+ running = (running_timer == timer);
spin_unlock_irqrestore(&timerlist_lock, flags);
if (!running)
return ret;
- timer_synchronize(timer);
+
+ /* spin until handler completion */
+ while (running_timer == timer);
}
return ret;
It's just the essence of your idea of `running_timer' variable.
del_timer() call from the handler is senseless, and such places (if any) may be
spotted during the audit and testing.
>
> Andrey, does this need an "rmb()"? I didn't understnad why your
> wait_on_timers() implementation was using one.
No, the code doesn't need it.
It was just what my fingers typed... :-)
What wait_on_timers() may need is just one full memory barrier at the
very beginning before the loop. It may be important to put a barrier between
the loop and operations executed before wait_on_timers() call (for example,
setting do-not-readd-timer flag). However, we may have defined
wait_on_timers() interface so that it would be the caller's due.
Clearly, del_timer() doesn't need any such kind of stuff.
Spinlock operations provide all the necessary barriers, and the only needed
thing is volatile declaration for running_timer to cope with compiler
optimization.
Best regards
Andrey
-
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 May 31 2000 - 21:00:18 EST