[timers] net/core/*

[Andrew Morton <andrewm@xxxxxxxxxx>]

A couple of possible races.  neigh_del_timer() looks ugly.  Fixed a race
in whitehole_close().

We could just apply these patches and revisit the REVIEWME comments at
some time in the future of course.   Or you could just tell me to buzz
off :)

-- 
-akpm-

dst.c
=====

dst_gc_run()

  OK, this is the timer handler, so del_timer_async is safe.

  ** Why bother deleting the timer in the timer handler?  It's gone!

__dst_free()

  Called from dst_free() (include/net/dst.h)
    Called from decnet/dn_route.c:dn_dst_check_expire()
      OK, timer handler
    Called from decnet/dn_route.c:dn_dst_gc()
      Called from dst.c:dst_alloc() (Only place?)
        Called from decnet/dn_route.c:dn_route_output_slow()
          Called from dn_route_output()
            Called from dn_cache_getroute()
          Called from dn_connect()
            Called from process context.
        Called from decnet/dn_route.c:dn_route_input_slow()
        Called from ipv4/route.c
    Called from ipv4/route.c:
    Called from ipv6/ip6_fib.c:
    Called from ipv6/route.c:

  OK, we have one path by which __dst_free() can be called from process context.

  I'm dazed and confused.

neighbour.c
===========

neigh_del_timer()

  Called from neigh_ifdown()
    Called from neigh_table_clear()
      Called from decnet/dn_neigh.c:dn_neigh_cleanup()
        Called from process context
      Called from ipv6/ndisc.c:ndisc_cleanup()
        Called from process context

  neigh_if_down() uses write_lock_bh(&tbl->lock) and write_lock(&n->lock);
  neigh_timer_handler() uses write_lock(&neigh->lock);

  Oh how very sticky.  We can't use del_timer_sync() because it'll
  deadlock.  But neigh_release() will race horridly against
  neigh_timer_handler().

  I used del_timer_async().  Added REVIEWME.

neigh_proxy_process()

  OK, this is a timer handler.

  ** the del_timer_async() seems unnecessary.

pneigh_enqueue()
  Called from ipv4/arp.c:arp_rcv().
    OK, BH context.
  Called from ipv6/ndisc.c:ndisc_rcv()
    OK, BH context.

  **EXPORTED TO MODULES**

neigh_table_clear()
  Uses del_timer_sync()

profile.c
=========

whitehole_close()

  Racy.  Use del_timer_sync().


--- linux-2.4.0test1-ac5/net/core/dst.c Mon May 15 21:24:15 2000
+++ linux-akpm/net/core/dst.c   Tue May 30 22:32:41 2000
@@ -50,8 +50,10 @@
                return;
        }
 
-
-       del_timer(&dst_gc_timer);
+/*
+ * REVIEWME: I don't think we need to delete the timer at all here
+ */
+       del_timer_async(&dst_gc_timer);
        dstp = &dst_garbage_list;
        while ((dst = *dstp) != NULL) {
                if (atomic_read(&dst->__refcnt)) {
@@ -128,13 +130,16 @@
        dst->next = dst_garbage_list;
        dst_garbage_list = dst;
        if (dst_gc_timer_inc > DST_GC_INC) {
-               del_timer(&dst_gc_timer);
+               del_timer_async(&dst_gc_timer);
                dst_gc_timer_inc = DST_GC_INC;
                dst_gc_timer_expires = DST_GC_MIN;
                dst_gc_timer.expires = jiffies + dst_gc_timer_expires;
                add_timer(&dst_gc_timer);
        }
-
+/*
+ * REVIEWME: this function can be called from process context.  See core.txt.
+ * Can it race?
+ */
        spin_unlock_bh(&dst_lock);
 }
 
--- linux-2.4.0test1-ac5/net/core/neighbour.c   Sat Apr 29 21:22:41 2000
+++ linux-akpm/net/core/neighbour.c     Tue May 30 23:01:46 2000
@@ -156,8 +156,12 @@
 
 static int neigh_del_timer(struct neighbour *n)
 {
+/*
+ * REVIEWME.  Can be called from process context, can't use del_timer_sync.  I 
think we
+ * have a problem here.  See core.txt.
+ */
        if (n->nud_state & NUD_IN_TIMER) {
-               if (del_timer(&n->timer)) {
+               if (del_timer_async(&n->timer)) {
                        neigh_release(n);
                        return 1;
                }
@@ -1016,7 +1020,7 @@
                } else if (!sched_next || tdif < sched_next)
                        sched_next = tdif;
        }
-       del_timer(&tbl->proxy_timer);
+       del_timer_async(&tbl->proxy_timer);
        if (sched_next) {
                tbl->proxy_timer.expires = jiffies + sched_next;
                add_timer(&tbl->proxy_timer);
@@ -1036,7 +1040,7 @@
        }
        skb->stamp.tv_sec = 0;
        skb->stamp.tv_usec = now + sched_next;
-       if (del_timer(&tbl->proxy_timer)) {
+       if (del_timer_async(&tbl->proxy_timer)) {
                long tval = tbl->proxy_timer.expires - now;
                if (tval < sched_next)
                        sched_next = tval;
--- linux-2.4.0test1-ac5/net/core/profile.c     Wed Apr 26 22:51:21 2000
+++ linux-akpm/net/core/profile.c       Tue May 30 23:02:36 2000
@@ -172,7 +172,7 @@
 
 static int whitehole_close(struct net_device *dev)
 {
-       del_timer(&whitehole_timer);
+       del_timer_sync(&whitehole_timer);
        return 0;
 }