Ben Greear <gree...@candelatech.com> writes: > On 09/24/2014 12:44 AM, Michal Kazior wrote: >> On 23 September 2014 23:17, <gree...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html