Subject: Re: 2.6.8-rc4-bk1 problem: unregister_netdevice: waiting for ppp0 to become free. Usage count = 1

From: Patrick McHardy <[email-masked]>
Date: Sun, 22 Aug 2004 14:39:57 +0200

Herbert Xu wrote:

>Nuno Silva <[email-masked]> wrote:
>  
>
>>The problem is in the QoS code. If I start ppp whithout the 
>>    
>>
>
>OK, this appears to be due to the changeset titled
>
>[PKT_SCHED]: Refcount qdisc->dev for __qdisc_destroy rcu-callback
>
>It adds a reference to dev.
>
>I don't see any code that cleans up that reference when the dev goes
>down.  So someone needs to add that similar to the code in net/core/dst.c.
>
>Patrick, could you please have a look at this?
>  
>
The reference is dropped in __qdisc_destroy. The problem lies in the CBQ
qdisc, it doesn't destroy the root-class and leaks the inner qdisc. These
two patches for 2.4 and 2.6 fix the problem.

Regards
Patrick

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/08/22 14:37:58+02:00 [email-masked] 
#   [PKT_SCHED]: Fix class leak in CBQ scheduler
#   
#   Signed-off-by: Patrick McHardy <[email-masked]>
# 
# net/sched/sch_cbq.c
#   2004/08/22 14:37:27+02:00 [email-masked] +8 -6
#   [PKT_SCHED]: Fix class leak in CBQ scheduler
# 
diff -Nru a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
--- a/net/sched/sch_cbq.c	2004-08-22 14:38:18 +02:00
+++ b/net/sched/sch_cbq.c	2004-08-22 14:38:18 +02:00
@@ -1712,15 +1712,18 @@
 	}
 }
 
-static void cbq_destroy_class(struct cbq_class *cl)
+static void cbq_destroy_class(struct Qdisc *sch, struct cbq_class *cl)
 {
+	struct cbq_sched_data *q = (struct cbq_sched_data *)sch->data;
+
 	cbq_destroy_filters(cl);
 	qdisc_destroy(cl->q);
 	qdisc_put_rtab(cl->R_tab);
 #ifdef CONFIG_NET_ESTIMATOR
 	qdisc_kill_estimator(&cl->stats);
 #endif
-	kfree(cl);
+	if (cl != &q->link)
+		kfree(cl);
 }
 
 static void
@@ -1743,8 +1746,7 @@
 
 		for (cl = q->classes[h]; cl; cl = next) {
 			next = cl->next;
-			if (cl != &q->link)
-				cbq_destroy_class(cl);
+			cbq_destroy_class(sch, cl);
 		}
 	}
 
@@ -1766,7 +1768,7 @@
 		spin_unlock_bh(&sch->dev->queue_lock);
 #endif
 
-		cbq_destroy_class(cl);
+		cbq_destroy_class(sch, cl);
 	}
 }
 
@@ -2000,7 +2002,7 @@
 	sch_tree_unlock(sch);
 
 	if (--cl->refcnt == 0)
-		cbq_destroy_class(cl);
+		cbq_destroy_class(sch, cl);
 
 	return 0;
 }
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/08/22 14:30:32+02:00 [email-masked] 
#   [PKT_SCHED]: Fix class leak in CBQ scheduler
#   
#   Signed-off-by: Patrick McHardy <[email-masked]>
# 
# net/sched/sch_cbq.c
#   2004/08/22 14:30:13+02:00 [email-masked] +8 -6
#   [PKT_SCHED]: Fix class leak in CBQ scheduler
# 
diff -Nru a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
--- a/net/sched/sch_cbq.c	2004-08-22 14:33:59 +02:00
+++ b/net/sched/sch_cbq.c	2004-08-22 14:33:59 +02:00
@@ -1746,15 +1746,18 @@
 	}
 }
 
-static void cbq_destroy_class(struct cbq_class *cl)
+static void cbq_destroy_class(struct Qdisc *sch, struct cbq_class *cl)
 {
+	struct cbq_sched_data *q = qdisc_priv(sch);
+
 	cbq_destroy_filters(cl);
 	qdisc_destroy(cl->q);
 	qdisc_put_rtab(cl->R_tab);
 #ifdef CONFIG_NET_ESTIMATOR
 	qdisc_kill_estimator(&cl->stats);
 #endif
-	kfree(cl);
+	if (cl != &q->link)
+		kfree(cl);
 }
 
 static void
@@ -1777,8 +1780,7 @@
 
 		for (cl = q->classes[h]; cl; cl = next) {
 			next = cl->next;
-			if (cl != &q->link)
-				cbq_destroy_class(cl);
+			cbq_destroy_class(sch, cl);
 		}
 	}
 
@@ -1799,7 +1801,7 @@
 		spin_unlock_bh(&sch->dev->queue_lock);
 #endif
 
-		cbq_destroy_class(cl);
+		cbq_destroy_class(sch, cl);
 	}
 }
 
@@ -2035,7 +2037,7 @@
 	sch_tree_unlock(sch);
 
 	if (--cl->refcnt == 0)
-		cbq_destroy_class(cl);
+		cbq_destroy_class(sch, cl);
 
 	return 0;
 }