Re: [ath5k-devel] Oops with current kernel and ath5k

[Elias Oltmanns <eo@nebensachen.de>]

Bob Copeland <me@bobcopeland.com> wrote:
> On Tue, Oct
> Not sure 07, 2008 at 12:44:58PM +0200, Elias Oltmanns wrote:
>> Looking through the code, I'm wondering about the atomicity requirements
>
>> of sc->status. In my opinion, __set_bit() is not permissible in various
>> places (including your use case). But since this is a problem that has
>> been around before, I will send a separate patch once yours has been
>> merged.
>
> Ok, I don't see why it's a problem in this case, but I'll look for 
> your patch.  Note we should be doing sc->status updates under a mutex
> already; if not that's a bug.

Alright, in that case __set_bit() is quite sufficient, of course. In
order to enforce this policy, I'd suggest the following patch.

Regards,

Elias
---
From: Elias Oltmanns <eo@nebensachen.de>
Subject: [PATCH] ath5k: Ensure atomicity of bitops on sc->status

Bitops on sc->status have to be protected by the sc->lock as soon as
ieee80211_register_hw() has been called.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/net/wireless/ath5k/base.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 0676c6d..2c737da 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -490,6 +490,9 @@ ath5k_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto err_ah;
 
+	mutex_lock(&sc->lock);
+	ath5k_init_leds(sc);
+
 	ATH5K_INFO(sc, "Atheros AR%s chip found (MAC: 0x%x, PHY: 0x%x)\n",
 			ath5k_chip_name(AR5K_VERSION_VER,sc->ah->ah_mac_srev),
 					sc->ah->ah_mac_srev,
@@ -541,6 +544,7 @@ ath5k_pci_probe(struct pci_dev *pdev,
 
 	/* ready to process interrupts */
 	__clear_bit(ATH_STAT_INVALID, sc->status);
+	mutex_unlock(&sc->lock);
 
 	return 0;
 err_ah:
@@ -749,8 +753,6 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw)
 		goto err_queues;
 	}
 
-	ath5k_init_leds(sc);
-
 	return 0;
 err_queues:
 	ath5k_txq_release(sc);
@@ -2881,12 +2883,14 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw,
 		AR5K_RX_FILTER_MCAST);
 
 	if (changed_flags & (FIF_PROMISC_IN_BSS | FIF_OTHER_BSS)) {
+		mutex_lock(&sc->lock);
 		if (*new_flags & FIF_PROMISC_IN_BSS) {
 			rfilt |= AR5K_RX_FILTER_PROM;
 			__set_bit(ATH_STAT_PROMISC, sc->status);
 		}
 		else
 			__clear_bit(ATH_STAT_PROMISC, sc->status);
+		mutex_unlock(&sc->lock);
 	}
 
 	/* Note, AR5K_RX_FILTER_MCAST is already enabled */