Skip to contentSkip to conversation

pkt_sched: sch_htb: Clean htb_class prio and quantum fields

PATCH Applied Jarek Poplawski 2 participants 10 messages Dec 2, 2008 – Dec 4, 2008

Subsystems (3)
Networking Network drivers Traffic control (tc)
lore
Share & tools
lore

Jarek Poplawski

author
#0 lore raw

PATCH 3/3 pkt_sched: sch_htb: Clean htb_class prio and quantum fields

While implementing htb_parent_to_leaf() there where added backup prio and quantum struct htb_class fields to preserve these values for inner classes in case of their return to leaf. This patch cleans this a bit by removing union leaf duplicates.

Signed-off-by Jarek Poplawski <[email protected]>
---
 net/sched/sch_htb.c |   41 ++++++++++++++++-------------------------
 1 files changed, 16 insertions(+), 25 deletions(-)
net/sched/sch_htb.c +16 -25
index 7a71e94..80cb94d 100644
@@ -84,11 +84,12 @@ struct htb_class {
 	unsigned int children;
 	struct htb_class *parent;	/* parent class */
 
+	int prio;		/* these two are used only by leaves... */
+	int quantum;		/* but stored for parent-to-leaf return */
+
 	union {
 		struct htb_class_leaf {
 			struct Qdisc *q;
-			int prio;
-			int quantum;
 			int deficit[TC_HTB_MAXDEPTH];
 			struct list_head drop_list;
 		} leaf;

@@ -122,10 +123,6 @@ struct htb_class {
 	psched_tdiff_t mbuffer;	/* max wait time */
 	long tokens, ctokens;	/* current number of tokens */
 	psched_time_t t_c;	/* checkpoint time */
-
-	int prio;		/* For parent to leaf return possible here */
-	int quantum;		/* we do backup. Finally full replacement  */
-				/* of un.leaf originals should be done. */
 };
 
 static inline long L2T(struct htb_class *cl, struct qdisc_rate_table *rate,

@@ -523,10 +520,10 @@ static inline void htb_activate(struct htb_sched *q, struct htb_class *cl)
 	WARN_ON(cl->level || !cl->un.leaf.q || !cl->un.leaf.q->q.qlen);
 
 	if (!cl->prio_activity) {
-		cl->prio_activity = 1 << cl->un.leaf.prio;
+		cl->prio_activity = 1 << cl->prio;
 		htb_activate_prios(q, cl);
 		list_add_tail(&cl->un.leaf.drop_list,
-			      q->drops + cl->un.leaf.prio);
+			      q->drops + cl->prio);
 	}
 }
 

@@ -816,7 +813,7 @@ next:
 	if (likely(skb != NULL)) {
 		cl->un.leaf.deficit[level] -= qdisc_pkt_len(skb);
 		if (cl->un.leaf.deficit[level] < 0) {
-			cl->un.leaf.deficit[level] += cl->un.leaf.quantum;
+			cl->un.leaf.deficit[level] += cl->quantum;
 			htb_next_rb_node((level ? cl->parent->un.inner.ptr : q->
 					  ptr[0]) + prio);
 		}

@@ -1050,8 +1047,8 @@ static int htb_dump_class(struct Qdisc *sch, unsigned long arg,
 	opt.buffer = cl->buffer;
 	opt.ceil = cl->ceil->rate;
 	opt.cbuffer = cl->cbuffer;
-	opt.quantum = cl->un.leaf.quantum;
-	opt.prio = cl->un.leaf.prio;
+	opt.quantum = cl->quantum;
+	opt.prio = cl->prio;
 	opt.level = cl->level;
 	NLA_PUT(skb, TCA_HTB_PARMS, sizeof(opt), &opt);
 

@@ -1155,8 +1152,6 @@ static void htb_parent_to_leaf(struct htb_sched *q, struct htb_class *cl,
 	memset(&parent->un.inner, 0, sizeof(parent->un.inner));
 	INIT_LIST_HEAD(&parent->un.leaf.drop_list);
 	parent->un.leaf.q = new_q ? new_q : &noop_qdisc;
-	parent->un.leaf.quantum = parent->quantum;
-	parent->un.leaf.prio = parent->prio;
 	parent->tokens = parent->buffer;
 	parent->ctokens = parent->cbuffer;
 	parent->t_c = psched_get_time();

@@ -1400,27 +1395,23 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 	/* it used to be a nasty bug here, we have to check that node
 	   is really leaf before changing cl->un.leaf ! */
 	if (!cl->level) {
-		cl->un.leaf.quantum = rtab->rate.rate / q->rate2quantum;
-		if (!hopt->quantum && cl->un.leaf.quantum < 1000) {
+		cl->quantum = rtab->rate.rate / q->rate2quantum;
+		if (!hopt->quantum && cl->quantum < 1000) {
 			printk(KERN_WARNING
 			       "HTB: quantum of class %X is small. Consider r2q change.\n",
 			       cl->common.classid);
-			cl->un.leaf.quantum = 1000;
+			cl->quantum = 1000;
 		}
-		if (!hopt->quantum && cl->un.leaf.quantum > 200000) {
+		if (!hopt->quantum && cl->quantum > 200000) {
 			printk(KERN_WARNING
 			       "HTB: quantum of class %X is big. Consider r2q change.\n",
 			       cl->common.classid);
-			cl->un.leaf.quantum = 200000;
+			cl->quantum = 200000;
 		}
 		if (hopt->quantum)
-			cl->un.leaf.quantum = hopt->quantum;
-		if ((cl->un.leaf.prio = hopt->prio) >= TC_HTB_NUMPRIO)
-			cl->un.leaf.prio = TC_HTB_NUMPRIO - 1;
-
-		/* backup for htb_parent_to_leaf */
-		cl->quantum = cl->un.leaf.quantum;
-		cl->prio = cl->un.leaf.prio;
+			cl->quantum = hopt->quantum;
+		if ((cl->prio = hopt->prio) >= TC_HTB_NUMPRIO)
+			cl->prio = TC_HTB_NUMPRIO - 1;
 	}
 
 	cl->buffer = hopt->buffer;
Signature · 1 line
1.5.6.5
author
#1 lore raw

[PATCH 4/3] pkt_sched: sch_htb: Clean L2T()

L2T() is currently used only in one place, so let's move it closer to this place, remove unused cl parameter, and change to a macro.

Signed-off-by Jarek Poplawski <[email protected]>

---

 net/sched/sch_htb.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)
net/sched/sch_htb.c +3 -8
index 80cb94d..78f20d6 100644
@@ -125,13 +125,6 @@ struct htb_class {
 	psched_time_t t_c;	/* checkpoint time */
 };
 
-static inline long L2T(struct htb_class *cl, struct qdisc_rate_table *rate,
-			   int size)
-{
-	long result = qdisc_l2t(rate, size);
-	return result;
-}
-
 struct htb_sched {
 	struct Qdisc_class_hash clhash;
 	struct list_head drops[TC_HTB_NUMPRIO];/* active leaves (for drops) */

@@ -602,9 +595,11 @@ static void htb_charge_class(struct htb_sched *q, struct htb_class *cl,
 	long toks, diff;
 	enum htb_cmode old_mode;
 
+#define L2T(rate, size)	((long) qdisc_l2t(rate, size))
+
 #define HTB_ACCNT(T,B,R) toks = diff + cl->T; \
 	if (toks > cl->B) toks = cl->B; \
-	toks -= L2T(cl, cl->R, bytes); \
+	toks -= L2T(cl->R, bytes); \
 	if (toks <= -cl->mbuffer) toks = 1-cl->mbuffer; \
 	cl->T = toks
maintainer
#2 lore raw

Re: [PATCH 4/3] pkt_sched: sch_htb: Clean L2T()

From: Jarek Poplawski <[email protected]>
Date: Wed, 3 Dec 2008 07:09:47 +0000

Quoted 4 lines

L2T() is currently used only in one place, so let's move it closer to this place, remove unused cl parameter, and change to a macro.

Signed-off-by Jarek Poplawski <[email protected]>

Jarek, please just kill the macro and expand the calculation inline.

author
#3 lore raw

Re: [PATCH 4/3 v2] pkt_sched: sch_htb: Clean L2T()

On Tue, Dec 02, 2008 at 11:25:22PM -0800, David Miller wrote:
...

Quoted 1 line

Jarek, please just kill the macro and expand the calculation inline.

Hmm.. no problem... unless you think about the other macro?

Jarek P.
--------------->
pkt_sched: sch_htb: Clean L2T()

L2T() is currently used only in one place, so let's move it closer to this place, remove unused cl parameter.

Signed-off-by Jarek Poplawski <[email protected]>

---

 net/sched/sch_htb.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)
net/sched/sch_htb.c +7 -8
index 80cb94d..4e9b8f4 100644
@@ -125,13 +125,6 @@ struct htb_class {
 	psched_time_t t_c;	/* checkpoint time */
 };
 
-static inline long L2T(struct htb_class *cl, struct qdisc_rate_table *rate,
-			   int size)
-{
-	long result = qdisc_l2t(rate, size);
-	return result;
-}
-
 struct htb_sched {
 	struct Qdisc_class_hash clhash;
 	struct list_head drops[TC_HTB_NUMPRIO];/* active leaves (for drops) */

@@ -584,6 +577,12 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	return NET_XMIT_SUCCESS;
 }
 
+static inline long L2T(struct qdisc_rate_table *rate, int size)
+{
+	long result = qdisc_l2t(rate, size);
+	return result;
+}
+
 /**
  * htb_charge_class - charges amount "bytes" to leaf and ancestors
  *

@@ -604,7 +603,7 @@ static void htb_charge_class(struct htb_sched *q, struct htb_class *cl,
 
 #define HTB_ACCNT(T,B,R) toks = diff + cl->T; \
 	if (toks > cl->B) toks = cl->B; \
-	toks -= L2T(cl, cl->R, bytes); \
+	toks -= L2T(cl->R, bytes); \
 	if (toks <= -cl->mbuffer) toks = 1-cl->mbuffer; \
 	cl->T = toks
maintainer
#4 lore raw

Re: [PATCH 4/3 v2] pkt_sched: sch_htb: Clean L2T()

From: Jarek Poplawski <[email protected]>
Date: Wed, 3 Dec 2008 07:54:33 +0000

Quoted 5 lines · David Miller

...

Quoted 1 line

Jarek, please just kill the macro and expand the calculation inline.

Hmm.. no problem... unless you think about the other macro?

You misunderstood me.

I meant get rid of L2T completely, and just use
"qdisc_l2t(rate, size)" directly.

Macros like this one:

1) are used in only one spot
2) give no new information to the reader

so are just extra noise.

author
#5 lore raw

Re: [PATCH 4/3 v3] pkt_sched: sch_htb: Clean L2T()

On Wed, Dec 03, 2008 at 12:20:46AM -0800, David Miller wrote:
...

Quoted 11 lines

You misunderstood me.

I meant get rid of L2T completely, and just use
"qdisc_l2t(rate, size)" directly.

Macros like this one:

1) are used in only one spot
2) give no new information to the reader

so are just extra noise.

Hmm.. I think L2T is special in sched, but no problem...

Thanks for the explanation,
Jarek P.
------------------> (take 3)
pkt_sched: sch_htb: Remove L2T()

L2T() is currently used only in one place (and has one spurious parameter, btw), so let's: 'get rid of L2T completely, and just use "qdisc_l2t(rate, size)" directly.' - quote & feedback from David S. Miller.

Signed-off-by Jarek Poplawski <[email protected]>

---

 net/sched/sch_htb.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)
net/sched/sch_htb.c +1 -8
index 80cb94d..fcd06e2 100644
@@ -125,13 +125,6 @@ struct htb_class {
 	psched_time_t t_c;	/* checkpoint time */
 };
 
-static inline long L2T(struct htb_class *cl, struct qdisc_rate_table *rate,
-			   int size)
-{
-	long result = qdisc_l2t(rate, size);
-	return result;
-}
-
 struct htb_sched {
 	struct Qdisc_class_hash clhash;
 	struct list_head drops[TC_HTB_NUMPRIO];/* active leaves (for drops) */

@@ -604,7 +597,7 @@ static void htb_charge_class(struct htb_sched *q, struct htb_class *cl,
 
 #define HTB_ACCNT(T,B,R) toks = diff + cl->T; \
 	if (toks > cl->B) toks = cl->B; \
-	toks -= L2T(cl, cl->R, bytes); \
+	toks -= (long) qdisc_l2t(cl->R, bytes); \
 	if (toks <= -cl->mbuffer) toks = 1-cl->mbuffer; \
 	cl->T = toks
author
#7 lore raw

[PATCH 5/3] pkt_sched: sch_htb: Replace HTB_ACCNT() macro with inlines

In reply to Jarek Poplawski

Replace HTB_ACCNT() macro with inlines to make it more readable.

Signed-off-by Jarek Poplawski <[email protected]>

---

 net/sched/sch_htb.c |   38 +++++++++++++++++++++++++++++---------
 1 files changed, 29 insertions(+), 9 deletions(-)
net/sched/sch_htb.c +29 -9
index fcd06e2..f89fd71 100644
@@ -577,6 +577,32 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	return NET_XMIT_SUCCESS;
 }
 
+static inline void htb_accnt_tokens(struct htb_class *cl, int bytes, long diff)
+{
+	long toks = diff + cl->tokens;
+
+	if (toks > cl->buffer)
+		toks = cl->buffer;
+	toks -= (long) qdisc_l2t(cl->rate, bytes);
+	if (toks <= -cl->mbuffer)
+		toks = 1 - cl->mbuffer;
+
+	cl->tokens = toks;
+}
+
+static inline void htb_accnt_ctokens(struct htb_class *cl, int bytes, long diff)
+{
+	long toks = diff + cl->ctokens;
+
+	if (toks > cl->cbuffer)
+		toks = cl->cbuffer;
+	toks -= (long) qdisc_l2t(cl->ceil, bytes);
+	if (toks <= -cl->mbuffer)
+		toks = 1 - cl->mbuffer;
+
+	cl->ctokens = toks;
+}
+
 /**
  * htb_charge_class - charges amount "bytes" to leaf and ancestors
  *

@@ -592,26 +618,20 @@ static void htb_charge_class(struct htb_sched *q, struct htb_class *cl,
 			     int level, struct sk_buff *skb)
 {
 	int bytes = qdisc_pkt_len(skb);
-	long toks, diff;
 	enum htb_cmode old_mode;
-
-#define HTB_ACCNT(T,B,R) toks = diff + cl->T; \
-	if (toks > cl->B) toks = cl->B; \
-	toks -= (long) qdisc_l2t(cl->R, bytes); \
-	if (toks <= -cl->mbuffer) toks = 1-cl->mbuffer; \
-	cl->T = toks
+	long diff;
 
 	while (cl) {
 		diff = psched_tdiff_bounded(q->now, cl->t_c, cl->mbuffer);
 		if (cl->level >= level) {
 			if (cl->level == level)
 				cl->xstats.lends++;
-			HTB_ACCNT(tokens, buffer, rate);
+			htb_accnt_tokens(cl, bytes, diff);
 		} else {
 			cl->xstats.borrows++;
 			cl->tokens += diff;	/* we moved t_c; update tokens */
 		}
-		HTB_ACCNT(ctokens, cbuffer, ceil);
+		htb_accnt_ctokens(cl, bytes, diff);
 		cl->t_c = q->now;
 
 		old_mode = cl->cmode;
maintainer
#9 lore raw
In reply to Jarek Poplawski

From: Jarek Poplawski <[email protected]>
Date: Tue, 2 Dec 2008 08:56:48 +0000

Quoted 6 lines

While implementing htb_parent_to_leaf() there where added backup prio and quantum struct htb_class fields to preserve these values for inner classes in case of their return to leaf. This patch cleans this a bit by removing union leaf duplicates.

Signed-off-by Jarek Poplawski <[email protected]>

Also applied, thanks Jarek.

maintainer Applied
#6 lore raw

Re: [PATCH 4/3 v3] pkt_sched: sch_htb: Clean L2T()

In reply to Jarek Poplawski

From: Jarek Poplawski <[email protected]>
Date: Wed, 3 Dec 2008 08:50:06 +0000

Quoted 8 lines

pkt_sched: sch_htb: Remove L2T()

L2T() is currently used only in one place (and has one spurious parameter, btw), so let's: 'get rid of L2T completely, and just use "qdisc_l2t(rate, size)" directly.' - quote & feedback from David S. Miller.

Signed-off-by Jarek Poplawski <[email protected]>

Applied.