Re: [PATCH net-next 1/9] vlan: adopt u64_stats_t

[Eric Dumazet <edumazet@google.com>]

On Wed, Jun 8, 2022 at 3:18 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Eric Dumazet
> > Sent: 08 June 2022 00:36
> >
> > As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
> > we should use u64_stats_t and related accessors to avoid load/store tearing.
> >
> > Add READ_ONCE() when reading rx_errors & tx_dropped.
>
> Isn't this all getting entirely stupid?

Be careful about using these kinds of words, some people get offended
quite easily.

>
> AFAICT nearly every 'memory' access in the kernel is going
> to get wrapped in READ/WRITE_ONCE() to avoid something
> that really never actually happens?
>

When needed, yes. KCSAN can catch real bugs, thank you.

> It might be better to just mark everything 'volatile'.
> Although perhaps that ought to be a compiler option.
>
> OTOH I've seen gcc generate extra instructions for 'volatile'
> accesses - to the point where I used 'barrier()' to optimise
> code.
> I think the volatile casts in READ_ONCE() can generate worse
> code than volatile variables.

For these patches, code generation on x86 is actually exactly the same.

 Read https://lwn.net/Articles/793253

Then if you really think Jade Alglave, Will Deacon, Boqun Feng, David Howells,
 Daniel Lustig, Luc Maranget, Paul E. McKenney, Andrea Parri, Nicholas Piggin,
Alan Stern, Akira Yokosawa, and Peter Zijlstra were stupid, please
start a conversation
with them.

Thank you.