lkml.org 
[lkml]   [2003]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: PATCH: Race in 2.6.0-test2 timer code
Hi Mingo,

On Wed, Jul 30, 2003 at 07:57:32AM +0200, Ingo Molnar wrote:
>
> On Tue, 29 Jul 2003, Andrew Morton wrote:
> > 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.
>
> this makes no sense - in 2.6 (and in 2.4) there's no safe add_timer() /
> del_timer() use without using external SMP synchronization. (There's one
> special timer use variant involving del_timer_sync() that was safe in 2.4
> but is unsafe in 2.6, see below.)

I don't understand why you don't like this, since your patch seems to
acheive the same results as Andrea's patch (he uses timer->lock to
serialize add_timer() and del_timer(), where as your patch modifies
add_timer so that it grabs locks on old_base as well as new_base;
either approach should fix the add_timer/del_timer race.)

> What i'd propose is the attached (tested, against 2.6.0-test2) patch
> instead. It unifies the functionality of add_timer() and mod_timer(), and
> makes any combination of the timer API calls completely SMP-safe.
> del_timer() is still not using the timer lock.
>
> this patch fixes the only timer bug in 2.6 i'm aware of: the
> del_timer_sync() + add_timer() combination in kernel/itimer.c is buggy.
> This was correct code in 2.4, because there it was safe to do an
> add_timer() from the timer handler itself, parallel to a del_timer_sync().
> If we want to make this safe in 2.6 too (which i think we want to) then we
> have to make add_timer() almost equivalent to mod_timer(), locking-wise.
> And once we are at this point i think it's much cleaner to actually make
> add_timer() a variant of mod_timer(). (There's no locking cost for
> add_timer(), only the cost of an extra branch. And we've removed another
> commonly used function from the icache.)

Well, I'm confused by this a bit too, now. I saw this bug in 2.4 and I
thought that Andrea was implying that it couldn't happen in 2.6.
He seemed to be saying that the del_timer_sync() + add_timer() race
can happen only in 2.4, where add_timer() is running on the 'wrong' cpu
bacuase it got there through the evil run_all_timers()/TIMER_BH. Since
there's no run_all_timers() in 2.6, he seemed to imply that the race
'couldn't happen'. Is he right?

So, to add to my 'stupid question' quota of the day: the only way that
a timer could run on a CPU other than what it was added on would be for
a softirq to somehow get moved to a different cpu, and that is impossible,
right?

> Linas, could you please give this patch a go, does it make a difference to
> your timer list corruption problem? I've booted it on SMP and UP as well.

Still trying ... but after reading it, it looks like it will fix my 2.4 bug.

--linas

-
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/

\
 
 \ /
  Last update: 2005-03-22 13:47    [W:0.122 / U:3.640 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site