Re: [PATCH 6/5] timers: enable irqs in __mod_timer()

[Andrew Morton <akpm@osdl.org>]

Oleg Nesterov <oleg@tv-sign.ru> wrote:
>
> 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.
> 
>  If you think that this patch uglifies the code or does not match
>  kernel's coding style - just say nack :)

I've seen worse ;)

I think this makes it a bit more kernel-like?

--- 25/kernel/timer.c~timers-enable-irqs-in-__mod_timer-tidy	2005-03-21 21:41:03.000000000 -0800
+++ 25-akpm/kernel/timer.c	2005-03-21 21:41:57.000000000 -0800
@@ -174,12 +174,13 @@ int __mod_timer(struct timer_list *timer
 {
 	tvec_base_t *old_base, *new_base;
 	unsigned long flags;
-	int new_lock, ret;
+	int new_lock;
+	int ret = -1;
 
 	BUG_ON(!timer->function);
 	check_timer(timer);
 
-	for (ret = -1; ret < 0; ) {
+	do {
 		spin_lock_irqsave(&timer->lock, flags);
 		new_base = &__get_cpu_var(tvec_bases);
 		old_base = timer_base(timer);
@@ -227,7 +228,7 @@ unlock:
 		if (new_lock)
 			spin_unlock(&new_base->lock);
 		spin_unlock_irqrestore(&timer->lock, flags);
-	}
+	} while (ret == -1);
 
 	return ret;
 }
_