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

[Elias Oltmanns <eo@nebensachen.de>]

Bob Copeland <me@bobcopeland.com> 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