[PATCH 0/3] Revert of mac80211 scan complete callback move

[PATCH 0/3] Revert of mac80211 scan complete callback move

[Luis R. Rodriguez]

The two virtual wiphy messages seen on both ath9k and mac80211_hwsim was
debugged by Johannes and fixed but required a little more work on the
drivers end. So for now revert that fix and supress the messages.

We can address this for the next kernel release.

Luis R. Rodriguez (3):
  ath9k: remove the two wiphys scanning at the same time message
  mac80211_hwsim: supress scan conflict message
  Revert "mac80211: fix sw scan bracketing"

 drivers/net/wireless/ath/ath9k/main.c |   13 +++++++++----
 drivers/net/wireless/mac80211_hwsim.c |    4 +---
 net/mac80211/scan.c                   |    4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

[PATCH 1/3] ath9k: remove the two wiphys scanning at the same time message

[Luis R. Rodriguez]

When issuing two consecutive scans you could often end up
getting in the logs:

"ath9k: Two wiphys trying to scan at the same time"

This message is due to a race in mac80211 but addressing
that race requires some more major changes on the driver
and perhaps optimizations on mac80211 like removing the
scan complete callback alltogether. Its too late to address
this this kernel release so supress the complaint and annotate
this needs fixing for later.

Cc: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 6cf0410..0429dda 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1994,11 +1994,12 @@ static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
 
 	mutex_lock(&sc->mutex);
 	if (ath9k_wiphy_scanning(sc)) {
-		printk(KERN_DEBUG "ath9k: Two wiphys trying to scan at the "
-		       "same time\n");
 		/*
-		 * Do not allow the concurrent scanning state for now. This
-		 * could be improved with scanning control moved into ath9k.
+		 * There is a race here in mac80211 but fixing it requires
+		 * we revisit how we handle the scan complete callback.
+		 * After mac80211 fixes we will not have configured hardware
+		 * to the home channel nor would we have configured the RX
+		 * filter yet.
 		 */
 		mutex_unlock(&sc->mutex);
 		return;
@@ -2014,6 +2015,10 @@ static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
 	mutex_unlock(&sc->mutex);
 }
 
+/*
+ * XXX: this requires a revisit after the driver
+ * scan_complete gets moved to another place/removed in mac80211.
+ */
 static void ath9k_sw_scan_complete(struct ieee80211_hw *hw)
 {
 	struct ath_wiphy *aphy = hw->priv;
-- 
1.7.0.4

[PATCH 2/3] mac80211_hwsim: supress scan conflict message

[Luis R. Rodriguez]

Johannes already debugged this and determined it was due
to a race condition in mac80211, in order to fix this though
we would need to change drivers quite a bit so for now just
supress this and we'll correct this on the next kernel release.

Cc: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/mac80211_hwsim.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 01ad7f7..d766a5f 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -995,10 +995,8 @@ static void mac80211_hwsim_sw_scan(struct ieee80211_hw *hw)
 
 	mutex_lock(&hwsim->mutex);
 
-	if (hwsim->scanning) {
-		printk(KERN_DEBUG "two hwsim sw_scans detected!\n");
+	if (hwsim->scanning)
 		goto out;
-	}
 
 	printk(KERN_DEBUG "hwsim sw_scan request, prepping stuff\n");
 	hwsim->scanning = true;
-- 
1.7.0.4

Re: [PATCH 2/3] mac80211_hwsim: supress scan conflict message

[Johannes Berg]

On Tue, 2010-07-27 at 16:33 -0400, Luis R. Rodriguez wrote:
> Johannes already debugged this and determined it was due
> to a race condition in mac80211, in order to fix this though
> we would need to change drivers quite a bit so for now just
> supress this and we'll correct this on the next kernel release.

This one can stay for all I care since it's not used in practise by
actual users that get confused :-)

johannes

Re: [PATCH 2/3] mac80211_hwsim: supress scan conflict message

[Luis R. Rodriguez]

On Tue, Jul 27, 2010 at 11:52 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Tue, 2010-07-27 at 16:33 -0400, Luis R. Rodriguez wrote:
>> Johannes already debugged this and determined it was due
>> to a race condition in mac80211, in order to fix this though
>> we would need to change drivers quite a bit so for now just
>> supress this and we'll correct this on the next kernel release.
>
> This one can stay for all I care since it's not used in practise by
> actual users that get confused :-)

Whatever works.

  Luis

[PATCH 3/3] Revert "mac80211: fix sw scan bracketing"

[Luis R. Rodriguez]

This reverts this commit. While in theory the change is
correct the patch does not address current assumptions made
by some drivers, one which is definitley affected is ath9k.

Prior to this change the scan complete callback would be
called after we returned to the home channel and configured
the hardware RX filters. After this change we call the scan
complete callback prior to both the hw config and the config
filter. At least for ath9k this breaks quite a few assumptions
on the callback, leading to disconnects to the AP after every scan
making the driver pretty useless on STA mode. The goal behind
this commit was to address the now understood spurious warnings
from ath9k and mac80211_hwsim on scanning on two wiphys at the
same time but we have now supressed these and will address this
issue in the next kernel release.

When fixing this for good next we must first review the other
driver's dependence on this logic and perhaps consider removal
of the scan complete callback all together.

Cc: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 net/mac80211/scan.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 4dcbf8b..9aa19ec 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -287,8 +287,6 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted)
 	local->scanning = 0;
 	local->scan_channel = NULL;
 
-	drv_sw_scan_complete(local);
-
 	/* we only have to protect scan_req and hw/sw scan */
 	mutex_unlock(&local->scan_mtx);
 
@@ -298,6 +296,8 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted)
 
 	ieee80211_configure_filter(local);
 
+	drv_sw_scan_complete(local);
+
 	ieee80211_offchannel_return(local, true);
 
  done:
-- 
1.7.0.4