Re: [PATCH net] net: gen_estimator: protect seqcount updates from hardirq readers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: gen_estimator: unnecessary irq disable for non-existent hardirq reader
--

commit 9722f5db0aef7c6cb33fd991daa5c91e6188c2bb
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>

net: gen_estimator: protect seqcount updates from hardirq readers

This commit modifies est_timer() to disable local interrupts instead of
just disabling preemption when updating the estimator sequence counter.
The commit message notes this is to prevent a spinning hardirq reader,
reproducing a lockdep warning via a test-only hardirq injection.

> diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
> index c34e58c6c3e66..6aba5916d7959 100644
> --- a/net/core/gen_estimator.c
> +++ b/net/core/gen_estimator.c

[ ... ]

> @@ -90,12 +91,12 @@ static void est_timer(struct timer_list *t)
>  	rate = (b_packets - est->last_packets) << (10 - est->intvl_log);
>  	rate = (rate >> est->ewma_log) - (est->avpps >> est->ewma_log);
>  
> -	preempt_disable_nested();
> +	local_irq_save(flags);

[Severity: Medium]
Does this unnecessarily increase hardirq latency and CPU overhead by replacing
a zero-overhead preemption disable (on non-RT) with an expensive hardware
interrupt disable?

The commit message explicitly states this was triggered by a test-only hardirq
injection. In a production environment, gen_estimator_read() is only called
from softirq or process contexts (such as netfilter hooks, TC actions, or
stats dumping), and never from hardirq context.

Since est_timer() executes unconditionally, this change appears to add
overhead to the fast path to protect against a state that is structurally
impossible in actual usage.

>  	write_seqcount_begin(&est->seq);
>  	est->avbps += brate;
>  	est->avpps += rate;
>  	write_seqcount_end(&est->seq);
> -	preempt_enable_nested();
> +	local_irq_restore(flags);
>  
>  	est->last_bytes = b_bytes;
>  	est->last_packets = b_packets;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923032936.2020902-1-runyu.xiao@seu.edu.cn?part=1