[PATCH v2 03/10] ath10k: support ethtool stats.

Kalle Valo kvalo at qca.qualcomm.com
Mon Sep 29 01:21:10 PDT 2014


Ben Greear <greearb at candelatech.com> writes:

> On 09/24/2014 12:44 AM, Michal Kazior wrote:
>> On 23 September 2014 23:17,  <greearb at candelatech.com> wrote:
>> [...]
>>> +void ath10k_get_et_stats(struct ieee80211_hw *hw,
>>> +                        struct ieee80211_vif *vif,
>>> +                        struct ethtool_stats *stats, u64 *data)
>>> +{
>>> +       struct ath10k *ar = hw->priv;
>>> +       int i = 0;
>>> +       struct ath10k_target_stats *fw_stats;
>>> +
>>> +       fw_stats = &ar->debug.target_stats;
>>> +
>>> +       mutex_lock(&ar->conf_mutex);
>>> +
>>> +       if (ar->state == ATH10K_STATE_ON)
>>> +               ath10k_refresh_peer_stats(ar);
>>> +
>>> +       mutex_unlock(&ar->conf_mutex);
>>
>> Just for correctness sake - it's probably a good idea to
>> mutex_unlock() at the end (i.e. after spin_unlock_bh()) to make sure
>> the stats are for this particular request. With your patch there's a
>> very slight chance that, e.g. fw_stats debug file is being read at the
>> same time and it updates fw stats between the above mutex_unlock() and
>> following spin_lock_bh().
>
> That makes no difference at all to the user though, and it is one less
> set of nested locks to worry about.

I still do not want to have known races, especially when it's so easy to
fix. The ethtool patches patches conflict with Michal's fw_stats fixes.
Let me take the ethtool patches so that I can rebase them, fix this race
and do some other small changes as well. I'll send v2 soon.

-- 
Kalle Valo



More information about the ath10k mailing list