Message ID | 20240105181024.14418-1-petr@tesarici.cz |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-18197-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp6388332dyb; Fri, 5 Jan 2024 10:19:13 -0800 (PST) X-Google-Smtp-Source: AGHT+IHzUmMoGPY7hrXnQ8v360sFIFxbDVh8/Wp/f7Dpw8GpVH5JKsVB9enUv+RVpi/Yn/EZllpQ X-Received: by 2002:a05:6512:3609:b0:50e:2f46:1111 with SMTP id f9-20020a056512360900b0050e2f461111mr1031721lfs.97.1704478753079; Fri, 05 Jan 2024 10:19:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704478753; cv=none; d=google.com; s=arc-20160816; b=ea9Gr/IvaHTVN4T1mVHXSC0kkgEKY8kns95b5bMMLrmVYO9MFuZ/ZvklWwdRyK/kzm 6HJ9WXeo1AORKZTLCENe+7cCGrUNeQE+WBX+Rd25rhN+4G9AUapRfuhJXt36ptD8dRLW xix3Rv9YAOrac7fbkaqOp0lBl4DKaOsUqzj9mAHLkuQ1E/hZAYbYPNFUVeI+Jgoa369I 8U0ohmBmqv0ns0/liXq5Pnwpb8FPk/nKknsSfo3xy3rliHr+eQ9cRuc779WTQ2bGDUmY FgwOyyvyY7ukiaOjoQcJfvT0Plpf0IkM638XJxQpFXyMPrH6VziqvlF6v7+MPJIVETqF YH2g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=PifGxt+n9pxGLUM3qvwJzRvG4T/fiAlGgkUXdC6BB0o=; fh=M1ZrftjdwtdfnkwCMwcVwK1+j/H5v/KGLxYuPxMd8JU=; b=zY1hVLkYXOJWKPQzZ2G2c+a4hQBhwPfgm49wrNV2KNFTfeHLqPM/bvxQT/X/yxaXit 6GylQ0E8ISYf3ekr1panb+Yuz1gehGhZlDX0CEilt5H3kwxzPxjQfXJDaLI/dAtW7QFJ jfIMezfaM8ZY3zsOW3e4FlP5QSn99AZ7g2hBTsEYVva547F+kO+UCcPJDrwy//0KGVmu 6ETILenKb8YilIu76y6jb4lGCDH4MjLQt0XvCatdrcx2WVJImoqWQxe7e5epmDgEmMws IgAXPWrPdNk9p5bGv67dSo/dVPUJdMg3f9pCdsqpHYJbrR1G0PzCeIH7PQbeb3VkbvRr 8iuQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@tesarici.cz header.s=mail header.b=VYoSzag7; spf=pass (google.com: domain of linux-kernel+bounces-18197-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-18197-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=tesarici.cz Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id kb13-20020a170907924d00b00a26a961b85dsi807325ejb.58.2024.01.05.10.19.12 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jan 2024 10:19:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-18197-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@tesarici.cz header.s=mail header.b=VYoSzag7; spf=pass (google.com: domain of linux-kernel+bounces-18197-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-18197-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=tesarici.cz Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 978F91F238B8 for <ouuuleilei@gmail.com>; Fri, 5 Jan 2024 18:19:12 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 98BFD35EF1; Fri, 5 Jan 2024 18:18:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tesarici.cz header.i=@tesarici.cz header.b="VYoSzag7" X-Original-To: linux-kernel@vger.kernel.org Received: from bee.tesarici.cz (bee.tesarici.cz [77.93.223.253]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BF77835880; Fri, 5 Jan 2024 18:18:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=tesarici.cz Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=tesarici.cz Received: from meshulam.tesarici.cz (dynamic-2a00-1028-83b8-1e7a-4427-cc85-6706-c595.ipv6.o2.cz [IPv6:2a00:1028:83b8:1e7a:4427:cc85:6706:c595]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bee.tesarici.cz (Postfix) with ESMTPSA id 8FD9A1A7FB3; Fri, 5 Jan 2024 19:18:27 +0100 (CET) Authentication-Results: mail.tesarici.cz; dmarc=fail (p=none dis=none) header.from=tesarici.cz DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tesarici.cz; s=mail; t=1704478707; bh=V6jqxL1/0Rwi+XfoYiFSK5tUjtt5ttIj+0WycUW5aJE=; h=From:To:Cc:Subject:Date:From; b=VYoSzag7gI/sN1O/jNGFulge62i35RfaYf0k85MDkIsu7eevgsMX92G3xpfGLxf5W WBqDr91bVCNUtPKepn5vSPghxoiniLNarw4mH2/yACDxyx0ezS0D1/6p8mPcAUhuIB AuGpnkOOo04jW0Bq6xK5PJ7SVO1E819hXcyH9XWNDiXf0VcnW0egOJD8gXIJhni/3J IlCjemGuuvv/sAxmgUNG9P0v5hSNdV7oQu0RZiaGwwLVNklrHqJTeToUozgLlXOu/4 XBrdjURImORfuofX3jMpTLGQEejQ7lbxChRjuOWk8Nmtjuejk8pd0FHbzVVp+yAH3N H1RAkBm6CmQYw== From: Petr Tesarik <petr@tesarici.cz> To: Alexandre Torgue <alexandre.torgue@foss.st.com>, Jose Abreu <joabreu@synopsys.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Jisheng Zhang <jszhang@kernel.org>, netdev@vger.kernel.org (open list:STMMAC ETHERNET DRIVER), linux-stm32@st-md-mailman.stormreply.com (moderated list:ARM/STM32 ARCHITECTURE), linux-arm-kernel@lists.infradead.org (moderated list:ARM/STM32 ARCHITECTURE), linux-kernel@vger.kernel.org (open list) Cc: Petr Tesarik <petr@tesarici.cz> Subject: [PATCH] net: stmmac: fix ethtool per-queue statistics Date: Fri, 5 Jan 2024 19:10:24 +0100 Message-ID: <20240105181024.14418-1-petr@tesarici.cz> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1787275512892575513 X-GMAIL-MSGID: 1787275512892575513 |
Series |
net: stmmac: fix ethtool per-queue statistics
|
|
Commit Message
Petr Tesařík
Jan. 5, 2024, 6:10 p.m. UTC
Fix per-queue statistics for devices with more than one queue.
The output data pointer is currently reset in each loop iteration,
effectively summing all queue statistics in the first four u64 values.
The summary values are not even labeled correctly. For example, if eth0 has
2 queues, ethtool -S eth0 shows:
q0_tx_pkt_n: 374 (actually tx_pkt_n over all queues)
q0_tx_irq_n: 23 (actually tx_normal_irq_n over all queues)
q1_tx_pkt_n: 462 (actually rx_pkt_n over all queues)
q1_tx_irq_n: 446 (actually rx_normal_irq_n over all queues)
q0_rx_pkt_n: 0
q0_rx_irq_n: 0
q1_rx_pkt_n: 0
q1_rx_irq_n: 0
While touching this code, change the pointer type to u64 and get rid of the
weird pointer arithmetic.
Signed-off-by: Petr Tesarik <petr@tesarici.cz>
Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary")
---
.../ethernet/stmicro/stmmac/stmmac_ethtool.c | 23 ++++++-------------
1 file changed, 7 insertions(+), 16 deletions(-)
Comments
On Fri, 5 Jan 2024 19:10:24 +0100 Petr Tesarik <petr@tesarici.cz> wrote: > Fix per-queue statistics for devices with more than one queue. > > The output data pointer is currently reset in each loop iteration, > effectively summing all queue statistics in the first four u64 values. > > The summary values are not even labeled correctly. For example, if eth0 has > 2 queues, ethtool -S eth0 shows: > > q0_tx_pkt_n: 374 (actually tx_pkt_n over all queues) > q0_tx_irq_n: 23 (actually tx_normal_irq_n over all queues) > q1_tx_pkt_n: 462 (actually rx_pkt_n over all queues) > q1_tx_irq_n: 446 (actually rx_normal_irq_n over all queues) > q0_rx_pkt_n: 0 > q0_rx_irq_n: 0 > q1_rx_pkt_n: 0 > q1_rx_irq_n: 0 > > While touching this code, change the pointer type to u64 and get rid of the > weird pointer arithmetic. Just to make sure, this fix has nothing to do with my previous stmmac fix. It's just a fix for something I noticed while working on the other patch. I hope this fix can be merged fast, so I can base my other patch on it. Petr T > Signed-off-by: Petr Tesarik <petr@tesarici.cz> > Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary") > --- > .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 23 ++++++------------- > 1 file changed, 7 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > index f628411ae4ae..023876fc4da7 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > @@ -543,43 +543,34 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data) > u32 rx_cnt = priv->plat->rx_queues_to_use; > unsigned int start; > int q, stat; > - u64 *pos; > - char *p; > + u64 *p; > > - pos = data; > for (q = 0; q < tx_cnt; q++) { > struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[q]; > struct stmmac_txq_stats snapshot; > > - data = pos; > do { > start = u64_stats_fetch_begin(&txq_stats->syncp); > snapshot = *txq_stats; > } while (u64_stats_fetch_retry(&txq_stats->syncp, start)); > > - p = (char *)&snapshot + offsetof(struct stmmac_txq_stats, tx_pkt_n); > - for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) { > - *data++ += (*(u64 *)p); > - p += sizeof(u64); > - } > + p = &snapshot.tx_pkt_n; > + for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) > + *data++ = *p++; > } > > - pos = data; > for (q = 0; q < rx_cnt; q++) { > struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[q]; > struct stmmac_rxq_stats snapshot; > > - data = pos; > do { > start = u64_stats_fetch_begin(&rxq_stats->syncp); > snapshot = *rxq_stats; > } while (u64_stats_fetch_retry(&rxq_stats->syncp, start)); > > - p = (char *)&snapshot + offsetof(struct stmmac_rxq_stats, rx_pkt_n); > - for (stat = 0; stat < STMMAC_RXQ_STATS; stat++) { > - *data++ += (*(u64 *)p); > - p += sizeof(u64); > - } > + p = &snapshot.rx_pkt_n; > + for (stat = 0; stat < STMMAC_RXQ_STATS; stat++) > + *data++ = *p++; > } > } >
On Fri, Jan 05, 2024 at 07:10:24PM +0100, Petr Tesarik wrote: > Fix per-queue statistics for devices with more than one queue. > > The output data pointer is currently reset in each loop iteration, > effectively summing all queue statistics in the first four u64 values. > > The summary values are not even labeled correctly. For example, if eth0 has > 2 queues, ethtool -S eth0 shows: > > q0_tx_pkt_n: 374 (actually tx_pkt_n over all queues) > q0_tx_irq_n: 23 (actually tx_normal_irq_n over all queues) > q1_tx_pkt_n: 462 (actually rx_pkt_n over all queues) > q1_tx_irq_n: 446 (actually rx_normal_irq_n over all queues) > q0_rx_pkt_n: 0 > q0_rx_irq_n: 0 > q1_rx_pkt_n: 0 > q1_rx_irq_n: 0 > > While touching this code, change the pointer type to u64 and get rid of the > weird pointer arithmetic. > > Signed-off-by: Petr Tesarik <petr@tesarici.cz> > Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary") Hi Petr There are a few process things you are missing. Please take a look at https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html You need to indicate which tree this is for. Additionally, your Signed-off-by comes last. Patches for stable should ideally be minimal. And obviously correct. See: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html So the bits about changing the pointer type and removing the weird arithmetic might be better suited for net-next, not net. Andrew
On Fri, 5 Jan 2024 20:54:01 +0100 Andrew Lunn <andrew@lunn.ch> wrote: > On Fri, Jan 05, 2024 at 07:10:24PM +0100, Petr Tesarik wrote: > > Fix per-queue statistics for devices with more than one queue. > > > > The output data pointer is currently reset in each loop iteration, > > effectively summing all queue statistics in the first four u64 values. > > > > The summary values are not even labeled correctly. For example, if eth0 has > > 2 queues, ethtool -S eth0 shows: > > > > q0_tx_pkt_n: 374 (actually tx_pkt_n over all queues) > > q0_tx_irq_n: 23 (actually tx_normal_irq_n over all queues) > > q1_tx_pkt_n: 462 (actually rx_pkt_n over all queues) > > q1_tx_irq_n: 446 (actually rx_normal_irq_n over all queues) > > q0_rx_pkt_n: 0 > > q0_rx_irq_n: 0 > > q1_rx_pkt_n: 0 > > q1_rx_irq_n: 0 > > > > While touching this code, change the pointer type to u64 and get rid of the > > weird pointer arithmetic. > > > > Signed-off-by: Petr Tesarik <petr@tesarici.cz> > > Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary") > > Hi Petr > > There are a few process things you are missing. Please take a look at > > https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html > > You need to indicate which tree this is for. Ah. Thank you for the pointer. You see, this is my first netdev patch... > Additionally, your Signed-off-by comes last. Oh, I'm sorry for that. > Patches for stable should ideally be minimal. And obviously correct. See: > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > > So the bits about changing the pointer type and removing the weird > arithmetic might be better suited for net-next, not net. Good. Given that I have meanwhile found out that I will have to change this whole function to fix the lockup on 32-bit systems, let me just discard this part, add "net" to the subject and cc stable. Resending... Petr T
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index f628411ae4ae..023876fc4da7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -543,43 +543,34 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data) u32 rx_cnt = priv->plat->rx_queues_to_use; unsigned int start; int q, stat; - u64 *pos; - char *p; + u64 *p; - pos = data; for (q = 0; q < tx_cnt; q++) { struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[q]; struct stmmac_txq_stats snapshot; - data = pos; do { start = u64_stats_fetch_begin(&txq_stats->syncp); snapshot = *txq_stats; } while (u64_stats_fetch_retry(&txq_stats->syncp, start)); - p = (char *)&snapshot + offsetof(struct stmmac_txq_stats, tx_pkt_n); - for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) { - *data++ += (*(u64 *)p); - p += sizeof(u64); - } + p = &snapshot.tx_pkt_n; + for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) + *data++ = *p++; } - pos = data; for (q = 0; q < rx_cnt; q++) { struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[q]; struct stmmac_rxq_stats snapshot; - data = pos; do { start = u64_stats_fetch_begin(&rxq_stats->syncp); snapshot = *rxq_stats; } while (u64_stats_fetch_retry(&rxq_stats->syncp, start)); - p = (char *)&snapshot + offsetof(struct stmmac_rxq_stats, rx_pkt_n); - for (stat = 0; stat < STMMAC_RXQ_STATS; stat++) { - *data++ += (*(u64 *)p); - p += sizeof(u64); - } + p = &snapshot.rx_pkt_n; + for (stat = 0; stat < STMMAC_RXQ_STATS; stat++) + *data++ = *p++; } }