On 27 January 2016 at 15:26, Johannes Berg [off-list ref] wrote:

On Wed, 2016-01-27 at 15:26 +0100, Michal Kazior wrote:

quoted

@@ -1294,6 +1298,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct

ieee80211_hw *hw,
      if (!skb)
              goto out;

+     txqi->byte_cnt -= skb->len;
+
      atomic_dec(&sdata->txqs_len[ac]);

This *looks* a bit worrying - you have an atomic dec for the # of
packets and a non-atomic one for the bytes.

You probably thought about it and I guess it's fine, but can you
explain it?

The atomic was used because it accesses per-vif counters. You can't
use txqi->queue.lock for that.

On the other hand byte_cnt is per txqi and it was very easy to make
use of the txqi->queue.lock (which was already acquired in both drv_tx
and tx_dequeue functions). Using atomic* for byte_cnt would be
wasteful a bit.


Michał