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.

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=3

Reply via email to