Re: Fw: BUG: atomic counter underflow when running "rmmod tg3" on 2.6.10-rc1-mm3

[Patrick McHardy <kaber@trash.net>]

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

Andrew Morton wrote:

>There's a debug patch in -mm which generates a trace when someone does
>atomic_dec_and_test() on an atomic_t which already has a value of zero.
>
>ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.10-rc1/2.6.10-rc1-mm3/broken-out/detect-atomic-counter-underflows.patch
>
>It looks like it has found a problem in qdisc_destroy().
>  
>
Already fixed in latest -bk by the attached patch. The builtin qdiscs
are not refcounted, but qdisc_destroy treated them as other qdiscs.

Regards
Patrick


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1771 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/11/05 16:30:34-08:00 tgraf@suug.ch 
#   [PKT_SCHED]: Builtin qdiscs should avoid all qdisc_destroy() processing.
#   
#   None of the code in __qdisc_destroy should be applied to a builtin qdisc
#   or am I missing something?
#   
#   The patch below prevents builtin qdiscs from being destroyed and
#   fixes a refcnt underflow whould lead to a bogus list unlinking
#   and dev_put.
#   
#   Signed-off-by: Thomas Graf <tgraf@suug.ch>
#   Signed-off-by: David S. Miller <davem@davemloft.net>
# 
# net/sched/sch_generic.c
#   2004/11/05 16:30:14-08:00 tgraf@suug.ch +3 -3
#   [PKT_SCHED]: Builtin qdiscs should avoid all qdisc_destroy() processing.
#   
#   None of the code in __qdisc_destroy should be applied to a builtin qdisc
#   or am I missing something?
#   
#   The patch below prevents builtin qdiscs from being destroyed and
#   fixes a refcnt underflow whould lead to a bogus list unlinking
#   and dev_put.
#   
#   Signed-off-by: Thomas Graf <tgraf@suug.ch>
#   Signed-off-by: David S. Miller <davem@davemloft.net>
# 
diff -Nru a/net/sched/sch_generic.c b/net/sched/sch_generic.c
--- a/net/sched/sch_generic.c	2004-11-09 06:50:10 +01:00
+++ b/net/sched/sch_generic.c	2004-11-09 06:50:10 +01:00
@@ -479,15 +479,15 @@
 	module_put(ops->owner);
 
 	dev_put(qdisc->dev);
-	if (!(qdisc->flags&TCQ_F_BUILTIN))
-		kfree((char *) qdisc - qdisc->padded);
+	kfree((char *) qdisc - qdisc->padded);
 }
 
 /* Under dev->queue_lock and BH! */
 
 void qdisc_destroy(struct Qdisc *qdisc)
 {
-	if (!atomic_dec_and_test(&qdisc->refcnt))
+	if (qdisc->flags & TCQ_F_BUILTIN ||
+		!atomic_dec_and_test(&qdisc->refcnt))
 		return;
 	list_del(&qdisc->list);
 	call_rcu(&qdisc->q_rcu, __qdisc_destroy);