Commit a9efad8b authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

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: default avatarEric Dumazet <edumazet@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 3275c0c6
...@@ -61,6 +61,7 @@ psched_tdiff_bounded(psched_time_t tv1, psched_time_t tv2, psched_time_t bound) ...@@ -61,6 +61,7 @@ psched_tdiff_bounded(psched_time_t tv1, psched_time_t tv2, psched_time_t bound)
} }
struct qdisc_watchdog { struct qdisc_watchdog {
u64 last_expires;
struct hrtimer timer; struct hrtimer timer;
struct Qdisc *qdisc; struct Qdisc *qdisc;
}; };
......
...@@ -607,6 +607,10 @@ void qdisc_watchdog_schedule_ns(struct qdisc_watchdog *wd, u64 expires, bool thr ...@@ -607,6 +607,10 @@ void qdisc_watchdog_schedule_ns(struct qdisc_watchdog *wd, u64 expires, bool thr
if (throttle) if (throttle)
qdisc_throttled(wd->qdisc); qdisc_throttled(wd->qdisc);
if (wd->last_expires == expires)
return;
wd->last_expires = expires;
hrtimer_start(&wd->timer, hrtimer_start(&wd->timer,
ns_to_ktime(expires), ns_to_ktime(expires),
HRTIMER_MODE_ABS_PINNED); HRTIMER_MODE_ABS_PINNED);
......
...@@ -928,17 +928,10 @@ static struct sk_buff *htb_dequeue(struct Qdisc *sch) ...@@ -928,17 +928,10 @@ static struct sk_buff *htb_dequeue(struct Qdisc *sch)
} }
} }
qdisc_qstats_overlimit(sch); qdisc_qstats_overlimit(sch);
if (likely(next_event > q->now)) { if (likely(next_event > q->now))
if (!test_bit(__QDISC_STATE_DEACTIVATED, qdisc_watchdog_schedule_ns(&q->watchdog, next_event, true);
&qdisc_root_sleeping(q->watchdog.qdisc)->state)) { else
ktime_t time = ns_to_ktime(next_event);
qdisc_throttled(q->watchdog.qdisc);
hrtimer_start(&q->watchdog.timer, time,
HRTIMER_MODE_ABS_PINNED);
}
} else {
schedule_work(&q->work); schedule_work(&q->work);
}
fin: fin:
return skb; return skb;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment