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;
 }
 
_