net_sched: avoid too many hrtimer_start() calls

I found a serious performance bug in packet schedulers using hrtimers.

sch_htb and sch_fq are definitely impacted by this problem.

We constantly rearm high resolution timers if some packets are throttled
in one (or more) class, and other packets are flying through qdisc on
another (non throttled) class.

hrtimer_start() does not have the mod_timer() trick of doing nothing if
expires value does not change :

	if (timer_pending(timer) &&
            timer->expires == expires)
                return 1;

This issue is particularly visible when multiple cpus can queue/dequeue
packets on the same qdisc, as hrtimer code has to lock a remote base.

I used following fix :

1) Change htb to use qdisc_watchdog_schedule_ns() instead of open-coding
it.

2) Cache watchdog prior expiration. hrtimer might provide this, but I
prefer to not rely on some hrtimer internal.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 401038d2f9b88c536788507b6168675333d02fe0..fea53f4d92ca6bb16c367ee772d4c8c1aa19b701 100644 (file)
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -61,6 +61,7 @@ psched_tdiff_bounded(psched_time_t tv1, psched_time_t tv2, psched_time_t bound)
 }
 
 struct qdisc_watchdog {
+       u64             last_expires;
        struct hrtimer  timer;
        struct Qdisc    *qdisc;
 };

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 64f71a2155f3ae8199123c0ee635fc9fa420c35b..ddf047df5361b8c0d05fead97ee1b5ccdeee4113 100644 (file)
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -607,6 +607,10 @@ void qdisc_watchdog_schedule_ns(struct qdisc_watchdog *wd, u64 expires, bool thr
        if (throttle)
                qdisc_throttled(wd->qdisc);
 
+       if (wd->last_expires == expires)
+               return;
+
+       wd->last_expires = expires;
        hrtimer_start(&wd->timer,
                      ns_to_ktime(expires),
                      HRTIMER_MODE_ABS_PINNED);

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index f6bf5818ed4d265feafe9fd44319371fbc5c1d26..d4b4218af6b1b8dc9824408a3b4f62986cae3f18 100644 (file)
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -928,17 +928,10 @@ ok:
                }
        }
        qdisc_qstats_overlimit(sch);
-       if (likely(next_event > q->now)) {
-               if (!test_bit(__QDISC_STATE_DEACTIVATED,
-                             &qdisc_root_sleeping(q->watchdog.qdisc)->state)) {
-                       ktime_t time = ns_to_ktime(next_event);
-                       qdisc_throttled(q->watchdog.qdisc);
-                       hrtimer_start(&q->watchdog.timer, time,
-                                     HRTIMER_MODE_ABS_PINNED);
-               }
-       } else {
+       if (likely(next_event > q->now))
+               qdisc_watchdog_schedule_ns(&q->watchdog, next_event, true);
+       else
                schedule_work(&q->work);
-       }
 fin:
        return skb;
 }