Re: [patch] del_timer_sync scalability patch

[Oleg Nesterov]

Hello.

I am not sure, but I think this patch incorrect.

> @@ -466,6 +482,7 @@ repeat:
>  			set_running_timer(base, timer);
>  			smp_wmb();
>  			timer->base = NULL;
------>	WINDOW <------
> +			set_last_running(timer, base);
>  			spin_unlock_irq(&base->lock);

Suppose it is the first invocation of timer, so
timer->last_running == NULL.

What if del_timer_sync() happens in that window?

del_timer_sync:
	del_timer();	// timer->base == NULL, returns

	base = timer->last_running;
	if (base)	// no, it is still NULL
		...

	if (timer->base != NULL || timer->last_running != base)
		goto del_again;	// not taken

	return;

I think it is not enough to exchange these 2 lines in
__run_timers, we also need barriers.

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