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

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: David Miller
Date: Wednesday, August 13, 2008 - 2:25 am

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed, 13 Aug 2008 06:53:03 +0000


Here it my concrete proposal for a fix.

pkt_sched: Fix queue quiescence testing in dev_deactivate().

Based upon discussions with Jarek P. and Herbert Xu.

First, we're testing the wrong qdisc.  We just reset the device
queue qdiscs to &noop_qdisc and checking it's state is completely
pointless here.

We want to wait until the previous qdisc that was sitting at
the ->qdisc pointer is not busy any more.  And that would be
->qdisc_sleeping.

Because of how we propagate the samples qdisc pointer down into
qdisc_run and friends via per-cpu ->output_queue and netif_schedule,
we have to wait also for the __QDISC_STATE_SCHED bit to clear as
well.

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 7cf83b3..4685746 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -647,7 +647,7 @@ static void dev_deactivate_queue(struct net_device *dev,
 	}
 }
 
-static bool some_qdisc_is_running(struct net_device *dev, int lock)
+static bool some_qdisc_is_busy(struct net_device *dev, int lock)
 {
 	unsigned int i;
 
@@ -658,13 +658,14 @@ static bool some_qdisc_is_running(struct net_device *dev, int lock)
 		int val;
 
 		dev_queue = netdev_get_tx_queue(dev, i);
-		q = dev_queue->qdisc;
+		q = dev_queue->qdisc_sleeping;
 		root_lock = qdisc_lock(q);
 
 		if (lock)
 			spin_lock_bh(root_lock);
 
-		val = test_bit(__QDISC_STATE_RUNNING, &q->state);
+		val = (test_bit(__QDISC_STATE_RUNNING, &q->state) ||
+		       test_bit(__QDISC_STATE_SCHED, &q->state));
 
 		if (lock)
 			spin_unlock_bh(root_lock);
@@ -689,14 +690,14 @@ void dev_deactivate(struct net_device *dev)
 
 	/* Wait for outstanding qdisc_run calls. */
 	do {
-		while (some_qdisc_is_running(dev, 0))
+		while (some_qdisc_is_busy(dev, 0))
 			yield();
 
 		/*
 		 * Double-check inside queue lock to ensure that all effects
 		 * of the queue run are visible when we return.
 		 */
-		running = some_qdisc_is_running(dev, 1);
+		running = some_qdisc_is_busy(dev, 1);
 
 		/*
 		 * The running flag should never be set at this point because
--
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 ..., David Miller, (Wed Aug 13, 2:25 am)
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_l ..., Denys Fedoryshchenko, (Sun Aug 17, 11:08 pm)
[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)