linux-kernel - Re: [PATCH net-next] net: airoha: Use u64_stats_t with u64_stats

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAXyoMOtMz9Hn=gqS1Wj0ASTZ1JaReZnc685h7YN2sD9zjrtcA@mail.gmail.com>
Date: Mon, 26 Jan 2026 06:56:12 +0800
From: Yangfl <mmyangfl@...il.com>
To: David Laight <david.laight.linux@...il.com>
Cc: netdev@...r.kernel.org, Lorenzo Bianconi <lorenzo@...nel.org>, 
	Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net: airoha: Use u64_stats_t with u64_stats_sync properly

On Mon, Jan 26, 2026 at 6:35 AM David Laight
<david.laight.linux@...il.com> wrote:
>
> On Fri, 23 Jan 2026 02:52:51 +0800
> David Yang <mmyangfl@...il.com> wrote:
>
> > On 64bit arches, struct u64_stats_sync is empty and provides no help
> > against load/store tearing. Convert to u64_stats_t to ensure atomic
> > operations.
> >
> > Signed-off-by: David Yang <mmyangfl@...il.com>
> > ---
> >  drivers/net/ethernet/airoha/airoha_eth.c | 136 +++++++++++------------
> >  drivers/net/ethernet/airoha/airoha_eth.h |  34 +++---
> >  2 files changed, 85 insertions(+), 85 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index 62bcbbbe2a95..6ed220e5a094 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > @@ -1472,131 +1472,131 @@ static void airoha_update_hw_stats(struct airoha_gdm_port *port)
> >
> >       /* TX */
> >       val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
> > -     port->stats.tx_ok_pkts += ((u64)val << 32);
> > +     u64_stats_add(&port->stats.tx_ok_pkts, (u64)val << 32);
> >       val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
> > -     port->stats.tx_ok_pkts += val;
> > +     u64_stats_add(&port->stats.tx_ok_pkts, val);
>
> Wouldn't that be better written as:
>         u64 val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
>         val = val << 32 + airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
>         port->stats.tx_ok_pkts += val;
> (Assuming there something has stopped the hardware increment the register
> between the two accesses, and there is an associated atomic zero.)
>
> Otherwise you are generating 'tearing' on 64bit systems by adding the high
> and low halves separately - regardless of how the stats are read.
> I think that works for 32bit as well.
>
> Making the code completely unreadable with 'special' types and 'special'
> copy routines really doesn't seem worth while.
> The compiler won't generate code that does 'data tearing' for aligned word
> accesses, and even if it did nothing would really break.

Quoting 316580b69d0a ("u64_stats: provide u64_stats_t type"):

  On 64bit arches, struct u64_stats_sync is empty and provides no help
against load/store tearing.

  Using READ_ONCE()/WRITE_ONCE() would be needed.

> The most you want is a memcpy_in_words() function that guarantees to use
> 'word' size tranfsers for aligned buffers - and is probably an alias for
> normal memcpy() on all current systems.

See https://lore.kernel.org/r/20260120092137.2161162-1-mmyangfl@gmail.com

And if you want a memcpy, you need a buffer. If you want to fold the
buffer, that's basically WRITE_ONCE().

>
> OTOH data tearing can be seen for 64 bit adds on 32bit systems.
>
> It is worth nothing that on 32bits the 'packet count' and 'byte count'
> increments get seen as a pair, this doesn't happen on 64bit - one happens
> before the other.
>
>         David