Re: [PATCH net-next 1/4] eth: mpnic: count packets, bytes, and drops per ring

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] locking: u64_stats_sync deadlock in mpnic_clean_twq0() due to softirq reader
--

commit b438863957f763895e3efd300c4e2cd81b9bb11b
Author: Daniel Zahka <daniel.zahka@gmail.com>

eth: mpnic: count packets, bytes, and drops per ring

Keep packet, byte and drop counters on each Tx work, Tx completion, and
Rx completion queue. Report them to the core via the rtnl_link_stats64
interface using an RCU approach.

> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
[ ... ]
> @@ -288,8 +295,19 @@ static void mpnic_clean_twq0(struct mpnic_napi_vector *nv, int napi_budget,
>  
>  	ring->head = head;
>  
> -	if (discard)
> +	if (discard) {
> +		preempt_disable();
> +		u64_stats_update_begin(&ring->stats.syncp);
> +		u64_stats_add(&ring->stats.twq.dropped, total_packets);
> +		u64_stats_update_end(&ring->stats.syncp);
> +		preempt_enable();
>  		return;
> +	}

[Severity: High]
Can this cause a deadlock on 32-bit systems if a softirq preempts the writer?

Writers to u64_stats_sync must be serialized against readers. On 32-bit
systems, readers spin in a retry loop while the sequence counter is odd
(mid-update). The preempt_disable() call disables process preemption but
leaves softirqs enabled. 

If a reader like dev_get_stats is invoked from network receive or BPF softirq
contexts on the same CPU, it might interrupt this writer while the sequence
counter is odd. The reader would then spin infinitely waiting for the count
to become even, resulting in a deadlock.

Should this use local_bh_disable() to protect against BH-context readers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-mpnic-counters3-v1-0-7dc7644cc500@gmail.com?part=1