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

[Ben Greear <greearb@candelatech.com>]

On 09/24/2014 12:44 AM, Michal Kazior wrote:
> On 23 September 2014 23:17,  <greearb@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'd prefer to leave it as is.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com