2010-08-26 11:35:44

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 3/3] wl12xx: remove unneeded locking

From: Johannes Berg <[email protected]>

With the scan callback now being callable from
any context, these unlocks/locks can go away.
This makes the code easier to understand, since
callers of these functions must no longer be
aware that the mutex may be dropped.

As Stanislaw is working on iwlwifi scanning, I
didn't change it to take advantage of the new
mac80211 semantics.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/wl12xx/wl1251_event.c | 2 --
drivers/net/wireless/wl12xx/wl1251_main.c | 2 --
drivers/net/wireless/wl12xx/wl1271_main.c | 2 --
drivers/net/wireless/wl12xx/wl1271_scan.c | 2 --
4 files changed, 8 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/wl12xx/wl1251_event.c 2010-08-26 13:29:57.000000000 +0200
+++ wireless-testing/drivers/net/wireless/wl12xx/wl1251_event.c 2010-08-26 13:30:03.000000000 +0200
@@ -36,9 +36,7 @@ static int wl1251_event_scan_complete(st
mbox->scheduled_scan_channels);

if (wl->scanning) {
- mutex_unlock(&wl->mutex);
ieee80211_scan_completed(wl->hw, false);
- mutex_lock(&wl->mutex);
wl1251_debug(DEBUG_MAC80211, "mac80211 hw scan completed");
wl->scanning = false;
}
--- wireless-testing.orig/drivers/net/wireless/wl12xx/wl1251_main.c 2010-08-26 13:29:57.000000000 +0200
+++ wireless-testing/drivers/net/wireless/wl12xx/wl1251_main.c 2010-08-26 13:30:03.000000000 +0200
@@ -469,9 +469,7 @@ static void wl1251_op_stop(struct ieee80
WARN_ON(wl->state != WL1251_STATE_ON);

if (wl->scanning) {
- mutex_unlock(&wl->mutex);
ieee80211_scan_completed(wl->hw, true);
- mutex_lock(&wl->mutex);
wl->scanning = false;
}

--- wireless-testing.orig/drivers/net/wireless/wl12xx/wl1271_main.c 2010-08-26 13:29:57.000000000 +0200
+++ wireless-testing/drivers/net/wireless/wl12xx/wl1271_main.c 2010-08-26 13:30:03.000000000 +0200
@@ -948,9 +948,7 @@ static void wl1271_op_remove_interface(s
ieee80211_enable_dyn_ps(wl->vif);

if (wl->scan.state != WL1271_SCAN_STATE_IDLE) {
- mutex_unlock(&wl->mutex);
ieee80211_scan_completed(wl->hw, true);
- mutex_lock(&wl->mutex);
wl->scan.state = WL1271_SCAN_STATE_IDLE;
kfree(wl->scan.scanned_ch);
wl->scan.scanned_ch = NULL;
--- wireless-testing.orig/drivers/net/wireless/wl12xx/wl1271_scan.c 2010-08-26 13:29:57.000000000 +0200
+++ wireless-testing/drivers/net/wireless/wl12xx/wl1271_scan.c 2010-08-26 13:30:03.000000000 +0200
@@ -215,9 +215,7 @@ void wl1271_scan_stm(struct wl1271 *wl)
break;

case WL1271_SCAN_STATE_DONE:
- mutex_unlock(&wl->mutex);
ieee80211_scan_completed(wl->hw, false);
- mutex_lock(&wl->mutex);

kfree(wl->scan.scanned_ch);
wl->scan.scanned_ch = NULL;




2010-08-31 06:24:45

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH 3/3] wl12xx: remove unneeded locking

On Thu, 2010-08-26 at 13:30 +0200, ext Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> With the scan callback now being callable from
> any context, these unlocks/locks can go away.
> This makes the code easier to understand, since
> callers of these functions must no longer be
> aware that the mutex may be dropped.
>
> As Stanislaw is working on iwlwifi scanning, I
> didn't change it to take advantage of the new
> mac80211 semantics.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---

[...]

> --- wireless-testing.orig/drivers/net/wireless/wl12xx/wl1271_main.c 2010-08-26 13:29:57.000000000 +0200
> +++ wireless-testing/drivers/net/wireless/wl12xx/wl1271_main.c 2010-08-26 13:30:03.000000000 +0200
> @@ -948,9 +948,7 @@ static void wl1271_op_remove_interface(s
> ieee80211_enable_dyn_ps(wl->vif);
>
> if (wl->scan.state != WL1271_SCAN_STATE_IDLE) {
> - mutex_unlock(&wl->mutex);
> ieee80211_scan_completed(wl->hw, true);
> - mutex_lock(&wl->mutex);
> wl->scan.state = WL1271_SCAN_STATE_IDLE;
> kfree(wl->scan.scanned_ch);
> wl->scan.scanned_ch = NULL;
> --- wireless-testing.orig/drivers/net/wireless/wl12xx/wl1271_scan.c 2010-08-26 13:29:57.000000000 +0200
> +++ wireless-testing/drivers/net/wireless/wl12xx/wl1271_scan.c 2010-08-26 13:30:03.000000000 +0200
> @@ -215,9 +215,7 @@ void wl1271_scan_stm(struct wl1271 *wl)
> break;
>
> case WL1271_SCAN_STATE_DONE:
> - mutex_unlock(&wl->mutex);
> ieee80211_scan_completed(wl->hw, false);
> - mutex_lock(&wl->mutex);
>
> kfree(wl->scan.scanned_ch);
> wl->scan.scanned_ch = NULL;

Yes, the wl1271 part also looks good. This definitely simplifies things
and makes the code more robust.


--
Cheers,
Luca.


2010-08-28 06:22:09

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/3] wl12xx: remove unneeded locking

Johannes Berg <[email protected]> writes:

> From: Johannes Berg <[email protected]>
>
> With the scan callback now being callable from
> any context, these unlocks/locks can go away.
> This makes the code easier to understand, since
> callers of these functions must no longer be
> aware that the mutex may be dropped.
>
> As Stanislaw is working on iwlwifi scanning, I
> didn't change it to take advantage of the new
> mac80211 semantics.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> drivers/net/wireless/wl12xx/wl1251_event.c | 2 --
> drivers/net/wireless/wl12xx/wl1251_main.c | 2 --

Thanks, wl1251 part looks good. I didn't test them yet, but will do
soon.

--
Kalle Valo