2008-10-11 20:31:14

by Elias Oltmanns

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

Bob Copeland <[email protected]> wrote:
> [gmail keeps dropping CCs for some reason]
>
> On Tue, Oct 07, 2008 at 10:52:22PM +0200, Elias Oltmanns wrote:
>> Bitops on sc->status have to be protected by the sc->lock as soon as
>> ieee80211_register_hw() has been called.
>
> Agreed...
>
>> + mutex_lock(&sc->lock);
>> + ath5k_init_leds(sc);
>
> I'd rather leave ath5k_init_leds in attach so it somewhat matches up
> with detach, but yeah I suppose it could use locking due to a
> probe/start race.
>
> The LED flag is not really a status flag compared to the rest, it just
> says whether the hardware supports it or not. Another approach would
> be to separate it from the others.

Fine with me if that's what the people in charge prefer.

>
>> @@ -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);
>
> Unfortunately, I don't believe configure_filter can sleep :(

Right you are, sorry for missing that. So, should we switch to atomic
bitops on sc->status, or should we have a separate field for
ATH_STAT_PROMISC and ATH_STAT_LEDSOFT on which we'd use atomic bitops then?

Regards,

Elias