Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock().
From: Jarek Poplawski <hidden>
Date: 2008-08-18 21:26:30
David Miller wrote, On 08/18/2008 03:49 AM:
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Mon, 18 Aug 2008 11:36:33 +1000quoted
On Sun, Aug 17, 2008 at 06:35:05PM -0700, David Miller wrote:quoted
I think I see another way out of this: 1) Add __QDISC_STATE_DEACTIVATE. 2) Set it right before dev_deactivate() swaps resets the qdisc pointer. 3) Test it in dev_queue_xmit() et al. once the qdisc root lock is acquired, and drop lock and resample ->qdisc if __QDISC_STATE_DEACTIVATE is set.
Two little doubts below:
quoted hunk ↗ jump to hunk
diff --git a/net/core/dev.c b/net/core/dev.c index 600bb23..b88f669 100644 --- a/net/core/dev.c +++ b/net/core/dev.c@@ -1341,6 +1341,9 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev) void __netif_schedule(struct Qdisc *q) { + if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) + return; +
Why I can't see this code in net-2.6? BTW, I guess it should be now moved to the current __netif_reschedule()?
quoted hunk ↗ jump to hunk
if (!test_and_set_bit(__QDISC_STATE_SCHED, &q->state)) { struct softnet_data *sd; unsigned long flags;@@ -1790,6 +1793,8 @@ gso: rcu_read_lock_bh(); txq = dev_pick_tx(dev, skb); + +resample_qdisc: q = rcu_dereference(txq->qdisc); #ifdef CONFIG_NET_CLS_ACT@@ -1800,6 +1805,11 @@ gso: spin_lock(root_lock); + if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) { + spin_unlock(root_lock); + goto resample_qdisc; + } +
OK, we now have this kfree_skb() with NET_XMIT_DROP here, but how is it better than qdisc_enque_root() on noop_qdisc? Or how can we have here anything else under both rcu lock and spin_lock() while this __QDISC_STATE_DEACTIVATED bit is set? Jarek P.
rc = qdisc_enqueue_root(skb, q); qdisc_run(q);
...