Re: [RFC] mod_timer() helper functions?

[Thomas Gleixner <tglx@linutronix.de>]

On Sun, 17 May 2009, Andrew Morton wrote:

> On Sun, 17 May 2009 00:50:55 -0700 Chris Peterson <cpeterso@cpeterso.com> wrote:
> 
> > >> Reviewing the kernel's nearly one-thousand calls to mod_timer(), there
> > >> are three basic patterns:
> > >>
> > >> __* multi-second timeouts
> > >> __* millisecond timeouts
> > >> __* +1 jiffie ASAP events
> > >>
> > 
> > Is there a functional difference between the following "expire this
> > timer ASAP" statements?
> > 
> > mod_timer(timer, jiffies + 1); /* 48 uses */
> > mod_timer(timer, jiffies); /* 44 uses */
> > mod_timer(timer, jiffies - 1); /* 6 uses */
> 
> That's something which has always worried me.  Lots of code does:
> 
> 	mod_timer(timer, jiffies + 1);
> 
> (for varying values of "1").  What happens if this thread of control
> then gets stalled for a couple of jiffies.  Say it gets preempted or
> there's a long interrupt or whatever.  So there's a several-jiffy
> interval between the caller evaluating jiffies+1 and the entry to
> mod_timer().
> 
> 
> >From my reading, we'll hit
> 
> 		int i;
> 		/* If the timeout is larger than 0xffffffff on 64-bit
> 		 * architectures then we use the maximum timeout:
> 		 */
> 		if (idx > 0xffffffffUL) {
> 			idx = 0xffffffffUL;
> 			expires = idx + base->timer_jiffies;
> 		}
> 		i = (expires >> (TVR_BITS + 3 * TVN_BITS)) & TVN_MASK;
> 		vec = base->tv5.vec + i;
> 
> and the timer gets scheduled at some time in the far-distant future!
> 
> But this is such a glaring and huge problem that surely it cannot
> exist.  But I don't know why not.
> 
> If the bug _does_ exist then mod_timer(timer, jiffies - 1) will set the
> timer to go off in the far future.  mod_timer(timer, jiffies) will
> usually make it go off real soon now, but it's scary.  mod_timer(timer,
> jiffies + 1) is safer, but still vulnerable.

You missed a little detail:

	} else if ((signed long) idx < 0) {
		/*
		 * Can happen if you add a timer with expires == jiffies,
		 * or you set a timer to go off in the past
		 */
		vec = base->tv1.vec + (base->timer_jiffies & TVR_MASK);
	} else {
		int i;
		/* If the timeout is larger than 0xffffffff on 64-bit
		 * architectures then we use the maximum timeout:
		 */

So if the timer is set to go off in the past or right away then it is
enqueued into the timer list bucket which is processed in the next
timer softirq. That's why we use base->timer_jiffies and not jiffies
directly. base->timer_jiffies is incremented in the timer
softirq. softirq and enqueueing are serialized via base->lock.

Thanks,

	tglx