Re: [ANNOUNCE] Kernel Janitor's TODO list

Daniel Phillips
2001-01-30T17:52:00Z

Daniel Phillips wrote:
>
> Rusty Russell wrote:
> >
> > In message <3A74451F.DA29FD17@uow.edu.au> you write:
> > > http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html
> > >
> > > A lot of the timer deletion races are hard to fix because of
> > > the deadlock problem.
> >
> > Hmmm...
> >
> > For 2.5, changing the timer interface to disallow mod_timer or
> > add_timer (equivalent) on self, and making the timerfn return num
> > jiffies to next run (0 = don't rerun) would solve this, right?
> > I don't see a maintainable way of solving this otherwise,
>
> It seems silly not to provide direct support for such a simple, useful
> mechanism as a periodic timer. This can be accomplished easily by
> adding a field 'periodic' to struct timer_list. If 'periodic' is
> non-zero then run_timer_list uses it to set the 'expires' field and
> re-inserts the timer.
>
> For what it's worth, this is backward compatible with the existing
> strategy. The timer_list->function is still in complete control of
> things if it wants to be, but forbidding it from re-adding itself sounds
> like an awfully good idea.

Whoops, this post from Alexy makes it quite clear why I can't do that:

http://www.wcug.wwu.edu/lists/netdev/200005/msg00050.html
Timers are self-destructable as rule. See? Normal usage
for timer is to have it allocated inside an object and
timer event detroys the object together with timer.

I did a quick scan through timer usage, and sure enough, I found
self-destructive behaviour as Alexy describes, for example, in
ax25_std_heartbeat_expiry. Your suggestion is good and simple, but
requires every timer_list->function to be changed, a couple of hundred
places to check.

It would be nice to have a nice easy transition instead of a
jump-off-the-cliff and change all usage approach. Hmm, a hack is
coming... I'll add a new, improved function field beside the old one,
call it ->timer_event, and it can force rescheduling as you suggested.
If ->timer_event is non-null it gets called instead of ->function, and
the timer may be requeued. For good measure, I'll leave my ->period
field in there because it just makes sense. Then I can write a generic
->timer_event that just returns the ->period.

/me: hack, hack, hack

--
Daniel