Skip to contentSkip to conversation

hw scan fixes

PATCH In review Johannes Berg 1 participant 4 messages Jul 30, 2010

Subsystems (2)
lore
Share & tools
lore

Johannes Berg

author maintainer
#0 lore raw

cover hw scan fixes

I'm rolling these up because without the iwlwifi fix I'm pretty sure we will also get lockdep warnings even if the actual deadlock scenario is rather unlikely to start with.

Technically these are 2.6.35 regressions, but I guess it's a little late?

johannes

author maintainer
#1 lore raw

PATCH 1/2 iwlwifi: fix possible recursive locking deadlock

From: Johannes Berg <[email protected]>

commit f84b29ec0a1ab767679d3f2428877b65f94bc3ff

Author: Johannes Berg <[email protected]>
Date:   Tue May 18 02:29:13 2010 -0700

iwlwifi: queue user-initiated scan when doing internal scan

introduced a potential deadlock because it calls ieee80211_scan_completed() with the priv->mutex held, but mac80211 may call back into iwlwifi which would lead to recursive locking. Move this out from under the mutex.

---
 drivers/net/wireless/iwlwifi/iwl-core.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/net/wireless/iwlwifi/iwl-core.c +5 -1
@@ -2000,6 +2000,7 @@ void iwl_mac_remove_interface(struct iee
 			      struct ieee80211_vif *vif)
 {
 	struct iwl_priv *priv = hw->priv;
+	bool scan_completed = false;
 
 	IWL_DEBUG_MAC80211(priv, "enter\n");
 
@@ -2013,7 +2014,7 @@ void iwl_mac_remove_interface(struct iee
 	if (priv->vif == vif) {
 		priv->vif = NULL;
 		if (priv->scan_vif == vif) {
-			ieee80211_scan_completed(priv->hw, true);
+			scan_completed = true;
 			priv->scan_vif = NULL;
 			priv->scan_request = NULL;
 		}
@@ -2021,6 +2022,9 @@ void iwl_mac_remove_interface(struct iee
 	}
 	mutex_unlock(&priv->mutex);
 
+	if (scan_completed)
+		ieee80211_scan_completed(priv->hw, true);
+
 	IWL_DEBUG_MAC80211(priv, "leave\n");
 
 }
author maintainer
#2 lore raw

PATCH 2/2 mac80211: fix scan locking wrt. hw scan

In reply to Johannes Berg

From: Johannes Berg <[email protected]>

Releasing the scan mutex while starting scans can lead to unexpected things happening, so we shouldn't do that. Fix that and hold the mutex across the scan triggering.

---
 net/mac80211/scan.c |   14 --------------
 1 file changed, 14 deletions(-)
net/mac80211/scan.c +0 -14
@@ -401,19 +401,7 @@ static int __ieee80211_start_scan(struct
 	else
 		__set_bit(SCAN_SW_SCANNING, &local->scanning);
 
-	/*
-	 * Kicking off the scan need not be protected,
-	 * only the scan variable stuff, since now
-	 * local->scan_req is assigned and other callers
-	 * will abort their scan attempts.
-	 *
-	 * This avoids too many locking dependencies
-	 * so that the scan completed calls have more
-	 * locking freedom.
-	 */
-
 	ieee80211_recalc_idle(local);
-	mutex_unlock(&local->scan_mtx);
 
 	if (local->ops->hw_scan) {
 		WARN_ON(!ieee80211_prep_hw_scan(local));
@@ -421,8 +409,6 @@ static int __ieee80211_start_scan(struct
 	} else
 		rc = ieee80211_start_sw_scan(local);
 
-	mutex_lock(&local->scan_mtx);
-
 	if (rc) {
 		kfree(local->hw_scan_req);
 		local->hw_scan_req = NULL;
author maintainer
#3 lore raw
In reply to Johannes Berg
Quoted 2 lines · Johannes Berg

Technically these are 2.6.35 regressions, but
I guess it's a little late?

Wait, no, only the iwlwifi one is a regression.

johannes