Messages in this thread | | | Date | Thu, 22 Jun 2023 14:03:20 +0200 | From | Simon Horman <> | Subject | Re: [PATCH net-next v3 2/2] net: stmmac: use pcpu 64 bit statistics where necessary |
| |
On Tue, Jun 20, 2023 at 12:52:20AM +0800, Jisheng Zhang wrote:
...
Hi Jisheng Zhang,
some minor feedback from my side, as it seems there will be a v4 anyway.
> @@ -535,23 +548,37 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data) > { > u32 tx_cnt = priv->plat->tx_queues_to_use; > u32 rx_cnt = priv->plat->rx_queues_to_use; > - int q, stat; > + unsigned int start; > + int q, stat, cpu; > char *p; > - > - for (q = 0; q < tx_cnt; q++) { > - p = (char *)priv + offsetof(struct stmmac_priv, > - xstats.txq_stats[q].tx_pkt_n); > - for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) { > - *data++ = (*(unsigned long *)p); > - p += sizeof(unsigned long); > + u64 *pos;
Please use reverse xmas tree - longest line to shortest - for local variable declarations in new Networking code.
...
> @@ -563,7 +590,8 @@ static void stmmac_get_ethtool_stats(struct net_device *dev, > u32 rx_queues_count = priv->plat->rx_queues_to_use; > u32 tx_queues_count = priv->plat->tx_queues_to_use; > unsigned long count; > - int i, j = 0, ret; > + unsigned int start; > + int i, j = 0, pos, ret, cpu;
Ditto.
...
> @@ -606,6 +633,22 @@ static void stmmac_get_ethtool_stats(struct net_device *dev, > data[j++] = (stmmac_gstrings_stats[i].sizeof_stat == > sizeof(u64)) ? (*(u64 *)p) : (*(u32 *)p); > } > + pos = j; > + for_each_possible_cpu(cpu) { > + struct stmmac_pcpu_stats *stats, snapshot; > + > + stats = per_cpu_ptr(priv->xstats.pstats, cpu); > + j = pos; > + do { > + start = u64_stats_fetch_begin(&stats->syncp); > + snapshot = *stats; > + } while (u64_stats_fetch_retry(&stats->syncp, start)); > + > + for (i = 0; i < STMMAC_PCPU_STATS_LEN; i++) { > + char *p = (char *)&snapshot + stmmac_gstrings_pcpu_stats[i].stat_offset;
Blank line here please.
> + data[j++] += *(u64 *)p; > + } > + } > stmmac_get_per_qstats(priv, &data[j]); > } >
...
> @@ -695,6 +738,11 @@ static void stmmac_get_strings(struct net_device *dev, u32 stringset, u8 *data) > ETH_GSTRING_LEN); > p += ETH_GSTRING_LEN; > } > + for (i = 0; i < STMMAC_PCPU_STATS_LEN; i++) { > + memcpy(p, stmmac_gstrings_pcpu_stats[i].stat_string, > + ETH_GSTRING_LEN);
The indentation of the line above isn't quite right: it should align with the inside of the opening parentheses on the previous line.
memcpy(p, stmmac_gstrings_pcpu_stats[i].stat_string, ETH_GSTRING_LEN);
> + p += ETH_GSTRING_LEN; > + } > stmmac_get_qstats_string(priv, p); > break; > case ETH_SS_TEST:
...
> @@ -5015,8 +5042,10 @@ static struct stmmac_xdp_buff *xsk_buff_to_stmmac_ctx(struct xdp_buff *xdp) > > static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue) > { > + struct stmmac_pcpu_stats *stats = this_cpu_ptr(priv->xstats.pstats); > struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue]; > unsigned int count = 0, error = 0, len = 0; > + u32 rx_errors = 0, rx_dropped = 0; > int dirty = stmmac_rx_dirty(priv, queue); > unsigned int next_entry = rx_q->cur_rx; > unsigned int desc_size;
Reverse xmas tree here too.
...
| |