Re: [PATCH 1/9] timer locking optimization

[Roman Zippel]

Hi,

On Tue, 29 Nov 2005, Oleg Nesterov wrote:

> > +	base = timer->base;
> > +	spin_lock_irqsave(&base->lock, *flags);
> > +	while (unlikely(base != timer->base)) {
> > +		/* The timer has migrated to another CPU */
> > +		spin_unlock(&base->lock);
> >  		cpu_relax();
> > +		base = timer->base;
> > +		spin_lock(&base->lock);
> 
> This spins with interrupts disabled, not good, imho.

It's the slow path anyway, so restoring flags should indeed be fine.

> This way you can delete the timer (ret == 1), notice that timer's base
> was changed after re-locking, goto restart, and get ret == 0.

ret is only set, but not reset, so if __mod_timer() deleted the timer it 
will return 1.

> Also, you have wrong value of 'base' after 'goto restart'.

Indeed, thanks for spotting this.

bye, Roman
-
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/