[PATCH 0/13] iwlwifi: rewrite iwl-scan.c to avoid race conditions

[PATCH 0/13] iwlwifi: rewrite iwl-scan.c to avoid race conditions

[Stanislaw Gruszka]

Avoid iwlwifi hardware scanning race conditions
that may lead to not call ieee80211_scan_completed() (what in
consequences gives "WARNING: at net/wireless/core.c:614
wdev_cleanup_work+0xb7/0xf0"), or call iee80211_scan_completed() more
then once (what gives " WARNING: at net/mac80211/scan.c:312
ieee80211_scan_completed+0x5f/0x1f1").

First problem (warning in wdev_cleanup_work) make any further scan
request from cfg80211 are ignored by mac80211 with EBUSY error,
hence NetworkManager can not perform successful scan and not allow
to establish a new connection. So after suspend/resume (but maybe
not only then) user is not able to connect to wireless network again.

We can not rely on that the commands (start and abort scan) are
successful. Even if they are successfully send to the hardware, we can
not get back notification from firmware (i.e. firmware hung or was
reseted), or we can get notification when we actually perform abort
scan in driver code or after that.

To assure we call ieee80211_scan_completed() only once when scan
was started we use SCAN_SCANNING bit. Code path, which first clear
STATUS_SCANNING bit will call ieee80211_scan_completed().
We do this in many cases, in scan complete notification, scan
abort, device down, etc. Each time we check SCANNING bit.

[PATCH 01/13] iwlwifi: remove unused conf variables

[Stanislaw Gruszka]

From: Johannes Berg <johannes.berg@intel.com>

There are a number of conf variables that are
unused, remove them.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn-lib.c  |    3 ---
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    6 ------
 2 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index a8f2adf..51a8d7e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -1162,7 +1162,6 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 		.flags = CMD_SIZE_HUGE,
 	};
 	struct iwl_scan_cmd *scan;
-	struct ieee80211_conf *conf = NULL;
 	struct iwl_rxon_context *ctx = &priv->contexts[IWL_RXON_CTX_BSS];
 	u32 rate_flags = 0;
 	u16 cmd_len;
@@ -1179,8 +1178,6 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 	if (vif)
 		ctx = iwl_rxon_ctx_from_vif(vif);
 
-	conf = ieee80211_get_hw_conf(priv->hw);
-
 	cancel_delayed_work(&priv->scan_check);
 
 	if (!iwl_is_ready(priv)) {
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 68e624a..5b742eb 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2569,12 +2569,9 @@ static void __iwl3945_down(struct iwl_priv *priv)
 {
 	unsigned long flags;
 	int exit_pending = test_bit(STATUS_EXIT_PENDING, &priv->status);
-	struct ieee80211_conf *conf = NULL;
 
 	IWL_DEBUG_INFO(priv, DRV_NAME " is going down\n");
 
-	conf = ieee80211_get_hw_conf(priv->hw);
-
 	if (!exit_pending)
 		set_bit(STATUS_EXIT_PENDING, &priv->status);
 
@@ -2828,13 +2825,10 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 		.flags = CMD_SIZE_HUGE,
 	};
 	struct iwl3945_scan_cmd *scan;
-	struct ieee80211_conf *conf = NULL;
 	u8 n_probes = 0;
 	enum ieee80211_band band;
 	bool is_active = false;
 
-	conf = ieee80211_get_hw_conf(priv->hw);
-
 	cancel_delayed_work(&priv->scan_check);
 
 	if (!iwl_is_ready(priv)) {
-- 
1.7.1

[PATCH 02/13] iwlwifi: unify scan start checks

[Stanislaw Gruszka]

From: Johannes Berg <johannes.berg@intel.com>

Rather than duplicating all the checks and even
in case of errors accepting the scan request
from mac80211, we can push the checks to the
caller and in all error cases reject the scan
request right away (rather than accepting and
then saying it was aborted).

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-3945.h     |    2 +-
 drivers/net/wireless/iwlwifi/iwl-agn-lib.c  |   85 ++++--------------
 drivers/net/wireless/iwlwifi/iwl-agn.h      |    2 +-
 drivers/net/wireless/iwlwifi/iwl-core.h     |    2 +-
 drivers/net/wireless/iwlwifi/iwl-scan.c     |  127 ++++++++++++++++-----------
 drivers/net/wireless/iwlwifi/iwl3945-base.c |   74 ++--------------
 6 files changed, 107 insertions(+), 185 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h b/drivers/net/wireless/iwlwifi/iwl-3945.h
index bb2aeeb..98509c5 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.h
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
@@ -295,7 +295,7 @@ extern const struct iwl_channel_info *iwl3945_get_channel_info(
 extern int iwl3945_rs_next_rate(struct iwl_priv *priv, int rate);
 
 /* scanning */
-void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
+int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
 
 /* Requires full declaration of iwl_priv before including */
 #include "iwl-io.h"
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index 51a8d7e..5a2540e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -1154,7 +1154,7 @@ static int iwl_get_channels_for_scan(struct iwl_priv *priv,
 	return added;
 }
 
-void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
+int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 {
 	struct iwl_host_cmd cmd = {
 		.id = REPLY_SCAN_CMD,
@@ -1174,57 +1174,20 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 	int  chan_mod;
 	u8 active_chains;
 	u8 scan_tx_antennas = priv->hw_params.valid_tx_ant;
+	int ret;
+
+	lockdep_assert_held(&priv->mutex);
 
 	if (vif)
 		ctx = iwl_rxon_ctx_from_vif(vif);
 
-	cancel_delayed_work(&priv->scan_check);
-
-	if (!iwl_is_ready(priv)) {
-		IWL_WARN(priv, "request scan called when driver not ready.\n");
-		goto done;
-	}
-
-	/* Make sure the scan wasn't canceled before this queued work
-	 * was given the chance to run... */
-	if (!test_bit(STATUS_SCANNING, &priv->status))
-		goto done;
-
-	/* This should never be called or scheduled if there is currently
-	 * a scan active in the hardware. */
-	if (test_bit(STATUS_SCAN_HW, &priv->status)) {
-		IWL_DEBUG_INFO(priv, "Multiple concurrent scan requests in parallel. "
-			       "Ignoring second request.\n");
-		goto done;
-	}
-
-	if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
-		IWL_DEBUG_SCAN(priv, "Aborting scan due to device shutdown\n");
-		goto done;
-	}
-
-	if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
-		IWL_DEBUG_HC(priv, "Scan request while abort pending.  Queuing.\n");
-		goto done;
-	}
-
-	if (iwl_is_rfkill(priv)) {
-		IWL_DEBUG_HC(priv, "Aborting scan due to RF Kill activation\n");
-		goto done;
-	}
-
-	if (!test_bit(STATUS_READY, &priv->status)) {
-		IWL_DEBUG_HC(priv, "Scan request while uninitialized.  Queuing.\n");
-		goto done;
-	}
-
 	if (!priv->scan_cmd) {
 		priv->scan_cmd = kmalloc(sizeof(struct iwl_scan_cmd) +
 					 IWL_MAX_SCAN_SIZE, GFP_KERNEL);
 		if (!priv->scan_cmd) {
 			IWL_DEBUG_SCAN(priv,
 				       "fail to allocate memory for scan\n");
-			goto done;
+			return -ENOMEM;
 		}
 	}
 	scan = priv->scan_cmd;
@@ -1331,8 +1294,8 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 						IWL_GOOD_CRC_TH_NEVER;
 		break;
 	default:
-		IWL_WARN(priv, "Invalid scan band count\n");
-		goto done;
+		IWL_WARN(priv, "Invalid scan band\n");
+		return -EIO;
 	}
 
 	band = priv->scan_band;
@@ -1412,7 +1375,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 	}
 	if (scan->channel_count == 0) {
 		IWL_DEBUG_SCAN(priv, "channel count %d\n", scan->channel_count);
-		goto done;
+		return -EIO;
 	}
 
 	cmd.len += le16_to_cpu(scan->tx_cmd.len) +
@@ -1422,28 +1385,20 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 
 	set_bit(STATUS_SCAN_HW, &priv->status);
 
-	if (priv->cfg->ops->hcmd->set_pan_params &&
-	    priv->cfg->ops->hcmd->set_pan_params(priv))
-		goto done;
+	if (priv->cfg->ops->hcmd->set_pan_params) {
+		ret = priv->cfg->ops->hcmd->set_pan_params(priv);
+		if (ret)
+			return ret;
+	}
 
-	if (iwl_send_cmd_sync(priv, &cmd))
-		goto done;
+	ret = iwl_send_cmd_sync(priv, &cmd);
+	if (ret) {
+		clear_bit(STATUS_SCAN_HW, &priv->status);
+		if (priv->cfg->ops->hcmd->set_pan_params)
+			priv->cfg->ops->hcmd->set_pan_params(priv);
+	}
 
-	queue_delayed_work(priv->workqueue, &priv->scan_check,
-			   IWL_SCAN_CHECK_WATCHDOG);
-
-	return;
-
- done:
-	/* Cannot perform scan. Make sure we clear scanning
-	* bits from status so next scan request can be performed.
-	* If we don't clear scanning status bit here all next scan
-	* will fail
-	*/
-	clear_bit(STATUS_SCAN_HW, &priv->status);
-	clear_bit(STATUS_SCANNING, &priv->status);
-	/* inform mac80211 scan aborted */
-	queue_work(priv->workqueue, &priv->scan_completed);
+	return ret;
 }
 
 int iwlagn_manage_ibss_station(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.h b/drivers/net/wireless/iwlwifi/iwl-agn.h
index 7c542a8..b0fef62 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.h
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.h
@@ -217,7 +217,7 @@ void iwl_reply_statistics(struct iwl_priv *priv,
 			  struct iwl_rx_mem_buffer *rxb);
 
 /* scan */
-void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
+int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
 
 /* station mgmt */
 int iwlagn_manage_ibss_station(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index f7b57ed..8dab074 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -111,7 +111,7 @@ struct iwl_hcmd_utils_ops {
 				  __le16 fc, __le32 *tx_flags);
 	int  (*calc_rssi)(struct iwl_priv *priv,
 			  struct iwl_rx_phy_res *rx_resp);
-	void (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif);
+	int (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif);
 };
 
 struct iwl_apm_ops {
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 7727f09..01a4907 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -324,19 +324,68 @@ void iwl_init_scan_params(struct iwl_priv *priv)
 }
 EXPORT_SYMBOL(iwl_init_scan_params);
 
-static int iwl_scan_initiate(struct iwl_priv *priv, struct ieee80211_vif *vif)
+static int __must_check iwl_scan_initiate(struct iwl_priv *priv,
+					  struct ieee80211_vif *vif,
+					  bool internal,
+					  enum nl80211_band band)
 {
+	int ret;
+
 	lockdep_assert_held(&priv->mutex);
 
-	IWL_DEBUG_INFO(priv, "Starting scan...\n");
+	if (WARN_ON(!priv->cfg->ops->utils->request_scan))
+		return -EOPNOTSUPP;
+
+	cancel_delayed_work(&priv->scan_check);
+
+	if (!iwl_is_ready(priv)) {
+		IWL_WARN(priv, "request scan called when driver not ready.\n");
+		return -EIO;
+	}
+
+	if (test_bit(STATUS_SCAN_HW, &priv->status)) {
+		IWL_DEBUG_INFO(priv,
+			"Multiple concurrent scan requests in parallel.\n");
+		return -EBUSY;
+	}
+
+	if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
+		IWL_DEBUG_SCAN(priv, "Aborting scan due to device shutdown\n");
+		return -EIO;
+	}
+
+	if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
+		IWL_DEBUG_HC(priv, "Scan request while abort pending.\n");
+		return -EBUSY;
+	}
+
+	if (iwl_is_rfkill(priv)) {
+		IWL_DEBUG_HC(priv, "Aborting scan due to RF Kill activation\n");
+		return -EIO;
+	}
+
+	if (!test_bit(STATUS_READY, &priv->status)) {
+		IWL_DEBUG_HC(priv, "Scan request while uninitialized.\n");
+		return -EBUSY;
+	}
+
+	IWL_DEBUG_INFO(priv, "Starting %sscan...\n",
+			internal ? "internal short " : "");
+
 	set_bit(STATUS_SCANNING, &priv->status);
-	priv->is_internal_short_scan = false;
+
+	priv->is_internal_short_scan = internal;
 	priv->scan_start = jiffies;
+	priv->scan_band = band;
 
-	if (WARN_ON(!priv->cfg->ops->utils->request_scan))
-		return -EOPNOTSUPP;
+	ret = priv->cfg->ops->utils->request_scan(priv, vif);
+	if (ret) {
+		clear_bit(STATUS_SCANNING, &priv->status);
+		return ret;
+	}
 
-	priv->cfg->ops->utils->request_scan(priv, vif);
+	queue_delayed_work(priv->workqueue, &priv->scan_check,
+			   IWL_SCAN_CHECK_WATCHDOG);
 
 	return 0;
 }
@@ -355,12 +404,6 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw,
 
 	mutex_lock(&priv->mutex);
 
-	if (!iwl_is_ready_rf(priv)) {
-		ret = -EIO;
-		IWL_DEBUG_MAC80211(priv, "leave - not ready or exit pending\n");
-		goto out_unlock;
-	}
-
 	if (test_bit(STATUS_SCANNING, &priv->status) &&
 	    !priv->is_internal_short_scan) {
 		IWL_DEBUG_SCAN(priv, "Scan already in progress.\n");
@@ -368,14 +411,7 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw,
 		goto out_unlock;
 	}
 
-	if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
-		IWL_DEBUG_SCAN(priv, "Scan request while abort pending\n");
-		ret = -EAGAIN;
-		goto out_unlock;
-	}
-
 	/* mac80211 will only ask for one band at a time */
-	priv->scan_band = req->channels[0]->band;
 	priv->scan_request = req;
 	priv->scan_vif = vif;
 
@@ -386,7 +422,8 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw,
 	if (priv->is_internal_short_scan)
 		ret = 0;
 	else
-		ret = iwl_scan_initiate(priv, vif);
+		ret = iwl_scan_initiate(priv, vif, false,
+					req->channels[0]->band);
 
 	IWL_DEBUG_MAC80211(priv, "leave\n");
 
@@ -418,31 +455,13 @@ static void iwl_bg_start_internal_scan(struct work_struct *work)
 		goto unlock;
 	}
 
-	if (!iwl_is_ready_rf(priv)) {
-		IWL_DEBUG_SCAN(priv, "not ready or exit pending\n");
-		goto unlock;
-	}
-
 	if (test_bit(STATUS_SCANNING, &priv->status)) {
 		IWL_DEBUG_SCAN(priv, "Scan already in progress.\n");
 		goto unlock;
 	}
 
-	if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
-		IWL_DEBUG_SCAN(priv, "Scan request while abort pending\n");
-		goto unlock;
-	}
-
-	priv->scan_band = priv->band;
-
-	IWL_DEBUG_SCAN(priv, "Start internal short scan...\n");
-	set_bit(STATUS_SCANNING, &priv->status);
-	priv->is_internal_short_scan = true;
-
-	if (WARN_ON(!priv->cfg->ops->utils->request_scan))
-		goto unlock;
-
-	priv->cfg->ops->utils->request_scan(priv, NULL);
+	if (iwl_scan_initiate(priv, NULL, true, priv->band))
+		IWL_DEBUG_SCAN(priv, "failed to start internal short scan\n");
  unlock:
 	mutex_unlock(&priv->mutex);
 }
@@ -536,7 +555,6 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 	struct iwl_priv *priv =
 	    container_of(work, struct iwl_priv, scan_completed);
 	bool internal = false;
-	bool scan_completed = false;
 	struct iwl_rxon_context *ctx;
 
 	IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
@@ -549,16 +567,27 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 		IWL_DEBUG_SCAN(priv, "internal short scan completed\n");
 		internal = true;
 	} else if (priv->scan_request) {
-		scan_completed = true;
 		priv->scan_request = NULL;
 		priv->scan_vif = NULL;
+		ieee80211_scan_completed(priv->hw, false);
 	}
 
 	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
 		goto out;
 
-	if (internal && priv->scan_request)
-		iwl_scan_initiate(priv, priv->scan_vif);
+	if (internal && priv->scan_request) {
+		int err = iwl_scan_initiate(priv, priv->scan_vif, false,
+					priv->scan_request->channels[0]->band);
+
+		if (err) {
+			IWL_DEBUG_SCAN(priv,
+				"failed to initiate pending scan: %d\n", err);
+			priv->scan_request = NULL;
+			priv->scan_vif = NULL;
+			ieee80211_scan_completed(priv->hw, true);
+		} else
+			goto out;
+	}
 
 	/* Since setting the TXPOWER may have been deferred while
 	 * performing the scan, fire one off */
@@ -571,19 +600,11 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 	for_each_context(priv, ctx)
 		iwlcore_commit_rxon(priv, ctx);
 
- out:
 	if (priv->cfg->ops->hcmd->set_pan_params)
 		priv->cfg->ops->hcmd->set_pan_params(priv);
 
+ out:
 	mutex_unlock(&priv->mutex);
-
-	/*
-	 * Do not hold mutex here since this will cause mac80211 to call
-	 * into driver again into functions that will attempt to take
-	 * mutex.
-	 */
-	if (scan_completed)
-		ieee80211_scan_completed(priv->hw, false);
 }
 
 void iwl_setup_scan_deferred_work(struct iwl_priv *priv)
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 5b742eb..3b3686b 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2817,7 +2817,7 @@ static void iwl3945_rfkill_poll(struct work_struct *data)
 
 }
 
-void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
+int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 {
 	struct iwl_host_cmd cmd = {
 		.id = REPLY_SCAN_CMD,
@@ -2828,55 +2828,16 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 	u8 n_probes = 0;
 	enum ieee80211_band band;
 	bool is_active = false;
+	int ret;
 
-	cancel_delayed_work(&priv->scan_check);
-
-	if (!iwl_is_ready(priv)) {
-		IWL_WARN(priv, "request scan called when driver not ready.\n");
-		goto done;
-	}
-
-	/* Make sure the scan wasn't canceled before this queued work
-	 * was given the chance to run... */
-	if (!test_bit(STATUS_SCANNING, &priv->status))
-		goto done;
-
-	/* This should never be called or scheduled if there is currently
-	 * a scan active in the hardware. */
-	if (test_bit(STATUS_SCAN_HW, &priv->status)) {
-		IWL_DEBUG_INFO(priv, "Multiple concurrent scan requests  "
-				"Ignoring second request.\n");
-		goto done;
-	}
-
-	if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
-		IWL_DEBUG_SCAN(priv, "Aborting scan due to device shutdown\n");
-		goto done;
-	}
-
-	if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
-		IWL_DEBUG_HC(priv,
-			"Scan request while abort pending. Queuing.\n");
-		goto done;
-	}
-
-	if (iwl_is_rfkill(priv)) {
-		IWL_DEBUG_HC(priv, "Aborting scan due to RF Kill activation\n");
-		goto done;
-	}
-
-	if (!test_bit(STATUS_READY, &priv->status)) {
-		IWL_DEBUG_HC(priv,
-			"Scan request while uninitialized. Queuing.\n");
-		goto done;
-	}
+	lockdep_assert_held(&priv->mutex);
 
 	if (!priv->scan_cmd) {
 		priv->scan_cmd = kmalloc(sizeof(struct iwl3945_scan_cmd) +
 					 IWL_MAX_SCAN_SIZE, GFP_KERNEL);
 		if (!priv->scan_cmd) {
 			IWL_DEBUG_SCAN(priv, "Fail to allocate scan memory\n");
-			goto done;
+			return -ENOMEM;
 		}
 	}
 	scan = priv->scan_cmd;
@@ -2971,7 +2932,7 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 		break;
 	default:
 		IWL_WARN(priv, "Invalid scan band\n");
-		goto done;
+		return -EIO;
 	}
 
 	if (!priv->is_internal_short_scan) {
@@ -3006,7 +2967,7 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 
 	if (scan->channel_count == 0) {
 		IWL_DEBUG_SCAN(priv, "channel count %d\n", scan->channel_count);
-		goto done;
+		return -EIO;
 	}
 
 	cmd.len += le16_to_cpu(scan->tx_cmd.len) +
@@ -3015,25 +2976,10 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
 	scan->len = cpu_to_le16(cmd.len);
 
 	set_bit(STATUS_SCAN_HW, &priv->status);
-	if (iwl_send_cmd_sync(priv, &cmd))
-		goto done;
-
-	queue_delayed_work(priv->workqueue, &priv->scan_check,
-			   IWL_SCAN_CHECK_WATCHDOG);
-
-	return;
-
- done:
-	/* can not perform scan make sure we clear scanning
-	 * bits from status so next scan request can be performed.
-	 * if we dont clear scanning status bit here all next scan
-	 * will fail
-	*/
-	clear_bit(STATUS_SCAN_HW, &priv->status);
-	clear_bit(STATUS_SCANNING, &priv->status);
-
-	/* inform mac80211 scan aborted */
-	queue_work(priv->workqueue, &priv->scan_completed);
+	ret = iwl_send_cmd_sync(priv, &cmd);
+	if (ret)
+		clear_bit(STATUS_SCAN_HW, &priv->status);
+	return ret;
 }
 
 static void iwl3945_bg_restart(struct work_struct *data)
-- 
1.7.1

Re: [PATCH 02/13] iwlwifi: unify scan start checks

[Guy, Wey-Yi]

Hi Johannes,

On Fri, 2010-09-03 at 04:57 -0700, Stanislaw Gruszka wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Rather than duplicating all the checks and even
> in case of errors accepting the scan request
> from mac80211, we can push the checks to the
> caller and in all error cases reject the scan
> request right away (rather than accepting and
> then saying it was aborted).
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945.h     |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-agn-lib.c  |   85 ++++--------------
>  drivers/net/wireless/iwlwifi/iwl-agn.h      |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-core.h     |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-scan.c     |  127 ++++++++++++++++-----------
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |   74 ++--------------
>  6 files changed, 107 insertions(+), 185 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h b/drivers/net/wireless/iwlwifi/iwl-3945.h
> index bb2aeeb..98509c5 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
> @@ -295,7 +295,7 @@ extern const struct iwl_channel_info *iwl3945_get_channel_info(
>  extern int iwl3945_rs_next_rate(struct iwl_priv *priv, int rate);
> 
>  /* scanning */
> -void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
> +int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
> 
>  /* Requires full declaration of iwl_priv before including */
>  #include "iwl-io.h"
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> index 51a8d7e..5a2540e 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> @@ -1154,7 +1154,7 @@ static int iwl_get_channels_for_scan(struct iwl_priv *priv,
>         return added;
>  }
> 
> -void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> +int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
>  {
>         struct iwl_host_cmd cmd = {
>                 .id = REPLY_SCAN_CMD,
> @@ -1174,57 +1174,20 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
>         int  chan_mod;
>         u8 active_chains;
>         u8 scan_tx_antennas = priv->hw_params.valid_tx_ant;
> +       int ret;
> +
> +       lockdep_assert_held(&priv->mutex);
> 
>         if (vif)
>                 ctx = iwl_rxon_ctx_from_vif(vif);
> 
> -       cancel_delayed_work(&priv->scan_check);
> -
> -       if (!iwl_is_ready(priv)) {
> -               IWL_WARN(priv, "request scan called when driver not ready.\n");
> -               goto done;
> -       }
> -
> -       /* Make sure the scan wasn't canceled before this queued work
> -        * was given the chance to run... */
> -       if (!test_bit(STATUS_SCANNING, &priv->status))
> -               goto done;
> -
> -       /* This should never be called or scheduled if there is currently
> -        * a scan active in the hardware. */
> -       if (test_bit(STATUS_SCAN_HW, &priv->status)) {
> -               IWL_DEBUG_INFO(priv, "Multiple concurrent scan requests in parallel. "
> -                              "Ignoring second request.\n");
> -               goto done;
> -       }
> -
> -       if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
> -               IWL_DEBUG_SCAN(priv, "Aborting scan due to device shutdown\n");
> -               goto done;
> -       }
> -
> -       if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
> -               IWL_DEBUG_HC(priv, "Scan request while abort pending.  Queuing.\n");
> -               goto done;
> -       }
> -
> -       if (iwl_is_rfkill(priv)) {
> -               IWL_DEBUG_HC(priv, "Aborting scan due to RF Kill activation\n");
> -               goto done;
> -       }
> -
> -       if (!test_bit(STATUS_READY, &priv->status)) {
> -               IWL_DEBUG_HC(priv, "Scan request while uninitialized.  Queuing.\n");
> -               goto done;
> -       }
> -
>         if (!priv->scan_cmd) {
>                 priv->scan_cmd = kmalloc(sizeof(struct iwl_scan_cmd) +
>                                          IWL_MAX_SCAN_SIZE, GFP_KERNEL);
>                 if (!priv->scan_cmd) {
>                         IWL_DEBUG_SCAN(priv,
>                                        "fail to allocate memory for scan\n");
> -                       goto done;
> +                       return -ENOMEM;
>                 }
>         }
>         scan = priv->scan_cmd;
> @@ -1331,8 +1294,8 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
>                                                 IWL_GOOD_CRC_TH_NEVER;
>                 break;
>         default:
> -               IWL_WARN(priv, "Invalid scan band count\n");
> -               goto done;
> +               IWL_WARN(priv, "Invalid scan band\n");
> +               return -EIO;
>         }
> 
>         band = priv->scan_band;
> @@ -1412,7 +1375,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
>         }
>         if (scan->channel_count == 0) {
>                 IWL_DEBUG_SCAN(priv, "channel count %d\n", scan->channel_count);
> -               goto done;
> +               return -EIO;
>         }
> 
>         cmd.len += le16_to_cpu(scan->tx_cmd.len) +
> @@ -1422,28 +1385,20 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> 
>         set_bit(STATUS_SCAN_HW, &priv->status);
> 
> -       if (priv->cfg->ops->hcmd->set_pan_params &&
> -           priv->cfg->ops->hcmd->set_pan_params(priv))
> -               goto done;
> +       if (priv->cfg->ops->hcmd->set_pan_params) {
> +               ret = priv->cfg->ops->hcmd->set_pan_params(priv);
> +               if (ret)
> +                       return ret;
> +       }
STATUS_SCAN_HW bit still set here

Wey

RE: [PATCH 02/13] iwlwifi: unify scan start checks

[Berg, Johannes]

> >         set_bit(STATUS_SCAN_HW, &priv->status);
> >
> > -       if (priv->cfg->ops->hcmd->set_pan_params &&
> > -           priv->cfg->ops->hcmd->set_pan_params(priv))
> > -               goto done;
> > +       if (priv->cfg->ops->hcmd->set_pan_params) {
> > +               ret = priv->cfg->ops->hcmd->set_pan_params(priv);
> > +               if (ret)
> > +                       return ret;
> > +       }

> STATUS_SCAN_HW bit still set here

Good catch. Stanislaw, would you mind fixing that? Seems easier than for me to re-send a part of your patch series, but I can do if you prefer.

johannes
--------------------------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland 
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 02/13] iwlwifi: unify scan start checks

[Stanislaw Gruszka]

On Fri, Sep 03, 2010 at 03:25:21PM +0100, Berg, Johannes wrote:
> > >         set_bit(STATUS_SCAN_HW, &priv->status);
> > >
> > > -       if (priv->cfg->ops->hcmd->set_pan_params &&
> > > -           priv->cfg->ops->hcmd->set_pan_params(priv))
> > > -               goto done;
> > > +       if (priv->cfg->ops->hcmd->set_pan_params) {
> > > +               ret = priv->cfg->ops->hcmd->set_pan_params(priv);
> > > +               if (ret)
> > > +                       return ret;
> > > +       }
> 
> > STATUS_SCAN_HW bit still set here
> 
> Good catch. 
+1

>Stanislaw, would you mind fixing that? Seems easier than for me to re-send a part of your patch series, but I can do if you prefer.

I will fix.

Stanislaw

[PATCH 03/13] iwlwifi: move scan completed flags handling

[Stanislaw Gruszka]

From: Johannes Berg <johannes.berg@intel.com>

Move the scan completed flags handling so that we
can notify mac80211 about aborted scans with the
correct status. Also queue the scan_completed work
before the BT status update so that it won't see
the bits still set (unless a new scan was started
in which case that's fine.)

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-scan.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 01a4907..9df8bb8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -222,21 +222,11 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
 		       jiffies_to_msecs(elapsed_jiffies
 					(priv->scan_start, jiffies)));
 
-	/*
-	 * If a request to abort was given, or the scan did not succeed
-	 * then we reset the scan state machine and terminate,
-	 * re-queuing another scan if one has been requested
-	 */
-	if (test_and_clear_bit(STATUS_SCAN_ABORTING, &priv->status))
-		IWL_DEBUG_INFO(priv, "Aborted scan completed.\n");
-
-	IWL_DEBUG_INFO(priv, "Setting scan to off\n");
-
-	clear_bit(STATUS_SCANNING, &priv->status);
+	queue_work(priv->workqueue, &priv->scan_completed);
 
 	if (priv->iw_mode != NL80211_IFTYPE_ADHOC &&
-	    priv->cfg->advanced_bt_coexist && priv->bt_status !=
-	    scan_notif->bt_status) {
+	    priv->cfg->advanced_bt_coexist &&
+	    priv->bt_status != scan_notif->bt_status) {
 		if (scan_notif->bt_status) {
 			/* BT on */
 			if (!priv->bt_ch_announce)
@@ -254,7 +244,6 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
 		priv->bt_status = scan_notif->bt_status;
 		queue_work(priv->workqueue, &priv->bt_traffic_change_work);
 	}
-	queue_work(priv->workqueue, &priv->scan_completed);
 }
 
 void iwl_setup_rx_scan_handlers(struct iwl_priv *priv)
@@ -554,7 +543,7 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 {
 	struct iwl_priv *priv =
 	    container_of(work, struct iwl_priv, scan_completed);
-	bool internal = false;
+	bool internal = false, aborted;
 	struct iwl_rxon_context *ctx;
 
 	IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
@@ -562,6 +551,15 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 	cancel_delayed_work(&priv->scan_check);
 
 	mutex_lock(&priv->mutex);
+
+	aborted = test_and_clear_bit(STATUS_SCAN_ABORTING, &priv->status);
+	if (aborted)
+		IWL_DEBUG_INFO(priv, "Aborted scan completed.\n");
+
+	IWL_DEBUG_INFO(priv, "Setting scan to off\n");
+
+	clear_bit(STATUS_SCANNING, &priv->status);
+
 	if (priv->is_internal_short_scan) {
 		priv->is_internal_short_scan = false;
 		IWL_DEBUG_SCAN(priv, "internal short scan completed\n");
@@ -569,7 +567,7 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 	} else if (priv->scan_request) {
 		priv->scan_request = NULL;
 		priv->scan_vif = NULL;
-		ieee80211_scan_completed(priv->hw, false);
+		ieee80211_scan_completed(priv->hw, aborted);
 	}
 
 	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
-- 
1.7.1

Re: [PATCH 03/13] iwlwifi: move scan completed flags handling

[Guy, Wey-Yi]

Hi Johannes,

On Fri, 2010-09-03 at 04:57 -0700, Stanislaw Gruszka wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Move the scan completed flags handling so that we
> can notify mac80211 about aborted scans with the
> correct status. Also queue the scan_completed work
> before the BT status update so that it won't see
> the bits still set (unless a new scan was started
> in which case that's fine.)
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-scan.c |   30 ++++++++++++++----------------
>  1 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
> index 01a4907..9df8bb8 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-scan.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
> @@ -222,21 +222,11 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
>  		       jiffies_to_msecs(elapsed_jiffies
>  					(priv->scan_start, jiffies)));
>  
> -	/*
> -	 * If a request to abort was given, or the scan did not succeed
> -	 * then we reset the scan state machine and terminate,
> -	 * re-queuing another scan if one has been requested
> -	 */
> -	if (test_and_clear_bit(STATUS_SCAN_ABORTING, &priv->status))
> -		IWL_DEBUG_INFO(priv, "Aborted scan completed.\n");
> -
> -	IWL_DEBUG_INFO(priv, "Setting scan to off\n");
> -
> -	clear_bit(STATUS_SCANNING, &priv->status);
> +	queue_work(priv->workqueue, &priv->scan_completed);
>  
>  	if (priv->iw_mode != NL80211_IFTYPE_ADHOC &&
> -	    priv->cfg->advanced_bt_coexist && priv->bt_status !=
> -	    scan_notif->bt_status) {
> +	    priv->cfg->advanced_bt_coexist &&
> +	    priv->bt_status != scan_notif->bt_status) {
>  		if (scan_notif->bt_status) {
>  			/* BT on */
>  			if (!priv->bt_ch_announce)
> @@ -254,7 +244,6 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
>  		priv->bt_status = scan_notif->bt_status;
>  		queue_work(priv->workqueue, &priv->bt_traffic_change_work);
>  	}
> -	queue_work(priv->workqueue, &priv->scan_completed);
>  }
>  
>  void iwl_setup_rx_scan_handlers(struct iwl_priv *priv)
> @@ -554,7 +543,7 @@ static void iwl_bg_scan_completed(struct work_struct *work)
>  {
>  	struct iwl_priv *priv =
>  	    container_of(work, struct iwl_priv, scan_completed);
> -	bool internal = false;
> +	bool internal = false, aborted;
>  	struct iwl_rxon_context *ctx;
>  
>  	IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
> @@ -562,6 +551,15 @@ static void iwl_bg_scan_completed(struct work_struct *work)
>  	cancel_delayed_work(&priv->scan_check);
>  
>  	mutex_lock(&priv->mutex);
> +
> +	aborted = test_and_clear_bit(STATUS_SCAN_ABORTING, &priv->status);
> +	if (aborted)
> +		IWL_DEBUG_INFO(priv, "Aborted scan completed.\n");
> +
> +	IWL_DEBUG_INFO(priv, "Setting scan to off\n");
> +
> +	clear_bit(STATUS_SCANNING, &priv->status);
> +
>  	if (priv->is_internal_short_scan) {
>  		priv->is_internal_short_scan = false;
>  		IWL_DEBUG_SCAN(priv, "internal short scan completed\n");
> @@ -569,7 +567,7 @@ static void iwl_bg_scan_completed(struct work_struct *work)
>  	} else if (priv->scan_request) {
>  		priv->scan_request = NULL;
>  		priv->scan_vif = NULL;
> -		ieee80211_scan_completed(priv->hw, false);
> +		ieee80211_scan_completed(priv->hw, aborted);
>  	}
>  
>  	if (test_bit(STATUS_EXIT_PENDING, &priv->status))

Not too sure about the flow, just asking is it ok to do all thses before
check "STATUS_EXIT_PENFING"?

Wey

RE: [PATCH 03/13] iwlwifi: move scan completed flags handling

[Berg, Johannes]

> > @@ -554,7 +543,7 @@ static void iwl_bg_scan_completed(struct
> work_struct *work)
> >  {
> >  	struct iwl_priv *priv =
> >  	    container_of(work, struct iwl_priv, scan_completed);
> > -	bool internal = false;
> > +	bool internal = false, aborted;
> >  	struct iwl_rxon_context *ctx;
> >
> >  	IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
> > @@ -562,6 +551,15 @@ static void iwl_bg_scan_completed(struct
> work_struct *work)
> >  	cancel_delayed_work(&priv->scan_check);
> >
> >  	mutex_lock(&priv->mutex);
> > +
> > +	aborted = test_and_clear_bit(STATUS_SCAN_ABORTING, &priv-
> >status);
> > +	if (aborted)
> > +		IWL_DEBUG_INFO(priv, "Aborted scan completed.\n");
> > +
> > +	IWL_DEBUG_INFO(priv, "Setting scan to off\n");
> > +
> > +	clear_bit(STATUS_SCANNING, &priv->status);
> > +
> >  	if (priv->is_internal_short_scan) {
> >  		priv->is_internal_short_scan = false;
> >  		IWL_DEBUG_SCAN(priv, "internal short scan completed\n");
> > @@ -569,7 +567,7 @@ static void iwl_bg_scan_completed(struct
> work_struct *work)
> >  	} else if (priv->scan_request) {
> >  		priv->scan_request = NULL;
> >  		priv->scan_vif = NULL;
> > -		ieee80211_scan_completed(priv->hw, false);
> > +		ieee80211_scan_completed(priv->hw, aborted);
> >  	}
> >
> >  	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
> 
> Not too sure about the flow, just asking is it ok to do all thses
> before check "STATUS_EXIT_PENFING"?

I believe it's necessary, because otherwise in the exit pending case we will never complete with mac80211.

johannes
--------------------------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland 
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

[PATCH 04/13] iwlwifi: cancel scan when down the device

[Stanislaw Gruszka]

Always cancel scan when stooping device and scan is currently pending,
we should newer have scan running after down device.

To assure we start scan cancel from restart work we have to schedule
abort_scan to different workqueue than priv->workqueue.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn.c      |   14 +++-----------
 drivers/net/wireless/iwlwifi/iwl-scan.c     |    2 +-
 drivers/net/wireless/iwlwifi/iwl3945-base.c |   16 ++++------------
 3 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index ad0e67f..0de2401 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -2875,8 +2875,9 @@ static void __iwl_down(struct iwl_priv *priv)
 
 	IWL_DEBUG_INFO(priv, DRV_NAME " is going down\n");
 
-	if (!exit_pending)
-		set_bit(STATUS_EXIT_PENDING, &priv->status);
+	iwl_scan_cancel_timeout(priv, 200);
+
+	exit_pending = test_and_set_bit(STATUS_EXIT_PENDING, &priv->status);
 
 	/* Stop TX queues watchdog. We need to have STATUS_EXIT_PENDING bit set
 	 * to prevent rearm timer */
@@ -3486,15 +3487,6 @@ static void iwl_mac_stop(struct ieee80211_hw *hw)
 
 	priv->is_open = 0;
 
-	if (iwl_is_ready_rf(priv) || test_bit(STATUS_SCAN_HW, &priv->status)) {
-		/* stop mac, cancel any scan request and clear
-		 * RXON_FILTER_ASSOC_MSK BIT
-		 */
-		mutex_lock(&priv->mutex);
-		iwl_scan_cancel_timeout(priv, 100);
-		mutex_unlock(&priv->mutex);
-	}
-
 	iwl_down(priv);
 
 	flush_workqueue(priv->workqueue);
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 9df8bb8..d31ed14 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -71,7 +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");
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 3b3686b..fb894d8 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2568,12 +2568,13 @@ static void iwl3945_cancel_deferred_work(struct iwl_priv *priv);
 static void __iwl3945_down(struct iwl_priv *priv)
 {
 	unsigned long flags;
-	int exit_pending = test_bit(STATUS_EXIT_PENDING, &priv->status);
+	int exit_pending;
 
 	IWL_DEBUG_INFO(priv, DRV_NAME " is going down\n");
 
-	if (!exit_pending)
-		set_bit(STATUS_EXIT_PENDING, &priv->status);
+	iwl_scan_cancel_timeout(priv, 200);
+
+	exit_pending = test_and_set_bit(STATUS_EXIT_PENDING, &priv->status);
 
 	/* Stop TX queues watchdog. We need to have STATUS_EXIT_PENDING bit set
 	 * to prevent rearm timer */
@@ -3173,15 +3174,6 @@ static void iwl3945_mac_stop(struct ieee80211_hw *hw)
 
 	priv->is_open = 0;
 
-	if (iwl_is_ready_rf(priv)) {
-		/* stop mac, cancel any scan request and clear
-		 * RXON_FILTER_ASSOC_MSK BIT
-		 */
-		mutex_lock(&priv->mutex);
-		iwl_scan_cancel_timeout(priv, 100);
-		mutex_unlock(&priv->mutex);
-	}
-
 	iwl3945_down(priv);
 
 	flush_workqueue(priv->workqueue);
-- 
1.7.1

Re: [PATCH 04/13] iwlwifi: cancel scan when down the device

[Johannes Berg]

On Fri, 2010-09-03 at 13:57 +0200, Stanislaw Gruszka wrote:
> Always cancel scan when stooping device and scan is currently pending,
> we should newer have scan running after down device.
> 
> To assure we start scan cancel from restart work we have to schedule
> abort_scan to different workqueue than priv->workqueue.

Hmmm...

> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -2875,8 +2875,9 @@ static void __iwl_down(struct iwl_priv *priv)
>  
>  	IWL_DEBUG_INFO(priv, DRV_NAME " is going down\n");
>  
> -	if (!exit_pending)
> -		set_bit(STATUS_EXIT_PENDING, &priv->status);
> +	iwl_scan_cancel_timeout(priv, 200);

This introduces a locking issue, because iwl_scan_cancel_timeout() drops
the mutex. I suspect you fix that later though, but for example the
restart work uses the mutex to atomically track a few BT related
variables.

johannes

Re: [PATCH 04/13] iwlwifi: cancel scan when down the device

[Guy, Wey-Yi]

Hi Gruszka,

On Fri, 2010-09-03 at 04:57 -0700, Stanislaw Gruszka wrote:
> Always cancel scan when stooping device and scan is currently pending,
> we should newer have scan running after down device.
> 
> To assure we start scan cancel from restart work we have to schedule
> abort_scan to different workqueue than priv->workqueue.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-agn.c      |   14 +++-----------
>  drivers/net/wireless/iwlwifi/iwl-scan.c     |    2 +-
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |   16 ++++------------
>  3 files changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index ad0e67f..0de2401 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -2875,8 +2875,9 @@ static void __iwl_down(struct iwl_priv *priv)
>  
>  	IWL_DEBUG_INFO(priv, DRV_NAME " is going down\n");
>  
> -	if (!exit_pending)
> -		set_bit(STATUS_EXIT_PENDING, &priv->status);
> +	iwl_scan_cancel_timeout(priv, 200);
> +
> +	exit_pending = test_and_set_bit(STATUS_EXIT_PENDING, &priv->status);
>  
>  	/* Stop TX queues watchdog. We need to have STATUS_EXIT_PENDING bit set
>  	 * to prevent rearm timer */
> @@ -3486,15 +3487,6 @@ static void iwl_mac_stop(struct ieee80211_hw *hw)
>  
>  	priv->is_open = 0;
>  
> -	if (iwl_is_ready_rf(priv) || test_bit(STATUS_SCAN_HW, &priv->status)) {
> -		/* stop mac, cancel any scan request and clear
> -		 * RXON_FILTER_ASSOC_MSK BIT
> -		 */
> -		mutex_lock(&priv->mutex);
> -		iwl_scan_cancel_timeout(priv, 100);
> -		mutex_unlock(&priv->mutex);
> -	}
> -
>  	iwl_down(priv);

I see you remove the "mutex" here, not needed?

Wey

Re: [PATCH 04/13] iwlwifi: cancel scan when down the device

[Stanislaw Gruszka]

Hi

On Fri, Sep 03, 2010 at 07:38:17AM -0700, Guy, Wey-Yi wrote:
> > -		mutex_lock(&priv->mutex);
> > -		iwl_scan_cancel_timeout(priv, 100);
> > -		mutex_unlock(&priv->mutex);
> > -	}
> > -
> >  	iwl_down(priv);
> 
> I see you remove the "mutex" here, not needed?

Yes, is needed, this is fixed in patch 8.

Stanislaw

[PATCH 05/13] iwlwifi: use IWL_DEBUG_SCAN

[Stanislaw Gruszka]

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-scan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index d31ed14..5fb49d2 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -158,7 +158,7 @@ static void iwl_rx_reply_scan(struct iwl_priv *priv,
 	struct iwl_scanreq_notification *notif =
 	    (struct iwl_scanreq_notification *)pkt->u.raw;
 
-	IWL_DEBUG_RX(priv, "Scan request status = 0x%x\n", notif->status);
+	IWL_DEBUG_SCAN(priv, "Scan request status = 0x%x\n", notif->status);
 #endif
 }
 
-- 
1.7.1

[PATCH 06/13] iwlwifi: report scan completion when abort fail

[Stanislaw Gruszka]

When we are not able to send abort command to firmware, notify mac80211
that we complete scan, as we will newer do it lately. Check for all
possible errors that low level sending command procedure does not check,
to assure we catch all failures cases.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-scan.c |  131 ++++++++++++++++---------------
 1 files changed, 67 insertions(+), 64 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 5fb49d2..4d2a09b 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -54,34 +54,83 @@
 #define IWL_PASSIVE_DWELL_BASE      (100)
 #define IWL_CHANNEL_TUNE_TIME       5
 
+static int iwl_send_scan_abort(struct iwl_priv *priv)
+{
+	int ret;
+	struct iwl_rx_packet *pkt;
+	struct iwl_host_cmd cmd = {
+		.id = REPLY_SCAN_ABORT_CMD,
+		.flags = CMD_WANT_SKB,
+	};
 
+	/* Exit instantly with error when device is not ready
+	 * to receive scan abort command or it does not perform
+	 * hardware scan currently */
+	if (!test_bit(STATUS_READY, &priv->status) ||
+	    !test_bit(STATUS_GEO_CONFIGURED, &priv->status) ||
+	    !test_bit(STATUS_SCAN_HW, &priv->status) ||
+	    test_bit(STATUS_FW_ERROR, &priv->status) ||
+	    test_bit(STATUS_EXIT_PENDING, &priv->status))
+		return -EIO;
 
-/**
- * iwl_scan_cancel - Cancel any currently executing HW scan
- *
- * NOTE: priv->mutex is not required before calling this function
- */
-int iwl_scan_cancel(struct iwl_priv *priv)
-{
-	if (!test_bit(STATUS_SCAN_HW, &priv->status)) {
-		clear_bit(STATUS_SCANNING, &priv->status);
-		return 0;
+	ret = iwl_send_cmd_sync(priv, &cmd);
+	if (ret)
+		return ret;
+
+	pkt = (struct iwl_rx_packet *)cmd.reply_page;
+	if (pkt->u.status != CAN_ABORT_STATUS) {
+		/* The scan abort will return 1 for success or
+		 * 2 for "failure".  A failure condition can be
+		 * due to simply not being in an active scan which
+		 * can occur if we send the scan abort before we
+		 * the microcode has notified us that a scan is
+		 * completed. */
+		IWL_DEBUG_INFO(priv, "SCAN_ABORT ret %d.\n", pkt->u.status);
+		ret = -EIO;
 	}
 
-	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");
-			schedule_work(&priv->abort_scan);
+	iwl_free_pages(priv, cmd.reply_page);
+	return ret;
+}
 
-		} else
-			IWL_DEBUG_SCAN(priv, "Scan abort already in progress.\n");
+static void iwl_do_scan_abort(struct iwl_priv *priv)
+{
+	int ret;
 
-		return test_bit(STATUS_SCANNING, &priv->status);
+	lockdep_assert_held(&priv->mutex);
+
+	if (!test_bit(STATUS_SCANNING, &priv->status)) {
+		IWL_DEBUG_SCAN(priv, "Not performing scan to abort\n");
+		return;
 	}
 
+	if (test_and_set_bit(STATUS_SCAN_ABORTING, &priv->status)) {
+		IWL_DEBUG_SCAN(priv, "Scan abort in progress\n");
+		return;
+	}
+
+	ret = iwl_send_scan_abort(priv);
+	if (ret) {
+		IWL_DEBUG_SCAN(priv, "Send scan abort failed %d\n", ret);
+		clear_bit(STATUS_SCANNING, &priv->status);
+		clear_bit(STATUS_SCAN_HW, &priv->status);
+		clear_bit(STATUS_SCAN_ABORTING, &priv->status);
+		ieee80211_scan_completed(priv->hw, true);
+	} else
+		IWL_DEBUG_SCAN(priv, "Sucessfully send scan abort\n");
+}
+
+/**
+ * iwl_scan_cancel - Cancel any currently executing HW scan
+ */
+int iwl_scan_cancel(struct iwl_priv *priv)
+{
+	IWL_DEBUG_SCAN(priv, "Queuing abort scan\n");
+	schedule_work(&priv->abort_scan);
 	return 0;
 }
 EXPORT_SYMBOL(iwl_scan_cancel);
+
 /**
  * iwl_scan_cancel_timeout - Cancel any currently executing HW scan
  * @ms: amount of time to wait (in milliseconds) for scan to abort
@@ -108,47 +157,6 @@ int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 }
 EXPORT_SYMBOL(iwl_scan_cancel_timeout);
 
-static int iwl_send_scan_abort(struct iwl_priv *priv)
-{
-	int ret = 0;
-	struct iwl_rx_packet *pkt;
-	struct iwl_host_cmd cmd = {
-		.id = REPLY_SCAN_ABORT_CMD,
-		.flags = CMD_WANT_SKB,
-	};
-
-	/* If there isn't a scan actively going on in the hardware
-	 * then we are in between scan bands and not actually
-	 * actively scanning, so don't send the abort command */
-	if (!test_bit(STATUS_SCAN_HW, &priv->status)) {
-		clear_bit(STATUS_SCAN_ABORTING, &priv->status);
-		return 0;
-	}
-
-	ret = iwl_send_cmd_sync(priv, &cmd);
-	if (ret) {
-		clear_bit(STATUS_SCAN_ABORTING, &priv->status);
-		return ret;
-	}
-
-	pkt = (struct iwl_rx_packet *)cmd.reply_page;
-	if (pkt->u.status != CAN_ABORT_STATUS) {
-		/* The scan abort will return 1 for success or
-		 * 2 for "failure".  A failure condition can be
-		 * due to simply not being in an active scan which
-		 * can occur if we send the scan abort before we
-		 * the microcode has notified us that a scan is
-		 * completed. */
-		IWL_DEBUG_INFO(priv, "SCAN_ABORT returned %d.\n", pkt->u.status);
-		clear_bit(STATUS_SCAN_ABORTING, &priv->status);
-		clear_bit(STATUS_SCAN_HW, &priv->status);
-	}
-
-	iwl_free_pages(priv, cmd.reply_page);
-
-	return ret;
-}
-
 /* Service response to REPLY_SCAN_CMD (0x80) */
 static void iwl_rx_reply_scan(struct iwl_priv *priv,
 			      struct iwl_rx_mem_buffer *rxb)
@@ -527,15 +535,10 @@ static void iwl_bg_abort_scan(struct work_struct *work)
 {
 	struct iwl_priv *priv = container_of(work, struct iwl_priv, abort_scan);
 
-	if (!test_bit(STATUS_READY, &priv->status) ||
-	    !test_bit(STATUS_GEO_CONFIGURED, &priv->status))
-		return;
-
 	cancel_delayed_work(&priv->scan_check);
 
 	mutex_lock(&priv->mutex);
-	if (test_bit(STATUS_SCAN_ABORTING, &priv->status))
-		iwl_send_scan_abort(priv);
+	iwl_do_scan_abort(priv);
 	mutex_unlock(&priv->mutex);
 }
 
-- 
1.7.1

[PATCH 07/13] iwlwifi: do not queue abort_scan work if can sleep

[Stanislaw Gruszka]

Since on timeout version of iwl_scan_cancel procedure we can sleep,
do not have to schedule abort_scan work to begin and perform scanning,
can do this directly. Also now, as we do not queue abort_scan from
restart work anymore, we can queue abort_scan to priv->workqueue.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-scan.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 4d2a09b..6ab7bb3 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -126,7 +126,7 @@ static void iwl_do_scan_abort(struct iwl_priv *priv)
 int iwl_scan_cancel(struct iwl_priv *priv)
 {
 	IWL_DEBUG_SCAN(priv, "Queuing abort scan\n");
-	schedule_work(&priv->abort_scan);
+	queue_work(priv->workqueue, &priv->abort_scan);
 	return 0;
 }
 EXPORT_SYMBOL(iwl_scan_cancel);
@@ -135,25 +135,25 @@ EXPORT_SYMBOL(iwl_scan_cancel);
  * iwl_scan_cancel_timeout - Cancel any currently executing HW scan
  * @ms: amount of time to wait (in milliseconds) for scan to abort
  *
- * NOTE: priv->mutex must be held before calling this function
  */
 int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 {
 	unsigned long now = jiffies;
-	int ret;
 
-	ret = iwl_scan_cancel(priv);
-	if (ret && ms) {
+	lockdep_assert_held(&priv->mutex);
+
+	iwl_do_scan_abort(priv);
+
+	if (ms) {
 		mutex_unlock(&priv->mutex);
 		while (!time_after(jiffies, now + msecs_to_jiffies(ms)) &&
 				test_bit(STATUS_SCANNING, &priv->status))
-			msleep(1);
+			msleep(20);
 		mutex_lock(&priv->mutex);
 
-		return test_bit(STATUS_SCANNING, &priv->status);
 	}
 
-	return ret;
+	return test_bit(STATUS_SCANNING, &priv->status);
 }
 EXPORT_SYMBOL(iwl_scan_cancel_timeout);
 
-- 
1.7.1

[PATCH 08/13] iwlwifi: avoid dropping muttex in iwl_scan_cancel_timeout

[Stanislaw Gruszka]

Don't drop mutex when waiting for scan complete. Use STATUS_HW_SCAN bit
to check if scanning is currency pending, because STATUS_SCANNING will
be cleared only with priv->mutex taken.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-scan.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 6ab7bb3..7e4faaf 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -138,22 +138,21 @@ EXPORT_SYMBOL(iwl_scan_cancel);
  */
 int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 {
-	unsigned long now = jiffies;
+	unsigned long timeout = jiffies + msecs_to_jiffies(ms);
 
 	lockdep_assert_held(&priv->mutex);
 
-	iwl_do_scan_abort(priv);
+	IWL_DEBUG_SCAN(priv, "Scan cancel timeout\n");
 
-	if (ms) {
-		mutex_unlock(&priv->mutex);
-		while (!time_after(jiffies, now + msecs_to_jiffies(ms)) &&
-				test_bit(STATUS_SCANNING, &priv->status))
-			msleep(20);
-		mutex_lock(&priv->mutex);
+	iwl_do_scan_abort(priv);
 
+	while (time_before_eq(jiffies, timeout)) {
+		if (!test_bit(STATUS_SCAN_HW, &priv->status))
+			break;
+		msleep(20);
 	}
 
-	return test_bit(STATUS_SCANNING, &priv->status);
+	return test_bit(STATUS_SCAN_HW, &priv->status);
 }
 EXPORT_SYMBOL(iwl_scan_cancel_timeout);
 
-- 
1.7.1

Re: [PATCH 08/13] iwlwifi: avoid dropping muttex in iwl_scan_cancel_timeout

[Guy, Wey-Yi]

Hi Gruszka,

On Fri, 2010-09-03 at 04:57 -0700, Stanislaw Gruszka wrote:
> Don't drop mutex when waiting for scan complete. Use STATUS_HW_SCAN bit
> to check if scanning is currency pending, because STATUS_SCANNING will
> be cleared only with priv->mutex taken.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-scan.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
> index 6ab7bb3..7e4faaf 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-scan.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
> @@ -138,22 +138,21 @@ EXPORT_SYMBOL(iwl_scan_cancel);
>   */
>  int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
>  {
> -	unsigned long now = jiffies;
> +	unsigned long timeout = jiffies + msecs_to_jiffies(ms);
>  
>  	lockdep_assert_held(&priv->mutex);
>  
> -	iwl_do_scan_abort(priv);
> +	IWL_DEBUG_SCAN(priv, "Scan cancel timeout\n");
>  
> -	if (ms) {
> -		mutex_unlock(&priv->mutex);
> -		while (!time_after(jiffies, now + msecs_to_jiffies(ms)) &&
> -				test_bit(STATUS_SCANNING, &priv->status))
> -			msleep(20);
> -		mutex_lock(&priv->mutex);
> +	iwl_do_scan_abort(priv);
>  
> +	while (time_before_eq(jiffies, timeout)) {
> +		if (!test_bit(STATUS_SCAN_HW, &priv->status))
> +			break;
> +		msleep(20);
>  	}
>  
> -	return test_bit(STATUS_SCANNING, &priv->status);
> +	return test_bit(STATUS_SCAN_HW, &priv->status);
>  }
>  EXPORT_SYMBOL(iwl_scan_cancel_timeout);
>  
Should patch 7 and 8 merge into single patch?

Wey

Re: [PATCH 08/13] iwlwifi: avoid dropping muttex in iwl_scan_cancel_timeout

[Stanislaw Gruszka]

On Fri, Sep 03, 2010 at 07:53:05AM -0700, Guy, Wey-Yi wrote:
> Should patch 7 and 8 merge into single patch?

Ok, I will do.

Stanislaw

[PATCH 09/13] iwlwifi: rewrite scan completion

[Stanislaw Gruszka]

Assure (partially) we call ieee80211_scan_completed() only once when
scan was requested from mac80211.

Code path that first clear STATUS_SCANNING bit is responsible to call
ieee80211_scan_completed(). Before the call, we check if mac80211
really request the scan.

Still persist some cases when we behave wrong, that will be addressed
in the next patches.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-scan.c |   65 +++++++++++++++++++------------
 1 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 7e4faaf..f79fb3e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -93,6 +93,19 @@ static int iwl_send_scan_abort(struct iwl_priv *priv)
 	return ret;
 }
 
+static void iwl_complete_scan(struct iwl_priv *priv, bool aborted)
+{
+	/* check if scan was requested from mac80211 */
+	if (priv->scan_request) {
+		IWL_DEBUG_SCAN(priv, "Complete scan in mac80211\n");
+		ieee80211_scan_completed(priv->hw, aborted);
+	}
+
+	priv->is_internal_short_scan = false;
+	priv->scan_vif = NULL;
+	priv->scan_request = NULL;
+}
+
 static void iwl_do_scan_abort(struct iwl_priv *priv)
 {
 	int ret;
@@ -115,7 +128,7 @@ static void iwl_do_scan_abort(struct iwl_priv *priv)
 		clear_bit(STATUS_SCANNING, &priv->status);
 		clear_bit(STATUS_SCAN_HW, &priv->status);
 		clear_bit(STATUS_SCAN_ABORTING, &priv->status);
-		ieee80211_scan_completed(priv->hw, true);
+		iwl_complete_scan(priv, true);
 	} else
 		IWL_DEBUG_SCAN(priv, "Sucessfully send scan abort\n");
 }
@@ -545,10 +558,11 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 {
 	struct iwl_priv *priv =
 	    container_of(work, struct iwl_priv, scan_completed);
-	bool internal = false, aborted;
+	bool aborted;
 	struct iwl_rxon_context *ctx;
 
-	IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
+	IWL_DEBUG_INFO(priv, "Completed %sscan.\n",
+		       priv->is_internal_short_scan ? "internal short " : "");
 
 	cancel_delayed_work(&priv->scan_check);
 
@@ -558,37 +572,38 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 	if (aborted)
 		IWL_DEBUG_INFO(priv, "Aborted scan completed.\n");
 
-	IWL_DEBUG_INFO(priv, "Setting scan to off\n");
-
-	clear_bit(STATUS_SCANNING, &priv->status);
-
-	if (priv->is_internal_short_scan) {
-		priv->is_internal_short_scan = false;
-		IWL_DEBUG_SCAN(priv, "internal short scan completed\n");
-		internal = true;
-	} else if (priv->scan_request) {
-		priv->scan_request = NULL;
-		priv->scan_vif = NULL;
-		ieee80211_scan_completed(priv->hw, aborted);
+	if (!test_and_clear_bit(STATUS_SCANNING, &priv->status)) {
+		IWL_DEBUG_INFO(priv, "Scan already completed.\n");
+		goto out;
 	}
 
-	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
-		goto out;
+	if (priv->is_internal_short_scan && !aborted) {
+		int err;
 
-	if (internal && priv->scan_request) {
-		int err = iwl_scan_initiate(priv, priv->scan_vif, false,
-					priv->scan_request->channels[0]->band);
+		/* Check if mac80211 requested scan during our internal scan */
+		if (priv->scan_request == NULL)
+			goto out_complete;
 
+		/* If so request a new scan */
+		err = iwl_scan_initiate(priv, priv->scan_vif, false,
+					priv->scan_request->channels[0]->band);
 		if (err) {
 			IWL_DEBUG_SCAN(priv,
 				"failed to initiate pending scan: %d\n", err);
-			priv->scan_request = NULL;
-			priv->scan_vif = NULL;
-			ieee80211_scan_completed(priv->hw, true);
-		} else
-			goto out;
+			aborted = true;
+			goto out_complete;
+		}
+
+		goto out;
 	}
 
+out_complete:
+	iwl_complete_scan(priv, aborted);
+
+	/* Can we still talk to firmware ? */
+	if (!iwl_is_ready_rf(priv))
+		goto out;
+
 	/* Since setting the TXPOWER may have been deferred while
 	 * performing the scan, fire one off */
 	iwl_set_tx_power(priv, priv->tx_power_user_lmt, true);
-- 
1.7.1

[PATCH 10/13] iwlwifi: force scan complete after timeout

[Stanislaw Gruszka]

If we do not get notification from hardware about scan complete after
timeout do mac80211 scan completion anyway. This assure we end scan
in case of firmware hung.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-core.c |   15 +++------------
 drivers/net/wireless/iwlwifi/iwl-scan.c |   20 +++++++++++++++-----
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index 87a2e40..7c1819f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -2041,7 +2041,6 @@ void iwl_mac_remove_interface(struct ieee80211_hw *hw,
 {
 	struct iwl_priv *priv = hw->priv;
 	struct iwl_rxon_context *ctx = iwl_rxon_ctx_from_vif(vif);
-	bool scan_completed = false;
 
 	IWL_DEBUG_MAC80211(priv, "enter\n");
 
@@ -2050,15 +2049,10 @@ void iwl_mac_remove_interface(struct ieee80211_hw *hw,
 	WARN_ON(ctx->vif != vif);
 	ctx->vif = NULL;
 
-	iwl_scan_cancel_timeout(priv, 100);
+	if (priv->scan_vif == vif)
+		iwl_scan_cancel_timeout(priv, 100);
 	iwl_set_mode(priv, vif);
 
-	if (priv->scan_vif == vif) {
-		scan_completed = true;
-		priv->scan_vif = NULL;
-		priv->scan_request = NULL;
-	}
-
 	/*
 	 * When removing the IBSS interface, overwrite the
 	 * BT traffic load with the stored one from the last
@@ -2072,9 +2066,6 @@ void iwl_mac_remove_interface(struct ieee80211_hw *hw,
 	memset(priv->bssid, 0, ETH_ALEN);
 	mutex_unlock(&priv->mutex);
 
-	if (scan_completed)
-		ieee80211_scan_completed(priv->hw, true);
-
 	IWL_DEBUG_MAC80211(priv, "leave\n");
 
 }
@@ -2255,6 +2246,7 @@ void iwl_mac_reset_tsf(struct ieee80211_hw *hw)
 
 	spin_unlock_irqrestore(&priv->lock, flags);
 
+	iwl_scan_cancel_timeout(priv, 100);
 	if (!iwl_is_ready_rf(priv)) {
 		IWL_DEBUG_MAC80211(priv, "leave - not ready\n");
 		mutex_unlock(&priv->mutex);
@@ -2264,7 +2256,6 @@ void iwl_mac_reset_tsf(struct ieee80211_hw *hw)
 	/* we are restarting association process
 	 * clear RXON_FILTER_ASSOC_MSK bit
 	 */
-	iwl_scan_cancel_timeout(priv, 100);
 	ctx->staging.filter_flags &= ~RXON_FILTER_ASSOC_MSK;
 	iwlcore_commit_rxon(priv, ctx);
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index f79fb3e..fcc826e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -106,6 +106,15 @@ static void iwl_complete_scan(struct iwl_priv *priv, bool aborted)
 	priv->scan_request = NULL;
 }
 
+static void iwl_force_scan_end(struct iwl_priv *priv)
+{
+	IWL_DEBUG_SCAN(priv, "Forcing scan end\n");
+	clear_bit(STATUS_SCANNING, &priv->status);
+	clear_bit(STATUS_SCAN_HW, &priv->status);
+	clear_bit(STATUS_SCAN_ABORTING, &priv->status);
+	iwl_complete_scan(priv, true);
+}
+
 static void iwl_do_scan_abort(struct iwl_priv *priv)
 {
 	int ret;
@@ -125,10 +134,7 @@ static void iwl_do_scan_abort(struct iwl_priv *priv)
 	ret = iwl_send_scan_abort(priv);
 	if (ret) {
 		IWL_DEBUG_SCAN(priv, "Send scan abort failed %d\n", ret);
-		clear_bit(STATUS_SCANNING, &priv->status);
-		clear_bit(STATUS_SCAN_HW, &priv->status);
-		clear_bit(STATUS_SCAN_ABORTING, &priv->status);
-		iwl_complete_scan(priv, true);
+		iwl_force_scan_end(priv);
 	} else
 		IWL_DEBUG_SCAN(priv, "Sucessfully send scan abort\n");
 }
@@ -151,6 +157,7 @@ EXPORT_SYMBOL(iwl_scan_cancel);
  */
 int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 {
+	int ret;
 	unsigned long timeout = jiffies + msecs_to_jiffies(ms);
 
 	lockdep_assert_held(&priv->mutex);
@@ -165,7 +172,10 @@ int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 		msleep(20);
 	}
 
-	return test_bit(STATUS_SCAN_HW, &priv->status);
+	ret = test_bit(STATUS_SCAN_HW, &priv->status);
+	if (ret)
+		iwl_force_scan_end(priv);
+	return ret;
 }
 EXPORT_SYMBOL(iwl_scan_cancel_timeout);
 
-- 
1.7.1

Re: [PATCH 10/13] iwlwifi: force scan complete after timeout

[Johannes Berg]

On Fri, 2010-09-03 at 13:57 +0200, Stanislaw Gruszka wrote:
> If we do not get notification from hardware about scan complete after
> timeout do mac80211 scan completion anyway. This assure we end scan
> in case of firmware hung.


> -	return test_bit(STATUS_SCAN_HW, &priv->status);
> +	ret = test_bit(STATUS_SCAN_HW, &priv->status);
> +	if (ret)
> +		iwl_force_scan_end(priv);
> +	return ret;

After this, is there any point in the return value from
iwl_scan_cancel_timeout()?

johannes

[PATCH 11/13] iwlwifi: assure we complete scan in scan_abort and scan_check works

[Stanislaw Gruszka]

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-scan.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index fcc826e..df61273 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -490,18 +490,8 @@ static void iwl_bg_scan_check(struct work_struct *data)
 	struct iwl_priv *priv =
 	    container_of(data, struct iwl_priv, scan_check.work);
 
-	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
-		return;
-
 	mutex_lock(&priv->mutex);
-	if (test_bit(STATUS_SCANNING, &priv->status) &&
-	    !test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
-		IWL_DEBUG_SCAN(priv, "Scan completion watchdog (%dms)\n",
-			       jiffies_to_msecs(IWL_SCAN_CHECK_WATCHDOG));
-
-		if (!test_bit(STATUS_EXIT_PENDING, &priv->status))
-			iwl_send_scan_abort(priv);
-	}
+	iwl_scan_cancel_timeout(priv, 200);
 	mutex_unlock(&priv->mutex);
 }
 
@@ -560,7 +550,7 @@ static void iwl_bg_abort_scan(struct work_struct *work)
 	cancel_delayed_work(&priv->scan_check);
 
 	mutex_lock(&priv->mutex);
-	iwl_do_scan_abort(priv);
+	iwl_scan_cancel_timeout(priv, 200);
 	mutex_unlock(&priv->mutex);
 }
 
-- 
1.7.1

[PATCH 12/13] iwlwifi: do not force complete scan too early

[Stanislaw Gruszka]

Currently we force scan complete at the end of iwl_scan_cancel_timeout
function. This cause race condition when we can get a new scan request
from mac80211 and complete it by iwl_bg_complete from older scan. Change
code to force scan complete only when really needed: device goes down,
interface is removed or scan timeout occurs.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn.c      |    6 +++-
 drivers/net/wireless/iwlwifi/iwl-core.c     |    6 +++-
 drivers/net/wireless/iwlwifi/iwl-core.h     |    2 +
 drivers/net/wireless/iwlwifi/iwl-scan.c     |   40 ++++++++++++++++++++-------
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    4 +-
 5 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 0de2401..9ef48f8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -4054,13 +4054,15 @@ static void iwl_cancel_deferred_work(struct iwl_priv *priv)
 		priv->cfg->ops->lib->cancel_deferred_work(priv);
 
 	cancel_delayed_work_sync(&priv->init_alive_start);
-	cancel_delayed_work(&priv->scan_check);
-	cancel_work_sync(&priv->start_internal_scan);
 	cancel_delayed_work(&priv->alive_start);
 	cancel_work_sync(&priv->run_time_calib_work);
 	cancel_work_sync(&priv->beacon_update);
+
+	iwl_cancel_scan_deferred_work(priv);
+
 	cancel_work_sync(&priv->bt_full_concurrency);
 	cancel_work_sync(&priv->bt_runtime_config);
+
 	del_timer_sync(&priv->statistics_periodic);
 	del_timer_sync(&priv->ucode_trace);
 }
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index 7c1819f..57d157f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -2049,8 +2049,10 @@ void iwl_mac_remove_interface(struct ieee80211_hw *hw,
 	WARN_ON(ctx->vif != vif);
 	ctx->vif = NULL;
 
-	if (priv->scan_vif == vif)
-		iwl_scan_cancel_timeout(priv, 100);
+	if (priv->scan_vif == vif) {
+		iwl_scan_cancel_timeout(priv, 200);
+		iwl_force_scan_end(priv);
+	}
 	iwl_set_mode(priv, vif);
 
 	/*
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index 8dab074..f1d40c6 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -553,6 +553,7 @@ static inline __le32 iwl_hw_set_rate_n_flags(u8 rate, u32 flags)
 void iwl_init_scan_params(struct iwl_priv *priv);
 int iwl_scan_cancel(struct iwl_priv *priv);
 int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms);
+void iwl_force_scan_end(struct iwl_priv *priv);
 int iwl_mac_hw_scan(struct ieee80211_hw *hw,
 		    struct ieee80211_vif *vif,
 		    struct cfg80211_scan_request *req);
@@ -568,6 +569,7 @@ u16 iwl_get_passive_dwell_time(struct iwl_priv *priv,
 			       enum ieee80211_band band,
 			       struct ieee80211_vif *vif);
 void iwl_setup_scan_deferred_work(struct iwl_priv *priv);
+void iwl_cancel_scan_deferred_work(struct iwl_priv *priv);
 
 /* For faster active scanning, scan will move to the next channel if fewer than
  * PLCP_QUIET_THRESH packets are heard on this channel within
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index df61273..10b0ccc 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -106,14 +106,22 @@ static void iwl_complete_scan(struct iwl_priv *priv, bool aborted)
 	priv->scan_request = NULL;
 }
 
-static void iwl_force_scan_end(struct iwl_priv *priv)
+void iwl_force_scan_end(struct iwl_priv *priv)
 {
+	lockdep_assert_held(&priv->mutex);
+
+	if (!test_bit(STATUS_SCANNING, &priv->status)) {
+		IWL_DEBUG_SCAN(priv, "Forcing scan end while not scanning\n");
+		return;
+	}
+
 	IWL_DEBUG_SCAN(priv, "Forcing scan end\n");
 	clear_bit(STATUS_SCANNING, &priv->status);
 	clear_bit(STATUS_SCAN_HW, &priv->status);
 	clear_bit(STATUS_SCAN_ABORTING, &priv->status);
 	iwl_complete_scan(priv, true);
 }
+EXPORT_SYMBOL(iwl_force_scan_end);
 
 static void iwl_do_scan_abort(struct iwl_priv *priv)
 {
@@ -157,7 +165,6 @@ EXPORT_SYMBOL(iwl_scan_cancel);
  */
 int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 {
-	int ret;
 	unsigned long timeout = jiffies + msecs_to_jiffies(ms);
 
 	lockdep_assert_held(&priv->mutex);
@@ -172,10 +179,7 @@ int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms)
 		msleep(20);
 	}
 
-	ret = test_bit(STATUS_SCAN_HW, &priv->status);
-	if (ret)
-		iwl_force_scan_end(priv);
-	return ret;
+	return test_bit(STATUS_SCAN_HW, &priv->status);
 }
 EXPORT_SYMBOL(iwl_scan_cancel_timeout);
 
@@ -490,8 +494,11 @@ static void iwl_bg_scan_check(struct work_struct *data)
 	struct iwl_priv *priv =
 	    container_of(data, struct iwl_priv, scan_check.work);
 
+	/* Since we are here firmware does not finish scan and
+	 * most likely is in bad shape, so we don't bother to
+	 * send abort command, just force scan complete to mac80211 */
 	mutex_lock(&priv->mutex);
-	iwl_scan_cancel_timeout(priv, 200);
+	iwl_force_scan_end(priv);
 	mutex_unlock(&priv->mutex);
 }
 
@@ -547,8 +554,8 @@ static void iwl_bg_abort_scan(struct work_struct *work)
 {
 	struct iwl_priv *priv = container_of(work, struct iwl_priv, abort_scan);
 
-	cancel_delayed_work(&priv->scan_check);
-
+	/* We keep scan_check work queued in case when firmware will not
+	 * report back scan completed notification */
 	mutex_lock(&priv->mutex);
 	iwl_scan_cancel_timeout(priv, 200);
 	mutex_unlock(&priv->mutex);
@@ -564,7 +571,7 @@ static void iwl_bg_scan_completed(struct work_struct *work)
 	IWL_DEBUG_INFO(priv, "Completed %sscan.\n",
 		       priv->is_internal_short_scan ? "internal short " : "");
 
-	cancel_delayed_work(&priv->scan_check);
+	cancel_delayed_work_sync(&priv->scan_check);
 
 	mutex_lock(&priv->mutex);
 
@@ -631,3 +638,16 @@ void iwl_setup_scan_deferred_work(struct iwl_priv *priv)
 }
 EXPORT_SYMBOL(iwl_setup_scan_deferred_work);
 
+void iwl_cancel_scan_deferred_work(struct iwl_priv *priv)
+{
+	cancel_work_sync(&priv->start_internal_scan);
+	cancel_work_sync(&priv->abort_scan);
+	cancel_work_sync(&priv->scan_completed);
+
+	if (cancel_delayed_work_sync(&priv->scan_check)) {
+		mutex_lock(&priv->mutex);
+		iwl_force_scan_end(priv);
+		mutex_unlock(&priv->mutex);
+	}
+}
+EXPORT_SYMBOL(iwl_cancel_scan_deferred_work);
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index fb894d8..6b8f654 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -3763,10 +3763,10 @@ static void iwl3945_cancel_deferred_work(struct iwl_priv *priv)
 	iwl3945_hw_cancel_deferred_work(priv);
 
 	cancel_delayed_work_sync(&priv->init_alive_start);
-	cancel_delayed_work(&priv->scan_check);
 	cancel_delayed_work(&priv->alive_start);
-	cancel_work_sync(&priv->start_internal_scan);
 	cancel_work_sync(&priv->beacon_update);
+
+	iwl_cancel_scan_deferred_work(priv);
 }
 
 static struct attribute *iwl3945_sysfs_entries[] = {
-- 
1.7.1

[PATCH 13/13] mac80211: wait for scan work complete before restarting hw

[Stanislaw Gruszka]

This is needed to avoid warning in ieee80211_restart_hw about hardware
scan in progress.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/main.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 4935b84..bf0eb6a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -305,6 +305,9 @@ void ieee80211_restart_hw(struct ieee80211_hw *hw)
 
 	trace_api_restart_hw(local);
 
+	/* wait for scan work complete */
+	flush_workqueue(local->workqueue);
+
 	WARN(test_bit(SCAN_HW_SCANNING, &local->scanning),
 		"%s called with hardware scan in progress\n", __func__);
 
-- 
1.7.1

Re: [PATCH 0/13] iwlwifi: rewrite iwl-scan.c to avoid race conditions

[Johannes Berg]

On Fri, 2010-09-03 at 13:57 +0200, Stanislaw Gruszka wrote:

[lots of patches]

Looks OK to me, apart from what I've pointed out, and I hope you've
tested it :-) I'm not an expert on the scanning code, to be honest.

johannes

Re: [PATCH 0/13] iwlwifi: rewrite iwl-scan.c to avoid race conditions

[Stanislaw Gruszka]

On Fri, Sep 03, 2010 at 02:21:58PM +0200, Johannes Berg wrote:
> On Fri, 2010-09-03 at 13:57 +0200, Stanislaw Gruszka wrote:
> 
> [lots of patches]
> 
> Looks OK to me, apart from what I've pointed out,

All concerns are fixed by later patches.

> and I hope you've
> tested it :-) I'm not an expert on the scanning code, to be honest.

I used below script on iwl3945. I plan to make some more testing, on
iwlagn and with different scenarios. But patches improve things, it was
easy to reproduce problem with script on iwl3945, with patches problem
seems to gone.

#!/bin/bash

export my_dev=`iw dev | grep Interface | head -1 | awk '{ print $2 }'`
export my_phy=`iw dev | grep phy  | head -1 | tr -d '#'`

(while true; do iwlist ${my_dev} scan ; sleep 3 ; done) &
b1=$!
(while true; do ifconfig ${my_dev} down ; ifconfig $my_dev up ; sleep 8 ; done) &
b2=$!

mount -t debugfs debugfs /sys/kernel/debug 2> /dev/null > /dev/null

(while true; do echo 1 > /sys/kernel/debug/ieee80211/${my_phy}/iwl3945/debug/force_reset ; sleep 5 ; done ) &
b3=$!
(while true; do echo 0 > /sys/kernel/debug/ieee80211/${my_phy}/iwl3945/debug/force_reset ; sleep 13 ; done ) &
b4=$!

function exit_script ()
{
	kill -KILL $b1
	kill -KILL $b2
	kill -KILL $b3
	kill -KILL $b4
	kill -KILL $$
}

trap exit_script INT TERM EXIT

while true ; do sleep 1 ; done

exit_script

Re: [PATCH 0/13] iwlwifi: rewrite iwl-scan.c to avoid race conditions

[Johannes Berg]

On Fri, 2010-09-03 at 14:55 +0200, Stanislaw Gruszka wrote:
> On Fri, Sep 03, 2010 at 02:21:58PM +0200, Johannes Berg wrote:
> > On Fri, 2010-09-03 at 13:57 +0200, Stanislaw Gruszka wrote:
> > 
> > [lots of patches]
> > 
> > Looks OK to me, apart from what I've pointed out,
> 
> All concerns are fixed by later patches.

Yeah, good point.

> > and I hope you've
> > tested it :-) I'm not an expert on the scanning code, to be honest.
> 
> I used below script on iwl3945. I plan to make some more testing, on
> iwlagn and with different scenarios. But patches improve things, it was
> easy to reproduce problem with script on iwl3945, with patches problem
> seems to gone.

Nice :-) AGN testing would be great.

johannes

Re: [PATCH 0/13] iwlwifi: rewrite iwl-scan.c to avoid race conditions

[Guy, Wey-Yi]

Hi Gruszka,

On Fri, 2010-09-03 at 04:57 -0700, Stanislaw Gruszka wrote:
> Avoid iwlwifi hardware scanning race conditions
> that may lead to not call ieee80211_scan_completed() (what in
> consequences gives "WARNING: at net/wireless/core.c:614
> wdev_cleanup_work+0xb7/0xf0"), or call iee80211_scan_completed() more
> then once (what gives " WARNING: at net/mac80211/scan.c:312
> ieee80211_scan_completed+0x5f/0x1f1").
> 
> First problem (warning in wdev_cleanup_work) make any further scan
> request from cfg80211 are ignored by mac80211 with EBUSY error,
> hence NetworkManager can not perform successful scan and not allow
> to establish a new connection. So after suspend/resume (but maybe
> not only then) user is not able to connect to wireless network again.
> 
> We can not rely on that the commands (start and abort scan) are
> successful. Even if they are successfully send to the hardware, we can
> not get back notification from firmware (i.e. firmware hung or was
> reseted), or we can get notification when we actually perform abort
> scan in driver code or after that.
> 
> To assure we call ieee80211_scan_completed() only once when scan
> was started we use SCAN_SCANNING bit. Code path, which first clear
> STATUS_SCANNING bit will call ieee80211_scan_completed().
> We do this in many cases, in scan complete notification, scan
> abort, device down, etc. Each time we check SCANNING bit.
> 

A lot of changes, great jobs. How much test you done for different
devices? Scan is a very important function and I am also not expert in
this area.

Thanks
Wey

Re: [PATCH 0/13] iwlwifi: rewrite iwl-scan.c to avoid race conditions

[Stanislaw Gruszka]

On Fri, Sep 03, 2010 at 08:04:53AM -0700, Guy, Wey-Yi wrote:
> A lot of changes, great jobs. How much test you done for different
> devices? Scan is a very important function and I am also not expert in
> this area.

I tested only on 3945. I will fix problems from patch 2 and 7-8, do 
additional tests on other devices that I have (4965 and 5300) and will
repost soon.

Thanks
Stanislaw