At 2026-09-09 17:27:18, [email protected] wrote: >Thank you for your contribution! Sashiko AI review found 1 potential issue(s) >to consider: >- [High] Concurrent writers to per-queue u64_stats_sync from timer context and >NAPI violate mutual exclusion. >-- > >commit 2e5fa229ff4c8c773848d0c1f36b14c2d2f2edf7 >Author: Longjun Tang <[email protected]> > >virtio_net: add rx_timeouts per-queue statistic > >This commit adds an rx_timeouts field to the per-queue RX statistics. >It increments the counter when the RX watchdog fires and exposes it >via ethtool to track per-queue stall events. > >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> --- a/drivers/net/virtio_net.c >> +++ b/drivers/net/virtio_net.c > >[ ... ] > >> @@ -3094,6 +3097,10 @@ static void virtnet_rx_watchdog(struct timer_list *t) >> unsigned int stall_ms = >> jiffies_to_msecs(jiffies - >> rq->watchdog_jiffies); >> >> + u64_stats_update_begin(&rq->stats.syncp); >> + u64_stats_inc(&rq->stats.rx_timeouts); >> + u64_stats_update_end(&rq->stats.syncp); > >[Severity: High] >Does this introduce a race condition with the NAPI poll loop? > >The u64_stats_sync API requires mutually exclusive writers per syncp. >This timer callback (virtnet_rx_watchdog) executes in a global timer softirq >context on an arbitrary CPU and writes to rq->stats.syncp. > >Concurrently, NAPI polling executes in the receive queue's bound softirq >context on another CPU and also writes to rq->stats.syncp. Since >rx_timeouts was added to virtnet_rq_stats_desc, NAPI dynamically iterates >over it during virtnet_receive(): > >drivers/net/virtio_net.c:virtnet_receive() { > ... > u64_stats_add(item, u64_stats_read(src)); > ... >} > >Concurrent writers corrupt the sequence counter on 32-bit systems, causing >readers (such as ndo_get_stats64 via ethtool) to spin in an infinite >retry loop.
This is a real bug. u64_stats_sync requires mutually exclusive writers, but the watchdog timer runs on an arbitrary CPU while the queue's NAPI aggregates rq->stats under the same syncp. I will fix this in the next revision by moving the rx_timeouts accounting write into the NAPI poll; the watchdog will only detect the stall and call napi_schedule(). I would appreciate any feedback, especially on whether this direction is worth pursuing. Thanks. Longjun > >-- >Sashiko AI review ยท >https://sashiko.dev/#/patchset/[email protected]?part=3
