On 2022-08-18 10:45:05 [-0700], Jakub Kicinski wrote:

BTW, I have a hazy memory of people saying they switched to the _irq()
version because it reduced the number of retries significantly under
traffic. Dunno how practical that is but would you be willing to scan
the git history to double check that's not some of the motivation?

I didn't find any changes like -u64_stats_fetch_begin()
+u64_stats_fetch_begin_irq() for perf reasons. Thinking about it:

- On 32bit UP u64_stats_fetch_begin() disables only preemption during
  stats read up, so the number of retries must be 0 because no seqcount
  is used.

- On 32bit UP u64_stats_fetch_begin_irq() disables interrupt during stats
  read out so the number of retries must be 0 because no seqcount is
  used.

- On 32bit SMP u64_stats_fetch_begin()/ u64_stats_fetch_begin_irq()
  keep preemption/ interrupts as-is, uses seqcount and so retries might
  be observed.

Based on that I don't see how a switch to _irq() makes a change in an
SMP build while on UP you shouldn't see a retry at all.

Using u64_stats_fetch_begin() is most likely wrong in networking because
the stats are updated while packets are received which is either in IRQ
or in BH (except maybe for the SPI/I2C ethernet driver). The _bh()
version was replaced with _irq() for netpoll reasons.
u64_stats_fetch_begin() does not disable any of those two (IRQ, BH) on
32bit-UP and does not use a seqcount so it is possible that an update
happens during the read out of stats. Let me look…
	drivers/net/ethernet/cortina/gemini.c
reads in net_device_ops::ndo_get_stats64(), updates in NAPI.

	drivers/net/ethernet/huawei/hinic/hinic_rx.c
reads in net_device_ops::ndo_get_stats64() and
ethtool_ops::get_ethtool_stats(), updates in NAPI. Good.

So I guess these want be fixed properly instead of silently via the
series?

Sebastian