[PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[Jarek Poplawski]

Here are some fixes proposals suggested by Patrick McHardy.

[NET_SCHED] sch_cbq:

- deactivating of active classes when grafting

- purging of queue/q.qlen adjustment when deleting an active class

- deactivating of active classes when q.qlen drops to zero in ->drop()

- a redundant instruction removed from cbq_deactivate_class (my own
  suggestion)

PS: - purging of queue and deactivating of active classes
when attaching a new child - not done (according to man, CBQ can carry
packets in any type of nodes).


Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
---

diff -Nurp linux-2.6.19-rc6-/net/sched/sch_cbq.c linux-2.6.19-rc6/net/sched/sch_cbq.c
--- linux-2.6.19-rc6-/net/sched/sch_cbq.c	2006-09-20 05:42:06.000000000 +0200
+++ linux-2.6.19-rc6/net/sched/sch_cbq.c	2006-11-26 15:10:56.000000000 +0100
@@ -371,8 +371,6 @@ static void cbq_deactivate_class(struct 
 					return;
 				}
 			}
-
-			cl = cl_prev->next_alive;
 			return;
 		}
 	} while ((cl_prev = cl) != q->active[prio]);
@@ -1258,6 +1256,8 @@ static unsigned int cbq_drop(struct Qdis
 		do {
 			if (cl->q->ops->drop && (len = cl->q->ops->drop(cl->q))) {
 				sch->q.qlen--;
+				if (!cl->q->q.qlen)
+					cbq_deactivate_class(cl);
 				return len;
 			}
 		} while ((cl = cl->next_alive) != cl_head);
@@ -1683,8 +1683,10 @@ static int cbq_graft(struct Qdisc *sch, 
 #endif
 		}
 		sch_tree_lock(sch);
-		*old = cl->q;
-		cl->q = new;
+		if (cl->next_alive)
+			cbq_deactivate_class(cl);
+
+		*old = xchg(&cl->q, new);
 		sch->q.qlen -= (*old)->q.qlen;
 		qdisc_reset(*old);
 		sch_tree_unlock(sch);
@@ -1992,6 +1994,9 @@ static int cbq_delete(struct Qdisc *sch,
 
 	sch_tree_lock(sch);
 
+	sch->q.qlen -= cl->q->q.qlen;
+	qdisc_reset(cl->q);
+
 	if (cl->next_alive)
 		cbq_deactivate_class(cl);
 

Re: [PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[Patrick McHardy]

Jarek Poplawski wrote:
> Here are some fixes proposals suggested by Patrick McHardy.
> 
> [NET_SCHED] sch_cbq:
> 
> - deactivating of active classes when grafting
> 
> - purging of queue/q.qlen adjustment when deleting an active class
> 
> - deactivating of active classes when q.qlen drops to zero in ->drop()
> 
> - a redundant instruction removed from cbq_deactivate_class (my own
>   suggestion)
> 
> PS: - purging of queue and deactivating of active classes
> when attaching a new child - not done (according to man, CBQ can carry
> packets in any type of nodes).


Your patch looks good, but it conflicts with my patches.

One thing I forgot to mention is that it should also have
a callback for deactivating classes when their childs
become empty. If you do that you can just call
disc_tree_decrement_qlen in graft/delete, which will take
care of the qlen decrement and class deactivation.

Re: [PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[Patrick McHardy]

Jarek Poplawski wrote:
> On Mon, Nov 27, 2006 at 10:55:39AM +0100, Patrick McHardy wrote:
> 
>>Your patch looks good, but it conflicts with my patches.
> 
> 
> I know, but I wasn't sure which version this changes
> are needed for. If there will be something more to do,
> name the version, please. 

Just add it on top of them. They apply cleanly to the current
-git tree.

>>One thing I forgot to mention is that it should also have
>>a callback for deactivating classes when their childs
>>become empty. If you do that you can just call
>>disc_tree_decrement_qlen in graft/delete, which will take
>>care of the qlen decrement and class deactivation.
> 
>  
> If I understand correctly I should apply this to 
> the version after your patch (plus the missing cbq part
> of "endless loops"). I'll try.

Thanks.

Re: [PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[Jarek Poplawski]

On Mon, Nov 27, 2006 at 10:55:39AM +0100, Patrick McHardy wrote:
> Jarek Poplawski wrote:
> > Here are some fixes proposals suggested by Patrick McHardy.
> > 
> > [NET_SCHED] sch_cbq:
> > 
> > - deactivating of active classes when grafting
> > 
> > - purging of queue/q.qlen adjustment when deleting an active class
> > 
> > - deactivating of active classes when q.qlen drops to zero in ->drop()
> > 
> > - a redundant instruction removed from cbq_deactivate_class (my own
> >   suggestion)
> > 
> > PS: - purging of queue and deactivating of active classes
> > when attaching a new child - not done (according to man, CBQ can carry
> > packets in any type of nodes).
> 
> 
> Your patch looks good, but it conflicts with my patches.

I know, but I wasn't sure which version this changes
are needed for. If there will be something more to do,
name the version, please. 
 
> One thing I forgot to mention is that it should also have
> a callback for deactivating classes when their childs
> become empty. If you do that you can just call
> disc_tree_decrement_qlen in graft/delete, which will take
> care of the qlen decrement and class deactivation.
 
If I understand correctly I should apply this to 
the version after your patch (plus the missing cbq part
of "endless loops"). I'll try.

[PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[Jarek Poplawski]

[NET_SCHED] sch_cbq:

[PATCH 2.6.19-rc6 with "Fix endless loops" set of patches]

- P. McHardy's "Fix endless loops" patch supplement
  (cbq_graft, cbq_qlen_notify, cbq_delete, cbq_class_ops)

- deactivating of active classes when q.qlen drops to zero
  (cbq_drop)

- a redundant instruction removed from cbq_deactivate_class

PS: probably htb_deactivate in htb_delete and
cbq_deactivate_class in cbq_delete are also
redundant now.

Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
---

diff -Nurp linux-2.6.19-rc6-endless-/net/sched/sch_cbq.c linux-2.6.19-rc6-endless/net/sched/sch_cbq.c
--- linux-2.6.19-rc6-endless-/net/sched/sch_cbq.c	2006-11-27 18:40:16.000000000 +0100
+++ linux-2.6.19-rc6-endless/net/sched/sch_cbq.c	2006-11-27 20:31:46.000000000 +0100
@@ -371,8 +371,6 @@ static void cbq_deactivate_class(struct 
 					return;
 				}
 			}
-
-			cl = cl_prev->next_alive;
 			return;
 		}
 	} while ((cl_prev = cl) != q->active[prio]);
@@ -1258,6 +1256,8 @@ static unsigned int cbq_drop(struct Qdis
 		do {
 			if (cl->q->ops->drop && (len = cl->q->ops->drop(cl->q))) {
 				sch->q.qlen--;
+				if (!cl->q->q.qlen)
+					cbq_deactivate_class(cl);
 				return len;
 			}
 		} while ((cl = cl->next_alive) != cl_head);
@@ -1685,8 +1685,7 @@ static int cbq_graft(struct Qdisc *sch, 
 #endif
 		}
 		sch_tree_lock(sch);
-		*old = cl->q;
-		cl->q = new;
+		*old = xchg(&cl->q, new);
 		qdisc_tree_decrease_qlen(*old, (*old)->q.qlen);
 		qdisc_reset(*old);
 		sch_tree_unlock(sch);
@@ -1704,6 +1703,14 @@ cbq_leaf(struct Qdisc *sch, unsigned lon
 	return cl ? cl->q : NULL;
 }
 
+static void cbq_qlen_notify(struct Qdisc *sch, unsigned long arg)
+{
+	struct cbq_class *cl = (struct cbq_class *)arg;
+
+	if (cl->q->q.qlen == 0)
+		cbq_deactivate_class(cl);
+}
+
 static unsigned long cbq_get(struct Qdisc *sch, u32 classid)
 {
 	struct cbq_sched_data *q = qdisc_priv(sch);
@@ -1988,12 +1995,17 @@ static int cbq_delete(struct Qdisc *sch,
 {
 	struct cbq_sched_data *q = qdisc_priv(sch);
 	struct cbq_class *cl = (struct cbq_class*)arg;
+	unsigned int qlen;
 
 	if (cl->filters || cl->children || cl == &q->link)
 		return -EBUSY;
 
 	sch_tree_lock(sch);
 
+	qlen = cl->q->q.qlen;
+	qdisc_reset(cl->q);
+	qdisc_tree_decrease_qlen(cl->q, qlen);
+
 	if (cl->next_alive)
 		cbq_deactivate_class(cl);
 
@@ -2084,6 +2096,7 @@ static void cbq_walk(struct Qdisc *sch, 
 static struct Qdisc_class_ops cbq_class_ops = {
 	.graft		=	cbq_graft,
 	.leaf		=	cbq_leaf,
+	.qlen_notify	=	cbq_qlen_notify,
 	.get		=	cbq_get,
 	.put		=	cbq_put,
 	.change		=	cbq_change_class,

Re: [PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[Patrick McHardy]

Jarek Poplawski wrote:
> [NET_SCHED] sch_cbq:
> 
> [PATCH 2.6.19-rc6 with "Fix endless loops" set of patches]
> 
> - P. McHardy's "Fix endless loops" patch supplement
>   (cbq_graft, cbq_qlen_notify, cbq_delete, cbq_class_ops)
> 
> - deactivating of active classes when q.qlen drops to zero
>   (cbq_drop)
> 
> - a redundant instruction removed from cbq_deactivate_class
> 
> PS: probably htb_deactivate in htb_delete and
> cbq_deactivate_class in cbq_delete are also
> redundant now.

This looks good, thanks.

Re: [PATCH][NET_SCHED] sch_cbq: deactivating when grafting, purging etc.

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 30 Nov 2006 13:22:47 +0100

> Jarek Poplawski wrote:
> > [NET_SCHED] sch_cbq:
> > 
> > [PATCH 2.6.19-rc6 with "Fix endless loops" set of patches]
> > 
> > - P. McHardy's "Fix endless loops" patch supplement
> >   (cbq_graft, cbq_qlen_notify, cbq_delete, cbq_class_ops)
> > 
> > - deactivating of active classes when q.qlen drops to zero
> >   (cbq_drop)
> > 
> > - a redundant instruction removed from cbq_deactivate_class
> > 
> > PS: probably htb_deactivate in htb_delete and
> > cbq_deactivate_class in cbq_delete are also
> > redundant now.
> 
> This looks good, thanks.

Applied, thanks everyone.