Re:Re: [RFC RESEND 3/3] virtio_net: add rx_timeouts per-queue statistic

["Lange Tang" <lange_tang@163.com>]

At 2026-09-09 17:27:18, sashiko-bot@kernel.org 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 <tanglongjun@kylinos.cn>
>
>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/20260908092646.108865-1-lange_tang@163.com?part=3