Re: [PATCH net] net: gen_estimator: protect seqcount updates from hardirq readers
From: sashiko-bot
Date: Wed Sep 23 2026 - 23:30:00 EST
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@xxxxxxxxxx>
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@xxxxxxxxxx?part=1