Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock().

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Herbert Xu
Date: Thursday, August 21, 2008 - 5:48 am

On Thu, Aug 21, 2008 at 10:35:38PM +1000, Herbert Xu wrote:

How about going back to a single list per-device again? This list
is only used on the slow path (well anything that tries to walk
a potentially unbounded linked list is slow :), and qdisc_lookup
walks through everything anyway.

We'll need to then add a new lock to protect this list, until we
remove requeue.

Actually just doing the locking will be sufficient.  Something like
this totally untested patch (I've abused your tx global lock):

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index ef0efec..3f5f9b9 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -202,16 +202,25 @@ struct Qdisc *qdisc_match_from_root(struct Qdisc *root, u32 handle)
 struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle)
 {
 	unsigned int i;
+	struct Qdisc *q;
+
+	spin_lock_bh(&dev->tx_global_lock);
 
 	for (i = 0; i < dev->num_tx_queues; i++) {
 		struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
-		struct Qdisc *q, *txq_root = txq->qdisc_sleeping;
+		struct Qdisc *txq_root = txq->qdisc_sleeping;
 
 		q = qdisc_match_from_root(txq_root, handle);
 		if (q)
-			return q;
+			goto unlock;
 	}
-	return qdisc_match_from_root(dev->rx_queue.qdisc_sleeping, handle);
+
+	q = qdisc_match_from_root(dev->rx_queue.qdisc_sleeping, handle);
+
+unlock:
+	spin_unlock_bh(&dev->tx_global_lock);
+
+	return q;
 }
 
 static struct Qdisc *qdisc_leaf(struct Qdisc *p, u32 classid)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index c3ed4d4..292a373 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -526,8 +526,10 @@ void qdisc_destroy(struct Qdisc *qdisc)
 	    !atomic_dec_and_test(&qdisc->refcnt))
 		return;
 
+	spin_lock_bh(&dev->tx_global_lock);
 	if (qdisc->parent)
 		list_del(&qdisc->list);
+	spin_unlock_bh(&dev->tx_global_lock);
 
 #ifdef CONFIG_NET_SCHED
 	qdisc_put_stab(qdisc->stab);

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH] pkt_sched: Destroy gen estimators under rtnl_lock()., Jarek Poplawski, (Mon Aug 11, 1:53 pm)
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_l ..., Denys Fedoryshchenko, (Sun Aug 17, 11:08 pm)
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_l ..., Herbert Xu, (Thu Aug 21, 5:48 am)
[PATCH] pkt_sched: Fix qdisc list locking, Jarek Poplawski, (Fri Aug 22, 1:41 am)
[PATCH take 2] pkt_sched: Fix qdisc list locking, Jarek Poplawski, (Fri Aug 22, 2:27 am)
Re: [PATCH] pkt_sched: Fix qdisc list locking, Herbert Xu, (Fri Aug 22, 3:14 am)
Re: [PATCH take 2] pkt_sched: Fix qdisc list locking, Herbert Xu, (Fri Aug 22, 3:15 am)
Re: [PATCH take 2] pkt_sched: Fix qdisc list locking, David Miller, (Fri Aug 22, 3:23 am)
Re: [PATCH take 2] pkt_sched: Fix qdisc list locking, David Miller, (Fri Aug 22, 3:28 am)
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_l ..., Stephen Hemminger, (Sun Aug 24, 4:26 pm)
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_l ..., Stephen Hemminger, (Sun Aug 24, 5:29 pm)
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_l ..., Stephen Hemminger, (Tue Aug 26, 5:24 am)
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_l ..., Stephen Hemminger, (Tue Aug 26, 5:50 am)
RE: [PATCH take 2] pkt_sched: Fix qdisc_watchdog() vs. dev ..., Duyck, Alexander H, (Mon Sep 15, 4:44 pm)