Skip to contentSkip to conversation

iwlwifi: abort scan when restarting

RFC PATCH In review Stanislaw Gruszka 2 participants 3 messages Aug 9, 2010 – Aug 10, 2010 Networking drivers (wireless)

lore
Share & tools
lore

Stanislaw Gruszka

author
#0 lore raw

RFC PATCH 1/1 iwlwifi: abort scan when restarting

We have to cancel hw scan when restarting device, otherwise we can get

WARNING: at net/wireless/core.c:614 wdev_cleanup_work+0xb7/0xf0

After warning, any further scan request from cfg80211 are ignored by mac80211 with EBUSY error, because from mac80211 perspective we still are performing scanning. This looks like wireless device hung, as NetworkManager can not perform successful scan and not allow to establish a new connection.

When firmware die we can not send any new command to device, in such case we just call ieee80211_scan_completed(priv->hw, true) to finish scanning in mac80211.

iwl_scan_cancel_timeout() will not success if we are performing it from work handler, because workqueue can not fire new work when still other one is performed. To prevent such situation schedule priv->abort_scan to generic workqueue. Also increase sleep time to have better change abort_scan work complete.

Additionally print message when iwl_scan_cancel_timeout() fail.

Signed-off-by Stanislaw Gruszka <[email protected]>

---
Patch was compile tested only on wireless-testing tree as this tree hangs on my machine (without the patch, and I did not have time to investigate that hang). I tested a bit modified patch on older kernel.

 drivers/net/wireless/iwlwifi/iwl-agn.c      |   11 +++++++++++
 drivers/net/wireless/iwlwifi/iwl-scan.c     |    9 +++++----
 drivers/net/wireless/iwlwifi/iwl3945-base.c |   11 +++++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)
drivers/net/wireless/iwlwifi/iwl-agn.c +11 -0
index 35337b1..b6dfeaf 100644
@@ -3047,13 +3047,24 @@ static void iwl_bg_restart(struct work_struct *data)
 		return;
 
 	if (test_and_clear_bit(STATUS_FW_ERROR, &priv->status)) {
+		bool scan_pending = false;
+
 		mutex_lock(&priv->mutex);
 		priv->vif = NULL;
 		priv->is_open = 0;
+		if (test_bit(STATUS_SCANNING, &priv->status) &&
+		    !priv->is_internal_short_scan)
+			scan_pending = true;
 		mutex_unlock(&priv->mutex);
+
+		if (scan_pending)
+			ieee80211_scan_completed(priv->hw, true);
 		iwl_down(priv);
 		ieee80211_restart_hw(priv->hw);
 	} else {
+		mutex_lock(&priv->mutex);
+		iwl_scan_cancel_timeout(priv, 200);
+		mutex_unlock(&priv->mutex);
 		iwl_down(priv);
 
 		if (test_bit(STATUS_EXIT_PENDING, &priv->status))
drivers/net/wireless/iwlwifi/iwl-scan.c +5 -4
index a4b3663..33ed3c8 100644
@@ -71,8 +71,7 @@ int iwl_scan_cancel(struct iwl_priv *priv)
 	if (test_bit(STATUS_SCANNING, &priv->status)) {
 		if (!test_and_set_bit(STATUS_SCAN_ABORTING, &priv->status)) {
 			IWL_DEBUG_SCAN(priv, "Queuing scan abort.\n");
-			queue_work(priv->workqueue, &priv->abort_scan);
-
+ 			schedule_work(&priv->abort_scan);
 		} else
 			IWL_DEBUG_SCAN(priv, "Scan abort already in progress.\n");
 

@@ -98,12 +97,14 @@ int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 		mutex_unlock(&priv->mutex);
 		while (!time_after(jiffies, now + msecs_to_jiffies(ms)) &&
 				test_bit(STATUS_SCANNING, &priv->status))
-			msleep(1);
+			msleep(10);
 		mutex_lock(&priv->mutex);
 
-		return test_bit(STATUS_SCANNING, &priv->status);
+		ret = test_bit(STATUS_SCANNING, &priv->status);
 	}
 
+	if (ret)
+		IWL_WARN(priv, "Could not cancel scan after %lu ms\n", ms);
 	return ret;
 }
 EXPORT_SYMBOL(iwl_scan_cancel_timeout);
drivers/net/wireless/iwlwifi/iwl3945-base.c +11 -0
index d24eb47..22e3738 100644
@@ -3032,13 +3032,24 @@ static void iwl3945_bg_restart(struct work_struct *data)
 		return;
 
 	if (test_and_clear_bit(STATUS_FW_ERROR, &priv->status)) {
+		bool scan_pending = false;
+
 		mutex_lock(&priv->mutex);
 		priv->vif = NULL;
 		priv->is_open = 0;
+		if (test_bit(STATUS_SCANNING, &priv->status) &&
+		    !priv->is_internal_short_scan)
+			scan_pending = true;
 		mutex_unlock(&priv->mutex);
+
+		if (scan_pending)
+			ieee80211_scan_completed(priv->hw, true);
 		iwl3945_down(priv);
 		ieee80211_restart_hw(priv->hw);
 	} else {
+		mutex_lock(&priv->mutex);
+		iwl_scan_cancel_timeout(priv, 200);
+		mutex_unlock(&priv->mutex);
 		iwl3945_down(priv);
 
 		if (test_bit(STATUS_EXIT_PENDING, &priv->status))
Signature · 1 line
1.5.5.6
maintainer
#1 lore raw
Quoted 5 lines · Stanislaw Gruszka

iwl_scan_cancel_timeout() will not success if we are performing it from work handler, because workqueue can not fire new work when still other one is performed. To prevent such situation schedule priv->abort_scan to generic workqueue. Also increase sleep time to have better change abort_scan work complete.

I don't like this much, but you're right. However, I think we can get around this by having iwl_scan_cancel_timeout() not call iwl_scan_cancel() but rather do everything that iwl_bg_abort_scan() would do directly, since _timeout() may sleep, while scan_cancel() may not. What do you think?

johannes

author
#2 lore raw
Quoted 8 lines · Johannes Berg
Quoted 5 lines · Stanislaw Gruszka

iwl_scan_cancel_timeout() will not success if we are performing it from work handler, because workqueue can not fire new work when still other one is performed. To prevent such situation schedule priv->abort_scan to generic workqueue. Also increase sleep time to have better change abort_scan work complete.

Quoted 5 lines

I don't like this much, but you're right. However, I think we can get around this by having iwl_scan_cancel_timeout() not call iwl_scan_cancel() but rather do everything that iwl_bg_abort_scan() would do directly, since _timeout() may sleep, while scan_cancel() may not. What do you think?

Make sense to me. I will do this as separate patch.

Stanislaw