linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Javier Cardona <javier@cozybit.com>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: Javier Cardona <javier@cozybit.com>,
	Thomas Pedersen <thomas@cozybit.com>,
	devel@lists.open80211s.org,
	Johannes Berg <johannes@sipsolutions.net>,
	linux-wireless@vger.kernel.org, jlopex@gmail.com
Subject: [PATCH 10/13] mac80211: Fix locking bug on mesh path table access
Date: Tue,  3 May 2011 16:57:16 -0700	[thread overview]
Message-ID: <1304467039-7730-11-git-send-email-javier@cozybit.com> (raw)
In-Reply-To: <1304467039-7730-1-git-send-email-javier@cozybit.com>

The mesh and mpp path tables are accessed from softirq and workqueue
context so non-irq locking cannot be used.  Or at least that's what
PROVE_RCU seems to tell us here:

[  431.240946] =================================
[  431.241061] [ INFO: inconsistent lock state ]
[  431.241061] 2.6.39-rc3-wl+ #354
[  431.241061] ---------------------------------
[  431.241061] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[  431.241061] kworker/u:1/1423 [HC0[0]:SC0[0]:HE1:SE1] takes:
[  431.241061]  (&(&newtbl->hashwlock[i])->rlock){+.?...}, at:
[<c14671bf>] mesh_path_add+0x167/0x257

Signed-off-by: Javier Cardona <javier@cozybit.com>
---
 net/mac80211/mesh_pathtbl.c |   54 ++++++++++++++++++++++--------------------
 1 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index d4e86fd..a1d59bd 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -55,12 +55,12 @@ void mesh_table_free(struct mesh_table *tbl, bool free_leafs)
 
 	mesh_hash = tbl->hash_buckets;
 	for (i = 0; i <= tbl->hash_mask; i++) {
-		spin_lock(&tbl->hashwlock[i]);
+		spin_lock_bh(&tbl->hashwlock[i]);
 		hlist_for_each_safe(p, q, &mesh_hash[i]) {
 			tbl->free_node(p, free_leafs);
 			atomic_dec(&tbl->entries);
 		}
-		spin_unlock(&tbl->hashwlock[i]);
+		spin_unlock_bh(&tbl->hashwlock[i]);
 	}
 	__mesh_table_free(tbl);
 }
@@ -274,7 +274,7 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata)
 	if (!new_node)
 		goto err_node_alloc;
 
-	read_lock(&pathtbl_resize_lock);
+	read_lock_bh(&pathtbl_resize_lock);
 	memcpy(new_mpath->dst, dst, ETH_ALEN);
 	new_mpath->sdata = sdata;
 	new_mpath->flags = 0;
@@ -289,7 +289,7 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata)
 	hash_idx = mesh_table_hash(dst, sdata, mesh_paths);
 	bucket = &mesh_paths->hash_buckets[hash_idx];
 
-	spin_lock(&mesh_paths->hashwlock[hash_idx]);
+	spin_lock_bh(&mesh_paths->hashwlock[hash_idx]);
 
 	err = -EEXIST;
 	hlist_for_each_entry(node, n, bucket, list) {
@@ -305,8 +305,8 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata)
 
 	mesh_paths_generation++;
 
-	spin_unlock(&mesh_paths->hashwlock[hash_idx]);
-	read_unlock(&pathtbl_resize_lock);
+	spin_unlock_bh(&mesh_paths->hashwlock[hash_idx]);
+	read_unlock_bh(&pathtbl_resize_lock);
 	if (grow) {
 		set_bit(MESH_WORK_GROW_MPATH_TABLE,  &ifmsh->wrkq_flags);
 		ieee80211_queue_work(&local->hw, &sdata->work);
@@ -314,8 +314,8 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata)
 	return 0;
 
 err_exists:
-	spin_unlock(&mesh_paths->hashwlock[hash_idx]);
-	read_unlock(&pathtbl_resize_lock);
+	spin_unlock_bh(&mesh_paths->hashwlock[hash_idx]);
+	read_unlock_bh(&pathtbl_resize_lock);
 	kfree(new_node);
 err_node_alloc:
 	kfree(new_mpath);
@@ -332,16 +332,17 @@ void mesh_mpath_table_grow(void)
 	newtbl = mesh_table_alloc(rcu_dereference(mesh_paths)->size_order + 1);
 	if (!newtbl)
 		return;
-	write_lock(&pathtbl_resize_lock);
+	write_lock_bh(&pathtbl_resize_lock);
 	oldtbl = mesh_paths;
 	if (mesh_table_grow(mesh_paths, newtbl) < 0) {
+		rcu_read_unlock();
 		__mesh_table_free(newtbl);
-		write_unlock(&pathtbl_resize_lock);
+		write_unlock_bh(&pathtbl_resize_lock);
 		return;
 	}
 	rcu_read_unlock();
 	rcu_assign_pointer(mesh_paths, newtbl);
-	write_unlock(&pathtbl_resize_lock);
+	write_unlock_bh(&pathtbl_resize_lock);
 
 	synchronize_rcu();
 	mesh_table_free(oldtbl, false);
@@ -355,16 +356,17 @@ void mesh_mpp_table_grow(void)
 	newtbl = mesh_table_alloc(rcu_dereference(mpp_paths)->size_order + 1);
 	if (!newtbl)
 		return;
-	write_lock(&pathtbl_resize_lock);
+	write_lock_bh(&pathtbl_resize_lock);
 	oldtbl = mpp_paths;
 	if (mesh_table_grow(mpp_paths, newtbl) < 0) {
+		rcu_read_unlock();
 		__mesh_table_free(newtbl);
-		write_unlock(&pathtbl_resize_lock);
+		write_unlock_bh(&pathtbl_resize_lock);
 		return;
 	}
 	rcu_read_unlock();
 	rcu_assign_pointer(mpp_paths, newtbl);
-	write_unlock(&pathtbl_resize_lock);
+	write_unlock_bh(&pathtbl_resize_lock);
 
 	synchronize_rcu();
 	mesh_table_free(oldtbl, false);
@@ -398,7 +400,7 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata)
 	if (!new_node)
 		goto err_node_alloc;
 
-	read_lock(&pathtbl_resize_lock);
+	read_lock_bh(&pathtbl_resize_lock);
 	memcpy(new_mpath->dst, dst, ETH_ALEN);
 	memcpy(new_mpath->mpp, mpp, ETH_ALEN);
 	new_mpath->sdata = sdata;
@@ -411,7 +413,7 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata)
 	hash_idx = mesh_table_hash(dst, sdata, mpp_paths);
 	bucket = &mpp_paths->hash_buckets[hash_idx];
 
-	spin_lock(&mpp_paths->hashwlock[hash_idx]);
+	spin_lock_bh(&mpp_paths->hashwlock[hash_idx]);
 
 	err = -EEXIST;
 	hlist_for_each_entry(node, n, bucket, list) {
@@ -425,8 +427,8 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata)
 		mpp_paths->mean_chain_len * (mpp_paths->hash_mask + 1))
 		grow = 1;
 
-	spin_unlock(&mpp_paths->hashwlock[hash_idx]);
-	read_unlock(&pathtbl_resize_lock);
+	spin_unlock_bh(&mpp_paths->hashwlock[hash_idx]);
+	read_unlock_bh(&pathtbl_resize_lock);
 	if (grow) {
 		set_bit(MESH_WORK_GROW_MPP_TABLE,  &ifmsh->wrkq_flags);
 		ieee80211_queue_work(&local->hw, &sdata->work);
@@ -434,8 +436,8 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata)
 	return 0;
 
 err_exists:
-	spin_unlock(&mpp_paths->hashwlock[hash_idx]);
-	read_unlock(&pathtbl_resize_lock);
+	spin_unlock_bh(&mpp_paths->hashwlock[hash_idx]);
+	read_unlock_bh(&pathtbl_resize_lock);
 	kfree(new_node);
 err_node_alloc:
 	kfree(new_mpath);
@@ -548,11 +550,11 @@ int mesh_path_del(u8 *addr, struct ieee80211_sub_if_data *sdata)
 	int hash_idx;
 	int err = 0;
 
-	read_lock(&pathtbl_resize_lock);
+	read_lock_bh(&pathtbl_resize_lock);
 	hash_idx = mesh_table_hash(addr, sdata, mesh_paths);
 	bucket = &mesh_paths->hash_buckets[hash_idx];
 
-	spin_lock(&mesh_paths->hashwlock[hash_idx]);
+	spin_lock_bh(&mesh_paths->hashwlock[hash_idx]);
 	hlist_for_each_entry(node, n, bucket, list) {
 		mpath = node->mpath;
 		if (mpath->sdata == sdata &&
@@ -570,8 +572,8 @@ int mesh_path_del(u8 *addr, struct ieee80211_sub_if_data *sdata)
 	err = -ENXIO;
 enddel:
 	mesh_paths_generation++;
-	spin_unlock(&mesh_paths->hashwlock[hash_idx]);
-	read_unlock(&pathtbl_resize_lock);
+	spin_unlock_bh(&mesh_paths->hashwlock[hash_idx]);
+	read_unlock_bh(&pathtbl_resize_lock);
 	return err;
 }
 
@@ -723,7 +725,7 @@ void mesh_path_expire(struct ieee80211_sub_if_data *sdata)
 	struct hlist_node *p;
 	int i;
 
-	read_lock(&pathtbl_resize_lock);
+	read_lock_bh(&pathtbl_resize_lock);
 	for_each_mesh_entry(mesh_paths, p, node, i) {
 		if (node->mpath->sdata != sdata)
 			continue;
@@ -738,7 +740,7 @@ void mesh_path_expire(struct ieee80211_sub_if_data *sdata)
 		} else
 			spin_unlock_bh(&mpath->state_lock);
 	}
-	read_unlock(&pathtbl_resize_lock);
+	read_unlock_bh(&pathtbl_resize_lock);
 }
 
 void mesh_pathtbl_unregister(void)
-- 
1.7.1


  parent reply	other threads:[~2011-05-03 23:57 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-03 23:57 Support for secure mesh in userspace and other mesh fixes Javier Cardona
2011-05-03 23:57 ` [PATCH 01/13] nl80211: Introduce NL80211_MESH_SETUP_USERSPACE_AMPE Javier Cardona
2011-05-04 12:27   ` Johannes Berg
2011-05-04 16:32     ` Javier Cardona
2011-05-04 16:34       ` Johannes Berg
2011-05-03 23:57 ` [PATCH 02/13] mac80211: Let userspace send action frames over mesh interfaces Javier Cardona
2011-05-03 23:57 ` [PATCH 03/13] mac80211: Drop MESH_PLINK category and use new ANA-approved MESH_ACTION Javier Cardona
2011-05-03 23:57 ` [PATCH 04/13] open80211s: Stop using zero for address 3 in mesh plink mgmt frames Javier Cardona
2011-05-04 12:57   ` Johannes Berg
2011-05-04 16:28     ` Javier Cardona
2011-05-04 17:24     ` [PATCH] cfg80211: Use capability info to detect mesh beacons Javier Cardona
2011-05-09  8:25       ` Johannes Berg
2011-05-30 10:51       ` Vivek Natarajan
2011-05-31 17:26         ` Javier Cardona
2011-05-31 18:10           ` Eliad Peller
2011-05-31 18:38             ` Javier Cardona
2011-06-01  7:01               ` Eliad Peller
2011-05-03 23:57 ` [PATCH 05/13] nl80211: Let userspace drive the peer link management states Javier Cardona
2011-05-04 12:28   ` Johannes Berg
2011-05-03 23:57 ` [PATCH 06/13] nl80211: allow installing keys for a meshif Javier Cardona
2011-05-03 23:57 ` [PATCH 07/13] nl80211: allow setting MFP flag " Javier Cardona
2011-05-03 23:57 ` [PATCH 08/13] mac80211: Self-protected management frames are not robust Javier Cardona
2011-05-03 23:57 ` [PATCH 09/13] Check size of a new mesh path table for changes since allocation Javier Cardona
2011-05-03 23:57 ` Javier Cardona [this message]
2011-05-03 23:57 ` [PATCH 11/13] mac80211: Move call to mpp_path_lookup inside RCU-read section Javier Cardona
2011-05-03 23:57 ` [PATCH 12/13] mac80211: allow setting supported rates on mesh peers Javier Cardona
2011-05-03 23:57 ` [PATCH 13/13] ath9k: fix beaconing for mesh interfaces Javier Cardona
2011-05-04 14:42   ` Felix Fietkau
2011-05-04 16:16     ` Steve Brown
2011-05-04 17:13     ` Javier Cardona
2011-05-04 17:25       ` Johannes Berg
2011-05-04 17:31         ` Javier Cardona

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1304467039-7730-11-git-send-email-javier@cozybit.com \
    --to=javier@cozybit.com \
    --cc=devel@lists.open80211s.org \
    --cc=jlopex@gmail.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=thomas@cozybit.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).