pkt_sched: sch_api: Remove qdisc_list_lock
Subsystems (3)
PATCH 1/1 pkt_sched: sch_api: Remove qdisc_list_lock
After implementing qdisc->ops->peek() there is no more calling qdisc_tree_decrease_qlen() without rtnl_lock(), so qdisc_list_lock added by commit: f6e0b239a2657ea8cb67f0d83d0bfdbfd19a481b "pkt_sched:
Fix qdisc list locking" can be removed.
---
net/sched/sch_api.c | 25 ++++--------------------- 1 files changed, 4 insertions(+), 21 deletions(-)
net/sched/sch_api.c +4 -21
@@ -204,28 +204,16 @@ struct Qdisc *qdisc_match_from_root(struct Qdisc *root, u32 handle)return NULL ; } -/* -* This lock is needed until some qdiscs stop calling qdisc_tree_decrease_qlen() -* without rtnl_lock(); currently hfsc_dequeue(), netem_dequeue(), tbf_dequeue() -*/ -static DEFINE_SPINLOCK (qdisc_list_lock); -static void qdisc_list_add (struct Qdisc *q) { -if ((q->parent !=TC_H_ROOT ) && !(q->flags &TCQ_F_INGRESS )) { -spin_lock_bh (&qdisc_list_lock); +if ((q->parent !=TC_H_ROOT ) && !(q->flags &TCQ_F_INGRESS ))list_add_tail (&q->list, &qdisc_root_sleeping (q)->list); -spin_unlock_bh (&qdisc_list_lock); - } }void qdisc_list_del (struct Qdisc *q) { -if ((q->parent !=TC_H_ROOT ) && !(q->flags &TCQ_F_INGRESS )) { -spin_lock_bh (&qdisc_list_lock); +if ((q->parent !=TC_H_ROOT ) && !(q->flags &TCQ_F_INGRESS ))list_del (&q->list); -spin_unlock_bh (&qdisc_list_lock); - } }EXPORT_SYMBOL (qdisc_list_del);
@@ -234,22 +222,17 @@ struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle)unsigned int i;struct Qdisc *q; -spin_lock_bh (&qdisc_list_lock); -for (i =0 ; i < dev->num_tx_queues; i++) {struct netdev_queue *txq =netdev_get_tx_queue (dev, i);struct Qdisc *txq_root = txq->qdisc_sleeping; q =qdisc_match_from_root (txq_root, handle);if (q) - goto unlock; + goto out; } q =qdisc_match_from_root (dev->rx_queue.qdisc_sleeping, handle); - -unlock: -spin_unlock_bh (&qdisc_list_lock); - +out:return q; }
Quoted 4 lines · Jarek Poplawski
After implementing
qdisc->ops->peek()there is no more callingqdisc_tree_decrease_qlen()withoutrtnl_lock(), soqdisc_list_lockadded by commit:f6e0b239a2657ea8cb67f0d83d0bfdbfd19a481b"pkt_sched:
Fix qdisc list locking" can be removed.
I might be misunderstanding something, but qdisc_tree_decrease_qlen() doesn't need rtnl_lock() but the sch_tree_lock() since it changes q.qlen.
Quoted 9 lines · Patrick McHardy
Quoted 4 lines · Jarek Poplawski
After implementing
qdisc->ops->peek()there is no more callingqdisc_tree_decrease_qlen()withoutrtnl_lock(), soqdisc_list_lockadded by commit:f6e0b239a2657ea8cb67f0d83d0bfdbfd19a481b"pkt_sched:
Fix qdisc list locking" can be removed.I might be misunderstanding something, but
qdisc_tree_decrease_qlen()doesn't needrtnl_lock()but thesch_tree_lock()since it changes q.qlen.
OK I see now, rtnl_lock() was taken to protect the parent qdisc lookup. In that case it seems fine, all callers are holding both sch_tree_lock() and rtnl_lock().
David Miller
maintainerFrom: Patrick McHardy <[email protected]>
Date: Tue, 25 Nov 2008 12:46:25 +0100
Quoted 6 lines
Quoted 5 lines · Patrick McHardy
Quoted 4 lines · Jarek Poplawski
After implementing
qdisc->ops->peek()there is no more callingqdisc_tree_decrease_qlen()withoutrtnl_lock(), soqdisc_list_lockadded by commit:f6e0b239a2657ea8cb67f0d83d0bfdbfd19a481b"pkt_sched:
Fix qdisc list locking" can be removed.
Quoted 7 lines
Quoted 3 lines
I might be misunderstanding something, but
qdisc_tree_decrease_qlen()doesn't needrtnl_lock()but thesch_tree_lock()since it changes q.qlen.OK I see now,
rtnl_lock()was taken to protect the parent qdisc lookup. In that case it seems fine, all callers are holding bothsch_tree_lock()andrtnl_lock().
:-)
I've applied Jarek's patch.
Thanks!