netdev
[Top] [All Lists]

[timers] net/sched/*

To: Alexey Kuznetsov <kuznet@xxxxxxxxxxxxx>
Subject: [timers] net/sched/*
From: Andrew Morton <andrewm@xxxxxxxxxx>
Date: Wed, 31 May 2000 00:12:33 +1000
Cc: "netdev@xxxxxxxxxxx" <netdev@xxxxxxxxxxx>
Sender: owner-netdev@xxxxxxxxxxx
I got a bit lost here.

There are a few del_timer()'s immediately preceding MOD_DEC_USE_COUNT
which look risky.
--- linux-2.4.0test1-ac5/net/sched/estimator.c  Thu Jun 10 07:45:37 1999
+++ linux-akpm/net/sched/estimator.c    Tue May 30 23:32:36 2000
@@ -191,7 +191,8 @@
                        killed++;
                }
                if (killed && elist[idx].list == NULL)
-                       del_timer(&elist[idx].timer);
+                       del_timer_async(&elist[idx].timer);
+/* REVIEWME: called from process context */
        }
 }
 
--- linux-2.4.0test1-ac5/net/sched/sch_api.c    Wed Apr 26 22:51:21 2000
+++ linux-akpm/net/sched/sch_api.c      Tue May 30 23:57:37 2000
@@ -1111,7 +1111,7 @@
 static void psched_tick(unsigned long);
 
 static struct timer_list psched_timer =
-       { NULL, NULL, 0, 0L, psched_tick };
+       { { NULL, NULL }, 0, 0L, psched_tick };
 
 static void psched_tick(unsigned long dummy)
 {
--- linux-2.4.0test1-ac5/net/sched/sch_cbq.c    Wed Apr 26 22:52:03 2000
+++ linux-akpm/net/sched/sch_cbq.c      Tue May 30 23:40:52 2000
@@ -553,7 +553,7 @@
                        cl->penalized = sched;
                        cl->cpriority = TC_CBQ_MAXPRIO;
                        q->pmask |= (1<<TC_CBQ_MAXPRIO);
-                       if (del_timer(&q->delay_timer) &&
+                       if (del_timer_async(&q->delay_timer) &&
                            (long)(q->delay_timer.expires - sched) > 0)
                                q->delay_timer.expires = sched;
                        add_timer(&q->delay_timer);
@@ -1055,7 +1055,7 @@
                sch->stats.overlimits++;
                if (q->wd_expires && !netif_queue_stopped(sch->dev)) {
                        long delay = PSCHED_US2JIFFIE(q->wd_expires);
-                       del_timer(&q->wd_timer);
+                       del_timer_async(&q->wd_timer);
                        if (delay <= 0)
                                delay = 1;
                        q->wd_timer.expires = jiffies + delay;
@@ -1263,8 +1263,11 @@
        q->pmask = 0;
        q->tx_class = NULL;
        q->tx_borrowed = NULL;
-       del_timer(&q->wd_timer);
-       del_timer(&q->delay_timer);
+/*
+ * REVIEWME: Called from process context.  I think del_timer_sync is safe here
+ */
+       del_timer_async(&q->wd_timer);
+       del_timer_async(&q->delay_timer);
        q->toplevel = TC_CBQ_MAXLEVEL;
        PSCHED_GET_TIME(q->now);
        q->now_rt = q->now;
--- linux-2.4.0test1-ac5/net/sched/sch_csz.c    Tue Aug 24 03:01:02 1999
+++ linux-akpm/net/sched/sch_csz.c      Tue May 30 23:43:25 2000
@@ -708,7 +708,7 @@
         */
        if (q->wd_expires) {
                unsigned long delay = PSCHED_US2JIFFIE(q->wd_expires);
-               del_timer(&q->wd_timer);
+               del_timer_async(&q->wd_timer);
                if (delay == 0)
                        delay = 1;
                q->wd_timer.expires = jiffies + delay;
@@ -741,7 +741,8 @@
 #ifdef CSZ_PLUS_TBF
        PSCHED_GET_TIME(&q->t_tbf);
        q->tokens = q->depth;
-       del_timer(&q->wd_timer);
+/* REVIEWME: called from process context */
+       del_timer_async(&q->wd_timer);
 #endif
        sch->q.qlen = 0;
 }
--- linux-2.4.0test1-ac5/net/sched/sch_generic.c        Mon May 15 21:24:15 2000
+++ linux-akpm/net/sched/sch_generic.c  Tue May 30 23:49:07 2000
@@ -190,7 +190,7 @@
 static void dev_watchdog_down(struct net_device *dev)
 {
        spin_lock_bh(&dev->xmit_lock);
-       if (del_timer(&dev->watchdog_timer))
+       if (del_timer_async(&dev->watchdog_timer))
                __dev_put(dev);
        spin_unlock_bh(&dev->xmit_lock);
 }
--- linux-2.4.0test1-ac5/net/sched/sch_sfq.c    Tue Aug 24 03:01:02 1999
+++ linux-akpm/net/sched/sch_sfq.c      Tue May 30 23:53:54 2000
@@ -391,7 +391,8 @@
        q->quantum = ctl->quantum ? : psched_mtu(sch->dev);
        q->perturb_period = ctl->perturb_period*HZ;
 
-       del_timer(&q->perturb_timer);
+/* REVIEWME: del_timer_sync() is safe here, and we're called from process 
context */
+       del_timer_async(&q->perturb_timer);
        if (q->perturb_period) {
                q->perturb_timer.expires = jiffies + q->perturb_period;
                add_timer(&q->perturb_timer);
@@ -435,7 +436,8 @@
 static void sfq_destroy(struct Qdisc *sch)
 {
        struct sfq_sched_data *q = (struct sfq_sched_data *)sch->data;
-       del_timer(&q->perturb_timer);
+/* REVIEWME: suggest del_timer_sync() */
+       del_timer_async(&q->perturb_timer);
        MOD_DEC_USE_COUNT;
 }
 
--- linux-2.4.0test1-ac5/net/sched/sch_tbf.c    Mon May 15 21:24:15 2000
+++ linux-akpm/net/sched/sch_tbf.c      Tue May 30 23:55:47 2000
@@ -265,7 +265,8 @@
        q->tokens = q->buffer;
        q->ptokens = q->mtu;
        sch->flags &= ~TCQ_F_THROTTLED;
-       del_timer(&q->wd_timer);
+/* REVIEWME: suggest del_timer_sync() */
+       del_timer_async(&q->wd_timer);
 }
 
 static int tbf_change(struct Qdisc* sch, struct rtattr *opt)
@@ -350,7 +351,8 @@
 {
        struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data;
 
-       del_timer(&q->wd_timer);
+/* REVIEWME: suggest del_timer_sync() */
+       del_timer_async(&q->wd_timer);
 
        if (q->P_tab)
                qdisc_put_rtab(q->P_tab);
estimator.c
===========

qdisc_kill_estimator()

  Called from sched/police.c:
    Called from tcf_police_destroy()
      Called from tcf_police_release() (include/net/pkt_sched.h)
        Called from cls_fw.c:fw_destroy()
          Called from sch_atm.c:destroy_filters()
            Called from atm_tc_put()
              Called from atm_tc_change()
                Called from cls_api.c:tc_ctl_tfilter()
                  Called from process context
                Called from sch_api.c:
            Called from atm_tc_destroy()

          Called from sch_cbq.c:
          Called from sch_dsmark.c:
          Called from sch_generic.c:
          Called from sch_ingress.c:

        Called from cls_route.c:
        Called from cls_tcindex.c:
        Called from cls_u32.c:

  Called from sched/sch_api.c:
  Called from sched/sch_cbq.c:
  Called from sched/sch_generic.c:

  Potentially racy.  Used del_timer_async.  Added REVIEWME.

sch_cbq.c
=========

cbq_ovl_delay()

  Called from cbq_under_limit()
    Called from cbq_dequeue_prio()
      Called from cbq_dequeue_1()
        Called from cbq_dequeue()
          Called from sch_dsmark.c:dsmark_dequeue(), perhaps.

  I'm lost.  del_timer_async.

cbq_dequeue()

  Lost again. del_timer_async.

cbq_reset()

  Called from qdisc_reset()
    Called from dev_deactivate()
      Called from process context.

  Used del_timer_async.  Added REVIEWME.

sch_csz.c
=========

csz_dequeue()

  I can't work out the call context for the dequeue methods.

  Make is del_timer_async().

csz_reset()

  I think this should be del_timer_sync().  Added REVIEWME.

sch_generic.c
=============

dev_watchdog_down()

  Called from dev_deactivate() under, effectively, dev->xmit_lock.
    Called from process context.

  But dev_watchdog() grabs dev->xmit_lock.

  It seems that refcounting makes this one safe.

sch_sfq.c
=========

sqf_change()

  Called from sfq_init()
    Called from process context, I assume.

  The timer is probably not running at this time.

sqf_destroy()

  This is racy (module unload).
  Suggest del_timer_sync()

sch_tbf.c
=========

tbf_reset()

  I think this is called from process context.  Suggest del_timer_sync().

tbf_destroy()

  del_timer_async() jsut before MOD_DEC_USE_COUNT.  Risky.



<Prev in Thread] Current Thread [Next in Thread>