Re: [patch] SMP races in the timer code, timer-fix-2.6.0-test7-A0

[Linus Torvalds]

On Sat, 11 Oct 2003, Ingo Molnar wrote:
> 
> i've implemented the timer->running field, and it's quite straightforward
> and solves all 3 types of races. It also simplifies del_timer_sync() quite
> visibly, and it's probably a fair del_timer_sync() speedup as well. I
> tested the attached patch on SMP and UP x86 as well, works fine for me.

This one is also buggy.

It is _not_ ok to do this:

>  			list_del(&timer->entry);
>  			timer->base = NULL;
> -			set_running_timer(base, timer);
>  			spin_unlock_irq(&base->lock);
>  			fn(data);
>  			spin_lock_irq(&base->lock);
> +#ifdef CONFIG_SMP
> +			timer->running = 0;
> +#endif

since the timer may not even _exist_ any more - the act of running the 
timer may well have free'd the timer structure, and you're now zeroing 
memory that may have been re-allocated for something else.

This is exactly why we load all the timer data into local variables before 
we run the timer function, and we don't touch "timer" afterwards.

In short, you really need to re-visit this whole thing.

		Linus

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