[PATCH] pkt_sched: Add BH protection for qdisc_stab_lock.

[PATCH] pkt_sched: Add BH protection for qdisc_stab_lock.

[Jarek Poplawski]

pkt_sched: Add BH protection for qdisc_stab_lock.

Since qdisc_stab_lock is used in qdisc_put_stab(), which is called in
BH context from __qdisc_destroy() RCU callback, softirq safe locking
is needed.


Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

---

 net/sched/sch_api.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index bbf149d..c25465e 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -331,7 +331,7 @@ static struct qdisc_size_table *qdisc_get_stab(struct nlattr *opt)
 	if (!s || tsize != s->tsize || (!tab && tsize > 0))
 		return ERR_PTR(-EINVAL);
 
-	spin_lock(&qdisc_stab_lock);
+	spin_lock_bh(&qdisc_stab_lock);
 
 	list_for_each_entry(stab, &qdisc_stab_list, list) {
 		if (memcmp(&stab->szopts, s, sizeof(*s)))
@@ -339,11 +339,11 @@ static struct qdisc_size_table *qdisc_get_stab(struct nlattr *opt)
 		if (tsize > 0 && memcmp(stab->data, tab, tsize * sizeof(u16)))
 			continue;
 		stab->refcnt++;
-		spin_unlock(&qdisc_stab_lock);
+		spin_unlock_bh(&qdisc_stab_lock);
 		return stab;
 	}
 
-	spin_unlock(&qdisc_stab_lock);
+	spin_unlock_bh(&qdisc_stab_lock);
 
 	stab = kmalloc(sizeof(*stab) + tsize * sizeof(u16), GFP_KERNEL);
 	if (!stab)
@@ -354,9 +354,9 @@ static struct qdisc_size_table *qdisc_get_stab(struct nlattr *opt)
 	if (tsize > 0)
 		memcpy(stab->data, tab, tsize * sizeof(u16));
 
-	spin_lock(&qdisc_stab_lock);
+	spin_lock_bh(&qdisc_stab_lock);
 	list_add_tail(&stab->list, &qdisc_stab_list);
-	spin_unlock(&qdisc_stab_lock);
+	spin_unlock_bh(&qdisc_stab_lock);
 
 	return stab;
 }
@@ -366,14 +366,14 @@ void qdisc_put_stab(struct qdisc_size_table *tab)
 	if (!tab)
 		return;
 
-	spin_lock(&qdisc_stab_lock);
+	spin_lock_bh(&qdisc_stab_lock);
 
 	if (--tab->refcnt == 0) {
 		list_del(&tab->list);
 		kfree(tab);
 	}
 
-	spin_unlock(&qdisc_stab_lock);
+	spin_unlock_bh(&qdisc_stab_lock);
 }
 EXPORT_SYMBOL(qdisc_put_stab);
 

[PATCH alternative] pkt_sched: Move qdisc_put_stab() from BH context.

[Jarek Poplawski]

On Mon, Aug 11, 2008 at 11:42:39PM +0200, Jarek Poplawski wrote:
> 
> pkt_sched: Add BH protection for qdisc_stab_lock.

...Or instead of this:
-------------> (alternative version)

pkt_sched: Move qdisc_put_stab() from BH context.

Since qdisc_stab_lock is used in qdisc_put_stab(), which is called in
BH context from __qdisc_destroy() RCU callback, softirq safe locking
should be used. Alternatively we can move this to qdisc_destroy(). 


Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

---
(Apply on top of: pkt_sched: Destroy gen estimator under rtnl_lock().)

diff -Nurp a/net/sched/sch_generic.c b/net/sched/sch_generic.c
--- a/net/sched/sch_generic.c	2008-08-12 00:04:22.000000000 +0200
+++ b/net/sched/sch_generic.c	2008-08-12 00:08:24.000000000 +0200
@@ -525,9 +525,6 @@ static void __qdisc_destroy(struct rcu_h
 {
 	struct Qdisc *qdisc = container_of(head, struct Qdisc, q_rcu);
 
-#ifdef CONFIG_NET_SCHED
-	qdisc_put_stab(qdisc->stab);
-#endif
 	dev_put(qdisc_dev(qdisc));
 
 	kfree_skb(qdisc->gso_skb);
@@ -548,6 +545,9 @@ void qdisc_destroy(struct Qdisc *qdisc)
 	if (qdisc->parent)
 		list_del(&qdisc->list);
 
+#ifdef CONFIG_NET_SCHED
+	qdisc_put_stab(qdisc->stab);
+#endif
 	gen_kill_estimator(&qdisc->bstats, &qdisc->rate_est);
 	if (ops->reset)
 		ops->reset(qdisc);

Re: [PATCH] pkt_sched: Add BH protection for qdisc_stab_lock.

[David Miller]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 11 Aug 2008 23:42:39 +0200

> pkt_sched: Add BH protection for qdisc_stab_lock.
> 
> Since qdisc_stab_lock is used in qdisc_put_stab(), which is called in
> BH context from __qdisc_destroy() RCU callback, softirq safe locking
> is needed.
> 
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

Applied, thanks Jarek.