[PATCH 4/5] ath10k: protect wep tx key setup

[Michal Kazior]

On 14 May 2014 21:50, Kalle Valo <kvalo at qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior at tieto.com> writes:
>
>> All configuration sequences should be protected
>> with conf_mutex to avoid concurrent/conflicting
>> requests.
>>
>> This should make sure that wep tx key setup is not
>> performed while hw is restarted (at least).
>
> Locks and mutexes should protect data, not thread of execution. What are
> we exactly protecting here, arvif->def_wep_key_idx?

Actually conf_mutex idea is to protect and serialize configuration
sequences (WMI) too, not just data. Perhaps we should revise this?


> The patch makes sense, I'm just trying to understand the idea behind the
> implementation.
>
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -1888,8 +1888,10 @@ static void ath10k_tx_wep_key_work(struct work_struct *work)
>>                                               wep_key_work);
>>       int ret, keyidx = arvif->def_wep_key_newidx;
>>
>> +     mutex_lock(&arvif->ar->conf_mutex);
>> +
>>       if (arvif->def_wep_key_idx == keyidx)
>> -             return;
>> +             goto unlock;
>
> BTW, shouldn't we also check the state and bail out if the state is not
> ON (and possibly RESTARTED)? How can this work otherwise?

Very good point!

But actually.. this should be cancelled during recovery in the first
place. I need to take a look at it.


Michał