From: Oleg Nesterov <oleg@tv-sign.ru>
If the timer is currently running on another CPU, __mod_timer() spins with
interrupts disabled and timer->lock held. I think it is better to
spin_unlock_irqrestore(&timer->lock) in __mod_timer's retry path.
This patch is unneccessary long. It is because it tries to cleanup the
code a bit. I do not like the fact that lock+test+unlock pattern is
duplicated in the code.
Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
25-akpm/kernel/timer.c | 92 +++++++++++++++++++++++--------------------------
1 files changed, 44 insertions(+), 48 deletions(-)
diff -puN kernel/timer.c~timers-enable-irqs-in-__mod_timer kernel/timer.c
--- 25/kernel/timer.c~timers-enable-irqs-in-__mod_timer 2005-03-21 21:38:15.000000000 -0800
+++ 25-akpm/kernel/timer.c 2005-03-21 21:38:15.000000000 -0800
@@ -174,65 +174,61 @@ int __mod_timer(struct timer_list *timer
{
tvec_base_t *old_base, *new_base;
unsigned long flags;
- int ret = 0;
+ int new_lock, ret;
BUG_ON(!timer->function);
-
check_timer(timer);
- spin_lock_irqsave(&timer->lock, flags);
- new_base = &__get_cpu_var(tvec_bases);
-repeat:
- old_base = timer_base(timer);
-
- /*
- * Prevent deadlocks via ordering by old_base < new_base.
- */
- if (old_base && (new_base != old_base)) {
- if (old_base < new_base) {
+ for (ret = -1; ret < 0; ) {
+ spin_lock_irqsave(&timer->lock, flags);
+ new_base = &__get_cpu_var(tvec_bases);
+ old_base = timer_base(timer);
+
+ /* Prevent AB-BA deadlocks. */
+ new_lock = old_base < new_base;
+ if (new_lock)
spin_lock(&new_base->lock);
- spin_lock(&old_base->lock);
- } else {
- spin_lock(&old_base->lock);
- spin_lock(&new_base->lock);
- }
- /*
- * The timer base might have been cancelled while we were
- * trying to take the lock(s), or can still be running on
- * old_base's CPU.
+
+ /* Note:
+ * (old_base == NULL) => (new_lock == 1)
+ * (old_base == new_base) => (new_lock == 0)
*/
- if (timer_base(timer) != old_base
- || old_base->running_timer == timer) {
- spin_unlock(&new_base->lock);
- spin_unlock(&old_base->lock);
- goto repeat;
+ if (old_base) {
+ spin_lock(&old_base->lock);
+
+ if (timer_base(timer) != old_base)
+ goto unlock;
+
+ if (old_base != new_base) {
+ /* Ensure the timer is serialized. */
+ if (old_base->running_timer == timer)
+ goto unlock;
+
+ if (!new_lock) {
+ spin_lock(&new_base->lock);
+ new_lock = 1;
+ }
+ }
}
- } else {
- spin_lock(&new_base->lock);
- if (timer_base(timer) != old_base) {
- spin_unlock(&new_base->lock);
- goto repeat;
+
+ ret = 0;
+ /* We hold timer->lock, no need to check old_base != 0. */
+ if (timer_pending(timer)) {
+ list_del(&timer->entry);
+ ret = 1;
}
- }
- /*
- * Delete the previous timeout (if there was any).
- * We hold timer->lock, no need to check old_base != 0.
- */
- if (timer_pending(timer)) {
- list_del(&timer->entry);
- ret = 1;
+ timer->expires = expires;
+ internal_add_timer(new_base, timer);
+ __set_base(timer, new_base, 1);
+unlock:
+ if (old_base)
+ spin_unlock(&old_base->lock);
+ if (new_lock)
+ spin_unlock(&new_base->lock);
+ spin_unlock_irqrestore(&timer->lock, flags);
}
- timer->expires = expires;
- internal_add_timer(new_base, timer);
- __set_base(timer, new_base, 1);
-
- if (old_base && (new_base != old_base))
- spin_unlock(&old_base->lock);
- spin_unlock(&new_base->lock);
- spin_unlock_irqrestore(&timer->lock, flags);
-
return ret;
}
_