[PATCH net-next 0/2] pkt_sched: allow scheduling points

[PATCH net-next 0/2] pkt_sched: allow scheduling points

[Eric Dumazet]

We have seen delays of more than 50ms in class or qdisc dumps, in case
device is under high TX stress, even with the prior 4KB per skb limit.

With the new 16KB limit, this could translate to 200ms delays.

Add cond_resched() to give a chance to higher prio tasks to get cpu.

But before doing so, we need to remove the rcu locking from tc_dump_qdisc()
as David spotted.

Eric Dumazet (2):
  pkt_sched: do not use rcu in tc_dump_qdisc()
  pkt_sched: add cond_resched() to class and qdisc dump

 net/sched/sch_api.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
1.9.0.279.gdc9e3eb

[PATCH net-next 1/2] pkt_sched: do not use rcu in tc_dump_qdisc()

[Eric Dumazet]

Like all rtnetlink dump operations, we hold RTNL in tc_dump_qdisc(),
so we do not need to use rcu protection to protect list of netdevices.

This will allow preemption to occur, thus reducing latencies.
Following patch adds explicit cond_resched() calls.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_api.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 1313145e3b86..272292efa7f0 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1434,9 +1434,9 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 	s_idx = cb->args[0];
 	s_q_idx = q_idx = cb->args[1];
 
-	rcu_read_lock();
 	idx = 0;
-	for_each_netdev_rcu(net, dev) {
+	ASSERT_RTNL();
+	for_each_netdev(net, dev) {
 		struct netdev_queue *dev_queue;
 
 		if (idx < s_idx)
@@ -1459,8 +1459,6 @@ cont:
 	}
 
 done:
-	rcu_read_unlock();
-
 	cb->args[0] = idx;
 	cb->args[1] = q_idx;
 
-- 
1.9.0.279.gdc9e3eb

[PATCH net-next 2/2] pkt_sched: add cond_resched() to class and qdisc dump

[Eric Dumazet]

We have seen delays of more than 50ms in class or qdisc dumps, in case
device is under high TX stress, even with the prior 4KB per skb limit.

Add cond_resched() to give a chance to higher prio tasks to get cpu.

Signed-off-by; Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_api.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 272292efa7f0..0a99d7ced71e 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1303,6 +1303,7 @@ static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
 	struct gnet_dump d;
 	struct qdisc_size_table *stab;
 
+	cond_resched();
 	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*tcm), flags);
 	if (!nlh)
 		goto out_nlmsg_trim;
@@ -1615,6 +1616,7 @@ static int tc_fill_tclass(struct sk_buff *skb, struct Qdisc *q,
 	struct gnet_dump d;
 	const struct Qdisc_class_ops *cl_ops = q->ops->cl_ops;
 
+	cond_resched();
 	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*tcm), flags);
 	if (!nlh)
 		goto out_nlmsg_trim;
-- 
1.9.0.279.gdc9e3eb

Re: [PATCH net-next 2/2] pkt_sched: add cond_resched() to class and qdisc dump

[Eric Dumazet]

On Mon, 2014-03-10 at 17:11 -0700, Eric Dumazet wrote:
> We have seen delays of more than 50ms in class or qdisc dumps, in case
> device is under high TX stress, even with the prior 4KB per skb limit.
> 
> Add cond_resched() to give a chance to higher prio tasks to get cpu.
> 
> Signed-off-by; Eric Dumazet <edumazet@google.com>

Gosh I managed to mess this SOB :(

Signed-off-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net-next 0/2] pkt_sched: allow scheduling points

[David Miller]

From: Eric Dumazet <edumazet@google.com>
Date: Mon, 10 Mar 2014 17:11:41 -0700

> We have seen delays of more than 50ms in class or qdisc dumps, in case
> device is under high TX stress, even with the prior 4KB per skb limit.
> 
> With the new 16KB limit, this could translate to 200ms delays.
> 
> Add cond_resched() to give a chance to higher prio tasks to get cpu.
> 
> But before doing so, we need to remove the rcu locking from tc_dump_qdisc()
> as David spotted.

Looks good, series applied, thanks Eric.