Re: PATCH: Race in 2.6.0-test2 timer code

From: Andrew Morton (akpm@osdl.org)
Date: Tue Jul 29 2003 - 15:56:43 EST


linas@austin.ibm.com wrote:
>
> I have been chasing a pointer corruption bug in the timer code, and
> found the following race in the mod_timer() routine. I am still
> testing to see if this fixes my bug ...

Andrea says that we need to take the per-timer lock in add_timer() and
del_timer(), but I haven't yet got around to working out why.

Does this (untested) patch fix whatever race it is which you are observing?

 25-akpm/kernel/timer.c | 25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff -puN kernel/timer.c~timer-locking-fixes kernel/timer.c
--- 25/kernel/timer.c~timer-locking-fixes Tue Jul 29 13:52:21 2003
+++ 25-akpm/kernel/timer.c Tue Jul 29 13:54:55 2003
@@ -167,10 +167,12 @@ void add_timer(struct timer_list *timer)
 
         check_timer(timer);
 
- spin_lock_irqsave(&base->lock, flags);
+ spin_lock_irqsave(&timer->lock, flags);
+ spin_lock(&base->lock);
         internal_add_timer(base, timer);
         timer->base = base;
- spin_unlock_irqrestore(&base->lock, flags);
+ spin_unlock(&base->lock);
+ spin_unlock_irqrestore(&timer->lock, flags);
         put_cpu();
 }
 
@@ -190,10 +192,12 @@ void add_timer_on(struct timer_list *tim
 
         check_timer(timer);
 
- spin_lock_irqsave(&base->lock, flags);
+ spin_lock_irqsave(&timer->lock, flags);
+ spin_lock(&base->lock);
         internal_add_timer(base, timer);
         timer->base = base;
- spin_unlock_irqrestore(&base->lock, flags);
+ spin_unlock(&base->lock);
+ spin_unlock_irqrestore(&timer->lock, flags);
 }
 
 /***
@@ -298,19 +302,22 @@ int del_timer(struct timer_list *timer)
         tvec_base_t *base;
 
         check_timer(timer);
-
+ spin_lock_irqsave(&timer->lock, flags);
 repeat:
          base = timer->base;
- if (!base)
+ if (!base) {
+ spin_unlock_irqrestore(&timer->lock, flags);
                 return 0;
- spin_lock_irqsave(&base->lock, flags);
+ }
+ spin_lock(&base, flags);
         if (base != timer->base) {
- spin_unlock_irqrestore(&base->lock, flags);
+ spin_unlock(&base->lock);
                 goto repeat;
         }
         list_del(&timer->entry);
         timer->base = NULL;
- spin_unlock_irqrestore(&base->lock, flags);
+ spin_unlock(&base->lock);
+ spin_unlock_irqrestore(&timer->lock, flags);
 
         return 1;
 }

_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Jul 31 2003 - 22:00:42 EST