* [PATCH net-next 0/5] sched: convert queues to single-linked list
@ 2016-09-17 22:57 Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 1/5] pie: use qdisc_dequeue_head wrapper Florian Westphal
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Florian Westphal @ 2016-09-17 22:57 UTC (permalink / raw)
To: netdev
During Netfilter Workshop 2016 Eric Dumazet pointed out that qdisc
schedulers use doubly-linked lists, even though single-linked list
would be enough.
The double-linked skb lists incur one extra write on enqueue/dequeue
operations (to change ->prev pointer of next list elem).
This series converts qdiscs to single-linked version, listhead
maintains pointers to first (for dequeue) and last skb (for enqueue).
Most qdiscs don't queue at all and instead use a leaf qdisc (typically
pfifo_fast) so only a few schedulers needed changes.
I briefly tested netem and htb and they seemed fine.
UDP_STREAM netperf with 64 byte packets via veth+pfifo_fast shows
a small (~2%) improvement.
Florian Westphal (5):
pie: use qdisc_dequeue_head wrapper
sched: don't use skb queue helpers
sched: remove qdisc arg from __qdisc_dequeue_head
sched: replace __skb_dequeue with __qdisc_dequeue_head
sched: add and use qdisc_skb_head helpers
include/net/sch_generic.h | 72 +++++++++++++++++++++++++++++++++++-----------
net/sched/sch_codel.c | 4 +-
net/sched/sch_fifo.c | 4 +-
net/sched/sch_generic.c | 30 +++++++++++--------
net/sched/sch_htb.c | 24 ++++++++++++---
net/sched/sch_netem.c | 20 +++++++++---
net/sched/sch_pie.c | 4 +-
7 files changed, 115 insertions(+), 43 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 1/5] pie: use qdisc_dequeue_head wrapper
2016-09-17 22:57 [PATCH net-next 0/5] sched: convert queues to single-linked list Florian Westphal
@ 2016-09-17 22:57 ` Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 2/5] sched: don't use skb queue helpers Florian Westphal
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2016-09-17 22:57 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
Doesn't change generated code.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/sched/sch_pie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index a570b0b..d976d74 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -511,7 +511,7 @@ static int pie_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
static struct sk_buff *pie_qdisc_dequeue(struct Qdisc *sch)
{
struct sk_buff *skb;
- skb = __qdisc_dequeue_head(sch, &sch->q);
+ skb = qdisc_dequeue_head(sch);
if (!skb)
return NULL;
--
2.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 2/5] sched: don't use skb queue helpers
2016-09-17 22:57 [PATCH net-next 0/5] sched: convert queues to single-linked list Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 1/5] pie: use qdisc_dequeue_head wrapper Florian Westphal
@ 2016-09-17 22:57 ` Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 3/5] sched: remove qdisc arg from __qdisc_dequeue_head Florian Westphal
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2016-09-17 22:57 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
A followup change will replace the sk_buff_head in the qdisc
struct with a slightly different list.
Use of the sk_buff_head helpers will thus cause compiler
warnings.
Open-code these accesses in an extra change to ease review.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/sched/sch_fifo.c | 4 ++--
net/sched/sch_generic.c | 2 +-
net/sched/sch_netem.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index baeed6a..1e37247 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -31,7 +31,7 @@ static int bfifo_enqueue(struct sk_buff *skb, struct Qdisc *sch,
static int pfifo_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
- if (likely(skb_queue_len(&sch->q) < sch->limit))
+ if (likely(sch->q.qlen < sch->limit))
return qdisc_enqueue_tail(skb, sch);
return qdisc_drop(skb, sch, to_free);
@@ -42,7 +42,7 @@ static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc *sch,
{
unsigned int prev_backlog;
- if (likely(skb_queue_len(&sch->q) < sch->limit))
+ if (likely(sch->q.qlen < sch->limit))
return qdisc_enqueue_tail(skb, sch);
prev_backlog = sch->qstats.backlog;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 0d21b56..5e63bf6 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -486,7 +486,7 @@ static inline struct sk_buff_head *band2list(struct pfifo_fast_priv *priv,
static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
struct sk_buff **to_free)
{
- if (skb_queue_len(&qdisc->q) < qdisc_dev(qdisc)->tx_queue_len) {
+ if (qdisc->q.qlen < qdisc_dev(qdisc)->tx_queue_len) {
int band = prio2band[skb->priority & TC_PRIO_MAX];
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
struct sk_buff_head *list = band2list(priv, band);
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index aaaf021..1832d77 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -502,7 +502,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
1<<(prandom_u32() % 8);
}
- if (unlikely(skb_queue_len(&sch->q) >= sch->limit))
+ if (unlikely(sch->q.qlen >= sch->limit))
return qdisc_drop(skb, sch, to_free);
qdisc_qstats_backlog_inc(sch, skb);
@@ -522,7 +522,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
if (q->rate) {
struct sk_buff *last;
- if (!skb_queue_empty(&sch->q))
+ if (sch->q.qlen)
last = skb_peek_tail(&sch->q);
else
last = netem_rb_to_skb(rb_last(&q->t_root));
--
2.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 3/5] sched: remove qdisc arg from __qdisc_dequeue_head
2016-09-17 22:57 [PATCH net-next 0/5] sched: convert queues to single-linked list Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 1/5] pie: use qdisc_dequeue_head wrapper Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 2/5] sched: don't use skb queue helpers Florian Westphal
@ 2016-09-17 22:57 ` Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 4/5] sched: replace __skb_dequeue with __qdisc_dequeue_head Florian Westphal
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2016-09-17 22:57 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
Moves qdisc stat accouting to qdisc_dequeue_head.
The only direct caller of the __qdisc_dequeue_head version open-codes
this now.
This allows us to later use __qdisc_dequeue_head as a replacement
of __skb_dequeue() (which operates on sk_buff_head list).
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/net/sch_generic.h | 15 ++++++++-------
net/sched/sch_generic.c | 7 ++++++-
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 52a2015..0741ed4 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -614,11 +614,17 @@ static inline int qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch)
return __qdisc_enqueue_tail(skb, sch, &sch->q);
}
-static inline struct sk_buff *__qdisc_dequeue_head(struct Qdisc *sch,
- struct sk_buff_head *list)
+static inline struct sk_buff *__qdisc_dequeue_head(struct sk_buff_head *list)
{
struct sk_buff *skb = __skb_dequeue(list);
+ return skb;
+}
+
+static inline struct sk_buff *qdisc_dequeue_head(struct Qdisc *sch)
+{
+ struct sk_buff *skb = __qdisc_dequeue_head(&sch->q);
+
if (likely(skb != NULL)) {
qdisc_qstats_backlog_dec(sch, skb);
qdisc_bstats_update(sch, skb);
@@ -627,11 +633,6 @@ static inline struct sk_buff *__qdisc_dequeue_head(struct Qdisc *sch,
return skb;
}
-static inline struct sk_buff *qdisc_dequeue_head(struct Qdisc *sch)
-{
- return __qdisc_dequeue_head(sch, &sch->q);
-}
-
/* Instead of calling kfree_skb() while root qdisc lock is held,
* queue the skb for future freeing at end of __dev_xmit_skb()
*/
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 5e63bf6..73877d9 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -506,7 +506,12 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc)
if (likely(band >= 0)) {
struct sk_buff_head *list = band2list(priv, band);
- struct sk_buff *skb = __qdisc_dequeue_head(qdisc, list);
+ struct sk_buff *skb = __qdisc_dequeue_head(list);
+
+ if (likely(skb != NULL)) {
+ qdisc_qstats_backlog_dec(qdisc, skb);
+ qdisc_bstats_update(qdisc, skb);
+ }
qdisc->q.qlen--;
if (skb_queue_empty(list))
--
2.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 4/5] sched: replace __skb_dequeue with __qdisc_dequeue_head
2016-09-17 22:57 [PATCH net-next 0/5] sched: convert queues to single-linked list Florian Westphal
` (2 preceding siblings ...)
2016-09-17 22:57 ` [PATCH net-next 3/5] sched: remove qdisc arg from __qdisc_dequeue_head Florian Westphal
@ 2016-09-17 22:57 ` Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 5/5] sched: add and use qdisc_skb_head helpers Florian Westphal
2016-09-19 5:47 ` [PATCH net-next 0/5] sched: convert queues to single-linked list David Miller
5 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2016-09-17 22:57 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
After previous patch these functions are identical.
Replace __skb_dequeue in qdiscs with __qdisc_dequeue_head.
Next patch will then make __qdisc_dequeue_head handle
single-linked list instead of strcut sk_buff_head argument.
Doesn't change generated code.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/sched/sch_codel.c | 4 ++--
net/sched/sch_netem.c | 2 +-
net/sched/sch_pie.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index 4002df3..5bfa79e 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c
@@ -69,7 +69,7 @@ struct codel_sched_data {
static struct sk_buff *dequeue_func(struct codel_vars *vars, void *ctx)
{
struct Qdisc *sch = ctx;
- struct sk_buff *skb = __skb_dequeue(&sch->q);
+ struct sk_buff *skb = __qdisc_dequeue_head(&sch->q);
if (skb)
sch->qstats.backlog -= qdisc_pkt_len(skb);
@@ -172,7 +172,7 @@ static int codel_change(struct Qdisc *sch, struct nlattr *opt)
qlen = sch->q.qlen;
while (sch->q.qlen > sch->limit) {
- struct sk_buff *skb = __skb_dequeue(&sch->q);
+ struct sk_buff *skb = __qdisc_dequeue_head(&sch->q);
dropped += qdisc_pkt_len(skb);
qdisc_qstats_backlog_dec(sch, skb);
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 1832d77..0a964b3 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -587,7 +587,7 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
struct rb_node *p;
tfifo_dequeue:
- skb = __skb_dequeue(&sch->q);
+ skb = __qdisc_dequeue_head(&sch->q);
if (skb) {
qdisc_qstats_backlog_dec(sch, skb);
deliver:
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index d976d74..5c3a99d 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -231,7 +231,7 @@ static int pie_change(struct Qdisc *sch, struct nlattr *opt)
/* Drop excess packets if new limit is lower */
qlen = sch->q.qlen;
while (sch->q.qlen > sch->limit) {
- struct sk_buff *skb = __skb_dequeue(&sch->q);
+ struct sk_buff *skb = __qdisc_dequeue_head(&sch->q);
dropped += qdisc_pkt_len(skb);
qdisc_qstats_backlog_dec(sch, skb);
--
2.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 5/5] sched: add and use qdisc_skb_head helpers
2016-09-17 22:57 [PATCH net-next 0/5] sched: convert queues to single-linked list Florian Westphal
` (3 preceding siblings ...)
2016-09-17 22:57 ` [PATCH net-next 4/5] sched: replace __skb_dequeue with __qdisc_dequeue_head Florian Westphal
@ 2016-09-17 22:57 ` Florian Westphal
2016-09-19 5:47 ` [PATCH net-next 0/5] sched: convert queues to single-linked list David Miller
5 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2016-09-17 22:57 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
This change replaces sk_buff_head struct in Qdiscs with new qdisc_skb_head.
Its similar to the skb_buff_head api, but does not use skb->prev pointers.
Qdiscs will commonly enqueue at the tail of a list and dequeue at head.
While skb_buff_head works fine for this, enqueue/dequeue needs to also
adjust the prev pointer of next element.
The ->prev pointer is not required for qdiscs so we can just leave
it undefined and avoid one cacheline write access for en/dequeue.
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/net/sch_generic.h | 63 ++++++++++++++++++++++++++++++++++++++---------
net/sched/sch_generic.c | 21 ++++++++--------
net/sched/sch_htb.c | 24 +++++++++++++++---
net/sched/sch_netem.c | 14 +++++++++--
4 files changed, 94 insertions(+), 28 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 0741ed4..e6aa0a2 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -36,6 +36,14 @@ struct qdisc_size_table {
u16 data[];
};
+/* similar to sk_buff_head, but skb->prev pointer is undefined. */
+struct qdisc_skb_head {
+ struct sk_buff *head;
+ struct sk_buff *tail;
+ __u32 qlen;
+ spinlock_t lock;
+};
+
struct Qdisc {
int (*enqueue)(struct sk_buff *skb,
struct Qdisc *sch,
@@ -76,7 +84,7 @@ struct Qdisc {
* For performance sake on SMP, we put highly modified fields at the end
*/
struct sk_buff *gso_skb ____cacheline_aligned_in_smp;
- struct sk_buff_head q;
+ struct qdisc_skb_head q;
struct gnet_stats_basic_packed bstats;
seqcount_t running;
struct gnet_stats_queue qstats;
@@ -600,10 +608,27 @@ static inline void qdisc_qstats_overlimit(struct Qdisc *sch)
sch->qstats.overlimits++;
}
+static inline void qdisc_skb_head_init(struct qdisc_skb_head *qh)
+{
+ qh->head = NULL;
+ qh->tail = NULL;
+ qh->qlen = 0;
+}
+
static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
- struct sk_buff_head *list)
+ struct qdisc_skb_head *qh)
{
- __skb_queue_tail(list, skb);
+ struct sk_buff *last = qh->tail;
+
+ if (last) {
+ skb->next = NULL;
+ last->next = skb;
+ qh->tail = skb;
+ } else {
+ qh->tail = skb;
+ qh->head = skb;
+ }
+ qh->qlen++;
qdisc_qstats_backlog_inc(sch, skb);
return NET_XMIT_SUCCESS;
@@ -614,9 +639,17 @@ static inline int qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch)
return __qdisc_enqueue_tail(skb, sch, &sch->q);
}
-static inline struct sk_buff *__qdisc_dequeue_head(struct sk_buff_head *list)
+static inline struct sk_buff *__qdisc_dequeue_head(struct qdisc_skb_head *qh)
{
- struct sk_buff *skb = __skb_dequeue(list);
+ struct sk_buff *skb = qh->head;
+
+ if (likely(skb != NULL)) {
+ qh->head = skb->next;
+ qh->qlen--;
+ if (qh->head == NULL)
+ qh->tail = NULL;
+ skb->next = NULL;
+ }
return skb;
}
@@ -643,10 +676,10 @@ static inline void __qdisc_drop(struct sk_buff *skb, struct sk_buff **to_free)
}
static inline unsigned int __qdisc_queue_drop_head(struct Qdisc *sch,
- struct sk_buff_head *list,
+ struct qdisc_skb_head *qh,
struct sk_buff **to_free)
{
- struct sk_buff *skb = __skb_dequeue(list);
+ struct sk_buff *skb = __qdisc_dequeue_head(qh);
if (likely(skb != NULL)) {
unsigned int len = qdisc_pkt_len(skb);
@@ -667,7 +700,9 @@ static inline unsigned int qdisc_queue_drop_head(struct Qdisc *sch,
static inline struct sk_buff *qdisc_peek_head(struct Qdisc *sch)
{
- return skb_peek(&sch->q);
+ const struct qdisc_skb_head *qh = &sch->q;
+
+ return qh->head;
}
/* generic pseudo peek method for non-work-conserving qdisc */
@@ -702,15 +737,19 @@ static inline struct sk_buff *qdisc_dequeue_peeked(struct Qdisc *sch)
return skb;
}
-static inline void __qdisc_reset_queue(struct sk_buff_head *list)
+static inline void __qdisc_reset_queue(struct qdisc_skb_head *qh)
{
/*
* We do not know the backlog in bytes of this list, it
* is up to the caller to correct it
*/
- if (!skb_queue_empty(list)) {
- rtnl_kfree_skbs(list->next, list->prev);
- __skb_queue_head_init(list);
+ ASSERT_RTNL();
+ if (qh->qlen) {
+ rtnl_kfree_skbs(qh->head, qh->tail);
+
+ qh->head = NULL;
+ qh->tail = NULL;
+ qh->qlen = 0;
}
}
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 73877d9..6cfb6e9 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -466,7 +466,7 @@ static const u8 prio2band[TC_PRIO_MAX + 1] = {
*/
struct pfifo_fast_priv {
u32 bitmap;
- struct sk_buff_head q[PFIFO_FAST_BANDS];
+ struct qdisc_skb_head q[PFIFO_FAST_BANDS];
};
/*
@@ -477,7 +477,7 @@ struct pfifo_fast_priv {
*/
static const int bitmap2band[] = {-1, 0, 1, 0, 2, 0, 1, 0};
-static inline struct sk_buff_head *band2list(struct pfifo_fast_priv *priv,
+static inline struct qdisc_skb_head *band2list(struct pfifo_fast_priv *priv,
int band)
{
return priv->q + band;
@@ -489,7 +489,7 @@ static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
if (qdisc->q.qlen < qdisc_dev(qdisc)->tx_queue_len) {
int band = prio2band[skb->priority & TC_PRIO_MAX];
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
- struct sk_buff_head *list = band2list(priv, band);
+ struct qdisc_skb_head *list = band2list(priv, band);
priv->bitmap |= (1 << band);
qdisc->q.qlen++;
@@ -505,8 +505,8 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc)
int band = bitmap2band[priv->bitmap];
if (likely(band >= 0)) {
- struct sk_buff_head *list = band2list(priv, band);
- struct sk_buff *skb = __qdisc_dequeue_head(list);
+ struct qdisc_skb_head *qh = band2list(priv, band);
+ struct sk_buff *skb = __qdisc_dequeue_head(qh);
if (likely(skb != NULL)) {
qdisc_qstats_backlog_dec(qdisc, skb);
@@ -514,7 +514,7 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc)
}
qdisc->q.qlen--;
- if (skb_queue_empty(list))
+ if (qh->qlen == 0)
priv->bitmap &= ~(1 << band);
return skb;
@@ -529,9 +529,9 @@ static struct sk_buff *pfifo_fast_peek(struct Qdisc *qdisc)
int band = bitmap2band[priv->bitmap];
if (band >= 0) {
- struct sk_buff_head *list = band2list(priv, band);
+ struct qdisc_skb_head *qh = band2list(priv, band);
- return skb_peek(list);
+ return qh->head;
}
return NULL;
@@ -569,7 +569,7 @@ static int pfifo_fast_init(struct Qdisc *qdisc, struct nlattr *opt)
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
for (prio = 0; prio < PFIFO_FAST_BANDS; prio++)
- __skb_queue_head_init(band2list(priv, prio));
+ qdisc_skb_head_init(band2list(priv, prio));
/* Can by-pass the queue discipline */
qdisc->flags |= TCQ_F_CAN_BYPASS;
@@ -617,7 +617,8 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
sch = (struct Qdisc *) QDISC_ALIGN((unsigned long) p);
sch->padded = (char *) sch - (char *) p;
}
- skb_queue_head_init(&sch->q);
+ qdisc_skb_head_init(&sch->q);
+ spin_lock_init(&sch->q.lock);
spin_lock_init(&sch->busylock);
lockdep_set_class(&sch->busylock,
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 53dbfa1..c798d0d 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -162,7 +162,7 @@ struct htb_sched {
struct work_struct work;
/* non shaped skbs; let them go directly thru */
- struct sk_buff_head direct_queue;
+ struct qdisc_skb_head direct_queue;
long direct_pkts;
struct qdisc_watchdog watchdog;
@@ -570,6 +570,22 @@ static inline void htb_deactivate(struct htb_sched *q, struct htb_class *cl)
list_del_init(&cl->un.leaf.drop_list);
}
+static void htb_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
+ struct qdisc_skb_head *qh)
+{
+ struct sk_buff *last = qh->tail;
+
+ if (last) {
+ skb->next = NULL;
+ last->next = skb;
+ qh->tail = skb;
+ } else {
+ qh->tail = skb;
+ qh->head = skb;
+ }
+ qh->qlen++;
+}
+
static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
@@ -580,7 +596,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
if (cl == HTB_DIRECT) {
/* enqueue to helper queue */
if (q->direct_queue.qlen < q->direct_qlen) {
- __skb_queue_tail(&q->direct_queue, skb);
+ htb_enqueue_tail(skb, sch, &q->direct_queue);
q->direct_pkts++;
} else {
return qdisc_drop(skb, sch, to_free);
@@ -888,7 +904,7 @@ static struct sk_buff *htb_dequeue(struct Qdisc *sch)
unsigned long start_at;
/* try to dequeue direct packets as high prio (!) to minimize cpu work */
- skb = __skb_dequeue(&q->direct_queue);
+ skb = __qdisc_dequeue_head(&q->direct_queue);
if (skb != NULL) {
ok:
qdisc_bstats_update(sch, skb);
@@ -1019,7 +1035,7 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt)
qdisc_watchdog_init(&q->watchdog, sch);
INIT_WORK(&q->work, htb_work_func);
- __skb_queue_head_init(&q->direct_queue);
+ qdisc_skb_head_init(&q->direct_queue);
if (tb[TCA_HTB_DIRECT_QLEN])
q->direct_qlen = nla_get_u32(tb[TCA_HTB_DIRECT_QLEN]);
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 0a964b3..9f7b380 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -413,6 +413,16 @@ static struct sk_buff *netem_segment(struct sk_buff *skb, struct Qdisc *sch,
return segs;
}
+static void netem_enqueue_skb_head(struct qdisc_skb_head *qh, struct sk_buff *skb)
+{
+ skb->next = qh->head;
+
+ if (!qh->head)
+ qh->tail = skb;
+ qh->head = skb;
+ qh->qlen++;
+}
+
/*
* Insert one skb into qdisc.
* Note: parent depends on return value to account for queue length.
@@ -523,7 +533,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff *last;
if (sch->q.qlen)
- last = skb_peek_tail(&sch->q);
+ last = sch->q.tail;
else
last = netem_rb_to_skb(rb_last(&q->t_root));
if (last) {
@@ -552,7 +562,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
cb->time_to_send = psched_get_time();
q->counter = 0;
- __skb_queue_head(&sch->q, skb);
+ netem_enqueue_skb_head(&sch->q, skb);
sch->qstats.requeues++;
}
--
2.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 0/5] sched: convert queues to single-linked list
2016-09-17 22:57 [PATCH net-next 0/5] sched: convert queues to single-linked list Florian Westphal
` (4 preceding siblings ...)
2016-09-17 22:57 ` [PATCH net-next 5/5] sched: add and use qdisc_skb_head helpers Florian Westphal
@ 2016-09-19 5:47 ` David Miller
5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2016-09-19 5:47 UTC (permalink / raw)
To: fw; +Cc: netdev
From: Florian Westphal <fw@strlen.de>
Date: Sun, 18 Sep 2016 00:57:29 +0200
> During Netfilter Workshop 2016 Eric Dumazet pointed out that qdisc
> schedulers use doubly-linked lists, even though single-linked list
> would be enough.
>
> The double-linked skb lists incur one extra write on enqueue/dequeue
> operations (to change ->prev pointer of next list elem).
>
> This series converts qdiscs to single-linked version, listhead
> maintains pointers to first (for dequeue) and last skb (for enqueue).
>
> Most qdiscs don't queue at all and instead use a leaf qdisc (typically
> pfifo_fast) so only a few schedulers needed changes.
>
> I briefly tested netem and htb and they seemed fine.
>
> UDP_STREAM netperf with 64 byte packets via veth+pfifo_fast shows
> a small (~2%) improvement.
Series applied, thanks Florian.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-09-19 5:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-17 22:57 [PATCH net-next 0/5] sched: convert queues to single-linked list Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 1/5] pie: use qdisc_dequeue_head wrapper Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 2/5] sched: don't use skb queue helpers Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 3/5] sched: remove qdisc arg from __qdisc_dequeue_head Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 4/5] sched: replace __skb_dequeue with __qdisc_dequeue_head Florian Westphal
2016-09-17 22:57 ` [PATCH net-next 5/5] sched: add and use qdisc_skb_head helpers Florian Westphal
2016-09-19 5:47 ` [PATCH net-next 0/5] sched: convert queues to single-linked list David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).