2010-05-11 10:54:19

by Sujith

[permalink] [raw]
Subject: [PATCH 1/5] ath9k_htc: Lock sta_notify() callback

Since ->sta_notify() can sleep, protect
the callback with a mutex.

Signed-off-by: Sujith <[email protected]>
---
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index 6c386da..9d371c1 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -1452,6 +1452,8 @@ static void ath9k_htc_sta_notify(struct ieee80211_hw *hw,
struct ath9k_htc_priv *priv = hw->priv;
int ret;

+ mutex_lock(&priv->mutex);
+
switch (cmd) {
case STA_NOTIFY_ADD:
ret = ath9k_htc_add_station(priv, vif, sta);
@@ -1464,6 +1466,8 @@ static void ath9k_htc_sta_notify(struct ieee80211_hw *hw,
default:
break;
}
+
+ mutex_unlock(&priv->mutex);
}

static int ath9k_htc_conf_tx(struct ieee80211_hw *hw, u16 queue,
--
1.7.1


--


2010-05-11 10:57:40

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/5] ath9k_htc: Lock sta_notify() callback

On Tue, 2010-05-11 at 16:24 +0530, [email protected] wrote:
> Since ->sta_notify() can sleep, protect
> the callback with a mutex.

While this may be a correct change, it's not a correct description :)

You should be locking _data_, not _code_, and due to mac80211 the
callback cannot be invoked twice at the same time anyway.

johannes



2010-05-11 11:17:39

by Sujith

[permalink] [raw]
Subject: Re: [PATCH 1/5] ath9k_htc: Lock sta_notify() callback

Johannes Berg wrote:
> On Tue, 2010-05-11 at 16:24 +0530, [email protected] wrote:
> > Since ->sta_notify() can sleep, protect
> > the callback with a mutex.
>
> While this may be a correct change, it's not a correct description :)
>
> You should be locking _data_, not _code_, and due to mac80211 the
> callback cannot be invoked twice at the same time anyway.

It protects all the binary data between lock and unlock. :-)

I encountered a problem with sta_notify() with a script doing
a load/scan/connect/unload cycle and NetworkManager also running,
doing its own scan/connect processing. And at some point,
ath9k_htc_remove_station() failed. Which led me to suspect overlapping
calls to sta_notify().

I do see sta_mtx being taken at all the places where sta_remove() is called
in mac80211, so am not sure how I managed to hit it.

Sujith