Re: [ANNOUNCE] Kernel Janitor's TODO list

[Daniel Phillips]

Daniel Phillips wrote:
> 
> Rusty Russell wrote:
> >
> > In message <[e-mail]> 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