Re: [PATCH] Add support for deferrable timers (respun)

Oleg Nesterov
2007-03-27T21:11:57Z

On 03/27, Venki Pallipadi wrote:
>
> for (;;) {
> - base = timer->base;
> + tvec_base_t *prelock_base = timer->base;
> + base = timer_get_base(timer);
> if (likely(base != NULL)) {
> spin_lock_irqsave(&base->lock, *flags);
> - if (likely(base == timer->base))
> + if (likely(prelock_base == timer->base))
> return base;

I don't think this is correct, at least in theory.

Suppose that

tvec_base_t *prelock_base = timer->base;
base = timer_get_base(timer);

are re-ordered (the second LOAD happens after the first one), and the timer
changes its base in between. Now, we lock the old base, and return it because
"prelock_base == timer->base" == true.

Oleg.