Re: [RFC PATCH 1/3] timer: fix comments of try_to_del_timer_sync()

From: Oleg Nesterov
Date: Tue Aug 24 2010 - 12:35:41 EST


On 08/24, Yong Zhang wrote:
>
> On Tue, Aug 24, 2010 at 02:11:09PM +0200, Oleg Nesterov wrote:
> > On 08/24, Yong Zhang wrote:
> > >
> > > From: Yong Zhang <yong.zhang@xxxxxxxxxxxxx>
> > >
> > > In commit fd450b7318b75343fd76b3d95416853e34e72c95, it was saying
> > > try_to_del_timer_sync() can be used in interrupt context.
> >
> > Yes, but not in UP case.
>
> Yeah, but in UP case there is no try_to_del_timer_sync(), it's redefined
> to del_timer().

Ah, indeed, I forgot. This was another reason for the comment.

> > Please remove "#ifdef CONFIG_SMP" from set_running_timer(), then iirc
> > it can be used from irq.
>
> I have noticed your comments in the commit log, but I think it's about
> introducing the same semantic of try_to_del_timer_sync() on UP as well
> as SMP. But this patch is focusing on the current code(SMP special).
> Not about realizing try_to_del_timer_sync() on UP case. Do we need
> to do that?

I dunno.

But look, currently try_to_del_timer_sync() is not allowed from interrupt
even if it works with CONFIG_SMP.

If we "officially" allow it to use from irq, it should work on UP too but
it doesn't. del_timer() can't hang, but it can never return -1 to indicate
we hit the running timer.

Consider:

// runs in interrup context

if (try_do_del_timer_sync(&TIMER) > 0)
kfree(something_which_can_be_used_by_TIMER_func);

This is unsafe on UP.


del_timer_sync == del_timer is fine on UP. Since it must not be called
from irq, it can never hit the running timer.

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/