Re: [PATCH] mac80211: provide race-free 64-bit traffic counters

[Ben Greear <greearb@candelatech.com>]

On 03/04/2013 09:02 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Make the TX bytes/packets counters race-free by keeping
> them per AC so concurrent TX on queues can't cause lost
> or wrong updates. This works since each station belongs
> to a single interface. While at it also make the bytes
> counters 64-bit.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>   net/mac80211/cfg.c      | 22 ++++++++++++++--------
>   net/mac80211/sta_info.h |  9 +++++----
>   net/mac80211/tx.c       |  5 +++--
>   3 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 8259a5b..1ff629a 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -445,12 +445,13 @@ static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
>   	struct ieee80211_sub_if_data *sdata = sta->sdata;
>   	struct ieee80211_local *local = sdata->local;
>   	struct timespec uptime;
> +	int ac;
>
>   	sinfo->generation = sdata->local->sta_generation;
>
>   	sinfo->filled = STATION_INFO_INACTIVE_TIME |
> -			STATION_INFO_RX_BYTES |
> -			STATION_INFO_TX_BYTES |
> +			STATION_INFO_RX_BYTES64 |
> +			STATION_INFO_TX_BYTES64 |
>   			STATION_INFO_RX_PACKETS |
>   			STATION_INFO_TX_PACKETS |
>   			STATION_INFO_TX_RETRIES |
> @@ -467,10 +468,14 @@ static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
>   	sinfo->connected_time = uptime.tv_sec - sta->last_connected;
>
>   	sinfo->inactive_time = jiffies_to_msecs(jiffies - sta->last_rx);
> +	sinfo->tx_bytes = 0;
> +	sinfo->tx_packets = 0;
> +	for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
> +		sinfo->tx_bytes += sta->tx_bytes[ac];
> +		sinfo->tx_packets += sta->tx_packets[ac];
> +	}

I think you'll need tx_packets to be u64 as well, as otherwise
if a queue wraps it's going to be quite hard to figure out?

Thanks,
Ben

>   	/* Updated from TX path only, no locking requirements */
> -	unsigned long tx_packets;
> -	unsigned long tx_bytes;
> -	unsigned long tx_fragments;
> +	u32 tx_packets[IEEE80211_NUM_ACS];
> +	u32 tx_fragments;
> +	u64 tx_bytes[IEEE80211_NUM_ACS];
>   	struct ieee80211_tx_rate last_tx_rate;
>   	int last_rx_rate_idx;
>   	u32 last_rx_rate_flag;
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index c79860f..6af7f60 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -991,14 +991,15 @@ static ieee80211_tx_result debug_noinline
>   ieee80211_tx_h_stats(struct ieee80211_tx_data *tx)
>   {
>   	struct sk_buff *skb;
> +	int ac = skb_get_queue_mapping(tx->skb);
>
>   	if (!tx->sta)
>   		return TX_CONTINUE;
>
> -	tx->sta->tx_packets++;
> +	tx->sta->tx_packets[ac]++;
>   	skb_queue_walk(&tx->skbs, skb) {
>   		tx->sta->tx_fragments++;
> -		tx->sta->tx_bytes += skb->len;
> +		tx->sta->tx_bytes[ac] += skb->len;
>   	}
>
>   	return TX_CONTINUE;
>


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