Re: [patch 5/8] hrtimer remove state field

[Thomas Gleixner]

On Sun, 2006-03-12 at 13:13 +0100, Roman Zippel wrote:
> >  		spin_lock_irq(&base->lock);
> >  
> > -		/* Another CPU has added back the timer */
> > -		if (timer->state != HRTIMER_INACTIVE)
> > -			continue;
> > -
> > -		if (restart != HRTIMER_NORESTART)
> > +		if (restart != HRTIMER_NORESTART &&
> > +		    !hrtimer_active(timer))
> >  			enqueue_hrtimer(timer, base);
> >  	}
> >  	set_curr_timer(base, NULL);
> 
> BTW the active check can be removed again, as it was added for a state 
> machine problem, I only didn't want to remove it for 2.6.16.

The check can not be removed. The reason why it was added is still the
same. 

It has nothing to do with a state machine. It's a simple SMP locking
issue.

softirq runs on CPU0
base->lock()

remove_timer(timer);

base->unlock()
			signal of previous expiry is delivered on CPU1
			timer is reqeued.
requeue = timer->fn();

base->lock()

if (requeue)
	enqueue_timer(timer)

--> OOPS

We can not wait in the signal delivery path until the callback has been
executed, as we hold the posix-timer->lock and this would deadlock
timer->fn().

	tglx


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