[net,0/2] net: u64_stats fixups for 32bit.

Message

Sebastian Andrzej Siewior Aug. 25, 2022, 11:36 a.m. UTC

Hi,

while looking at the u64-stats patch
	https://lore.kernel.org/all/[e-mail]

I noticed that u64_stats_fetch_begin() is used. That suspicious thing
about it is that network processing, including stats update, is
performed in NAPI and so I would expect to see
u64_stats_fetch_begin_irq() in order to avoid updates from NAPI during
the read. This is only needed on 32bit-UP where the seqcount is not
used. This is address in 2/2. The remaining user take some kind of
precaution and may use u64_stats_fetch_begin().

I updated the previously mentioned patch to get rid of
u64_stats_fetch_begin_irq(). If this is not considered stable patch
worthy then it can be ignored and considred fixed by the other series
which removes the special 32bit cases.

The xrs700x driver reads and writes the counter from preemptible context
so the only missing piece here is at least disable preemption on the
writer side to avoid preemption while the writer is in progress. The
possible reader would spin then until the writer completes its write
critical section which is considered bad. This is addressed in 1/2 by
using u64_stats_update_begin_irqsave() and so disable interrupts during
the write critical section.
The other closet resemblance I found is mdio_bus.c::mdiobus_stats_acct()
where preemtion is disabled unconditionally. This is something I want to
avoid since it also affects 64bit.

Sebastian

Comments

[e-mail] Aug. 29, 2022, 12:10 p.m. UTC | #1

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <[e-mail]>:

On Thu, 25 Aug 2022 13:36:43 +0200 you wrote:
> Hi,
> 
> while looking at the u64-stats patch
> 	https://lore.kernel.org/all/[e-mail]
> 
> I noticed that u64_stats_fetch_begin() is used. That suspicious thing
> about it is that network processing, including stats update, is
> performed in NAPI and so I would expect to see
> u64_stats_fetch_begin_irq() in order to avoid updates from NAPI during
> the read. This is only needed on 32bit-UP where the seqcount is not
> used. This is address in 2/2. The remaining user take some kind of
> precaution and may use u64_stats_fetch_begin().
> 
> [...]

Here is the summary with links:
  - [net,1/2] net: dsa: xrs700x: Use irqsave variant for u64 stats update
    https://git.kernel.org/netdev/net/c/3f8ae9fe0409
  - [net,2/2] net: Use u64_stats_fetch_begin_irq() for stats fetch.
    https://git.kernel.org/netdev/net/c/278d3ba61563

You are awesome, thank you!