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

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

[Sujith.Manoharan]

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

Signed-off-by: Sujith <Sujith.Manoharan@atheros.com>
---
 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


-- 

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

[Johannes Berg]

On Tue, 2010-05-11 at 16:24 +0530, Sujith.Manoharan@atheros.com 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

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

[Sujith.Manoharan]

Johannes Berg wrote:
> On Tue, 2010-05-11 at 16:24 +0530, Sujith.Manoharan@atheros.com 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