Andrey Savochkin wrote:
>
> ...
> However, when we finish cleanup of all timer users, I would prefer to see
> something very simple like
>
> [ code..]
>
> del_timer() call from the handler is senseless, and such places (if any) may be
> spotted during the audit and testing.
Unfortunately, del_timer() calls within the handler are very, very
common, for three reasons:
1: Compete bogosity :)
2: del_timer()+add_timer() -> can be replaced with mod_timer()
3: the handler is sometimes called from mainline, as well as from
run_timer_list(). Quite valid.
Number 3 is the problem.
There are also some handlers which will deadlock with del_timer_sync()
and which will be racy with del_timer_async(). We lose either way. The
code needs rearchitecting.
For example, net/ipv6/reassembly.c: fq_free() is sometimes called under
ip6_frag_lock so it must use del_timer_async(). But fq_free() frees
resources which the handler references, and so it must use
del_timer_sync(). And fq_free() can be called from process context, so
we don't get out of it that way. Fun.
> >
> > Andrey, does this need an "rmb()"?
> ...
> What wait_on_timers() may need is just one full memory barrier at the
> very beginning before the loop. It may be important to put a barrier between
> the loop and operations executed before wait_on_timers() call (for example,
> setting do-not-readd-timer flag). However, we may have defined
> wait_on_timers() interface so that it would be the caller's due.
Uh-oh. There are three "may"s in that paragraph! Reordering of reads
and writes is pretty scary stuff, and it seems that kernel does it more
often than is necessary. Fair enough - we're human. Unless it's on the
fast path. del_timer_sync's special handling is most assuredly not on
the fast path. Probably unnecessary but if in doubt it'll get one, OK?
> Clearly, del_timer() doesn't need any such kind of stuff.
> Spinlock operations provide all the necessary barriers, and the only needed
> thing is volatile declaration for running_timer to cope with compiler
> optimization.
I'm a bit wobbly when it comes to trusting gcc to get
'struct timer_list * volatile * vtl' correct, but we'll
find out soon enough if it doesn't.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed May 31 2000 - 21:00:19 EST