[PATCH net v4] net: do not let defer_count outlive an empty defer_list

[PATCH net v4] net: do not let defer_count outlive an empty defer_list

[Joseph Kain]

__skb_defer_free_flush() can leave defer_count at or above skb_defer_max
with an empty defer_list. That state is absorbing: every producer then
takes the nodefer path without queueing, so the list never refills.
Deferred freeing stays off for that (cpu, node) until reboot, and
cross-CPU frees fall back to the page allocator, which is the contention
deferral exists to avoid.

skb_attempt_defer_free() increments before testing the limit and does not
undo the increment when it rejects, so a rejected free leaves the counter
raised with nothing queued.

Nothing brings it back down. The reject path returns before
kick_defer_list_purge(), so once the counter is stuck no producer
schedules a flush on the owner CPU. A CPU that never raises NET_RX itself
-- an application CPU doing TX while NIC IRQs are pinned elsewhere and
RPS is off -- then never runs the flush at all, so clearing the counter
inside the flush cannot help it either. proc_do_skb_defer_max() neither
resets the counters nor schedules a flush when the limit changes.

It is reachable with no race at all:

  sysctl -w net.core.skb_defer_max=1    # frees increment, then reject
  sysctl -w net.core.skb_defer_max=128  # does not recover

skb_defer_max has .extra1 = SYSCTL_ZERO and no .extra2, so 1 is settable.
From v7.1 on, 0 is safe because proc_do_skb_defer_max() flips
skb_defer_disable_key and the producer returns before incrementing. On
6.18 through 7.0, which carry the llist conversion but not commit
08dc30de1a40 ("net: add skb_defer_disable_key static key"), setting 0 and
restoring the previous value strands the counter the same way -- and 0 is
the documented way to turn the feature off.

Test the limit before incrementing, so the counter is only ever raised by
a producer that goes on to queue. It then cannot outlive the skbs it
counts, and the absorbing state is unreachable without depending on a
flush ever running on the owner CPU.

This makes the limit advisory. The test no longer reserves the slot it
checks, and the recvmsg callers run in preemptible context, so the
overshoot is bounded by the number of producers in flight rather than by
skb_defer_max; the post-increment test reserved the slot and bounded the
queue exactly. The counter is already approximate -- the flush zeroes it
without regard for producers in flight -- and the next flush erases the
overshoot, where the latch it replaces never recovered at all.

Undoing the increment on the reject path instead would be wrong: a flush
that zeroes the counter between a producer's increment and its undo
leaves the counter negative, and the producer's local is an unsigned
long, so every later test reads it as enormous and rejects. That is the
same dead end, reached more easily.

While here, drain before clearing in the flush. A producer that queues
between the clear and the drain currently has its skb drained but its
increment kept, which over-counts and costs deferral until the next
flush. Draining first matches the ordering __dev_xmit_skb() already uses
for the qdisc defer_list.

Found by manual code inspection, LLM-assisted, while backporting the
llist conversion to 6.1 after hitting the sd->defer_lock contention it
removes. The latch itself was not hit in production; it was reproduced by
lowering net.core.skb_defer_max.

Fixes: 844c9db7f7f5 ("net: use llist for sd->defer_list")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Joseph Kain <jkain@nuro.ai>
---
v4:
 - v3's "undo the increment on reject" was wrong: a flush that zeroes the
   counter between a producer's increment and its undo leaves the counter
   negative, and the producer's local is an unsigned long, so every later
   test reads it as enormous and rejects forever. Test the limit before
   incrementing instead, so there is no decrement to underflow
   (netdev-bot sashiko).

Note this changes the effective limit from skb_defer_max - 1 to
skb_defer_max, because the test now runs before the increment rather than
after it. That matches what Documentation/admin-guide/sysctl/net.rst
describes. It also means the half-capacity kick can now fire at
skb_defer_max = 1, which commit c09b0cd2cc6c ("net: avoid strange
behavior with skb_defer_max == 1") set out to make possible.

Built for x86_64 and arm64, with and without CONFIG_PREEMPT_RT. On
v7.3-rc6 with skb_defer_max=1 and a cross-CPU TCP load, defer_count
reached 1918118 unpatched and did not recover when the default was
restored; with this patch it stays within the limit.

v3: https://lore.kernel.org/all/20261013205416.3417296-1-jkain@nuro.ai/
v2: https://lore.kernel.org/all/20261008220256.1844072-1-jkain@nuro.ai/
v1: https://lore.kernel.org/all/20261007180056.1394868-1-jkain@nuro.ai/
 net/core/dev.c    | 5 ++++-
 net/core/skbuff.c | 9 ++++++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 18dc88990510..60d86bc21632 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6940,8 +6940,11 @@ static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
 
 	if (llist_empty(&sdn->defer_list))
 		return;
-	atomic_long_set(&sdn->defer_count, 0);
 	free_list = llist_del_all(&sdn->defer_list);
+	/* Racy on purpose, as in __dev_xmit_skb(): a producer landing between
+	 * the del_all and this store stays queued but uncounted.
+	 */
+	atomic_long_set(&sdn->defer_count, 0);
 
 	llist_for_each_entry_safe(skb, next, free_list, ll_node) {
 		prefetch(next);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4aea06d5167d..25f82b7dcf01 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -7386,11 +7386,14 @@ nodefer:	kfree_skb_napi_cache(skb);
 	sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + cpu_to_node(my_cpu);
 
 	defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
-	defer_count = atomic_long_inc_return(&sdn->defer_count);
-
-	if (defer_count >= defer_max)
+	/* Test before incrementing: the reject path returns below without
+	 * reaching kick_defer_list_purge(), so a kept increment never flushes.
+	 */
+	if (atomic_long_read(&sdn->defer_count) >= defer_max)
 		goto nodefer;
 
+	defer_count = atomic_long_inc_return(&sdn->defer_count);
+
 	llist_add(&skb->ll_node, &sdn->defer_list);
 
 	if (unlikely(!cpu_online(cpu) || my_cpu != raw_smp_processor_id())) {
-- 
2.55.0

Re: [PATCH net v4] net: do not let defer_count outlive an empty defer_list

[kuniyu]

On Thu, 15 Oct 2026 09:22:09 -0700, Joseph Kain wrote:
> __skb_defer_free_flush() can leave defer_count at or above skb_defer_max
> with an empty defer_list. That state is absorbing: every producer then
> takes the nodefer path without queueing, so the list never refills.
> Deferred freeing stays off for that (cpu, node) until reboot, and
> cross-CPU frees fall back to the page allocator, which is the contention
> deferral exists to avoid.
> 
> skb_attempt_defer_free() increments before testing the limit and does not
> undo the increment when it rejects, so a rejected free leaves the counter
> raised with nothing queued.
> 
> Nothing brings it back down. The reject path returns before
> kick_defer_list_purge(), so once the counter is stuck no producer
> schedules a flush on the owner CPU. A CPU that never raises NET_RX itself
> -- an application CPU doing TX while NIC IRQs are pinned elsewhere and
> RPS is off -- then never runs the flush at all, so clearing the counter
> inside the flush cannot help it either. proc_do_skb_defer_max() neither
> resets the counters nor schedules a flush when the limit changes.
> 
> It is reachable with no race at all:
> 
>   sysctl -w net.core.skb_defer_max=1    # frees increment, then reject
>   sysctl -w net.core.skb_defer_max=128  # does not recover
> 
> skb_defer_max has .extra1 = SYSCTL_ZERO and no .extra2, so 1 is settable.
> From v7.1 on, 0 is safe because proc_do_skb_defer_max() flips
> skb_defer_disable_key and the producer returns before incrementing. On
> 6.18 through 7.0, which carry the llist conversion but not commit
> 08dc30de1a40 ("net: add skb_defer_disable_key static key"), setting 0 and
> restoring the previous value strands the counter the same way -- and 0 is
> the documented way to turn the feature off.
> 
> Test the limit before incrementing, so the counter is only ever raised by
> a producer that goes on to queue. It then cannot outlive the skbs it
> counts, and the absorbing state is unreachable without depending on a
> flush ever running on the owner CPU.

I think the state is still reachable when reducing the knob
if the new value >> 1 is smaller than the current defer_count
due to the kick condition (==).  This can be fixed by bumping
the knob to defer_count * 2 though.


> 
> This makes the limit advisory. The test no longer reserves the slot it
> checks, and the recvmsg callers run in preemptible context, so the
> overshoot is bounded by the number of producers in flight rather than by
> skb_defer_max; the post-increment test reserved the slot and bounded the
> queue exactly. The counter is already approximate -- the flush zeroes it
> without regard for producers in flight -- and the next flush erases the
> overshoot, where the latch it replaces never recovered at all.
> 
> Undoing the increment on the reject path instead would be wrong: a flush
> that zeroes the counter between a producer's increment and its undo
> leaves the counter negative, and the producer's local is an unsigned
> long, so every later test reads it as enormous and rejects. That is the
> same dead end, reached more easily.
> 
> While here, drain before clearing in the flush. A producer that queues
> between the clear and the drain currently has its skb drained but its
> increment kept, which over-counts and costs deferral until the next
> flush. Draining first matches the ordering __dev_xmit_skb() already uses
> for the qdisc defer_list.
> 
> Found by manual code inspection, LLM-assisted, while backporting the
> llist conversion to 6.1 after hitting the sd->defer_lock contention it
> removes. The latch itself was not hit in production; it was reproduced by
> lowering net.core.skb_defer_max.
> 
> Fixes: 844c9db7f7f5 ("net: use llist for sd->defer_list")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Joseph Kain <jkain@nuro.ai>
> ---
> v4:
>  - v3's "undo the increment on reject" was wrong: a flush that zeroes the
>    counter between a producer's increment and its undo leaves the counter
>    negative, and the producer's local is an unsigned long, so every later
>    test reads it as enormous and rejects forever. Test the limit before
>    incrementing instead, so there is no decrement to underflow
>    (netdev-bot sashiko).
> 
> Note this changes the effective limit from skb_defer_max - 1 to
> skb_defer_max, because the test now runs before the increment rather than
> after it. That matches what Documentation/admin-guide/sysctl/net.rst
> describes. It also means the half-capacity kick can now fire at
> skb_defer_max = 1, which commit c09b0cd2cc6c ("net: avoid strange
> behavior with skb_defer_max == 1") set out to make possible.
> 
> Built for x86_64 and arm64, with and without CONFIG_PREEMPT_RT. On
> v7.3-rc6 with skb_defer_max=1 and a cross-CPU TCP load, defer_count
> reached 1918118 unpatched and did not recover when the default was
> restored; with this patch it stays within the limit.
> 
> v3: https://lore.kernel.org/all/20261013205416.3417296-1-jkain@nuro.ai/
> v2: https://lore.kernel.org/all/20261008220256.1844072-1-jkain@nuro.ai/
> v1: https://lore.kernel.org/all/20261007180056.1394868-1-jkain@nuro.ai/
>  net/core/dev.c    | 5 ++++-
>  net/core/skbuff.c | 9 ++++++---
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 18dc88990510..60d86bc21632 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6940,8 +6940,11 @@ static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
>  
>  	if (llist_empty(&sdn->defer_list))
>  		return;
> -	atomic_long_set(&sdn->defer_count, 0);
>  	free_list = llist_del_all(&sdn->defer_list);
> +	/* Racy on purpose, as in __dev_xmit_skb(): a producer landing between
> +	 * the del_all and this store stays queued but uncounted.
> +	 */
> +	atomic_long_set(&sdn->defer_count, 0);
>  
>  	llist_for_each_entry_safe(skb, next, free_list, ll_node) {
>  		prefetch(next);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 4aea06d5167d..25f82b7dcf01 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -7386,11 +7386,14 @@ nodefer:	kfree_skb_napi_cache(skb);
>  	sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + cpu_to_node(my_cpu);
>  
>  	defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
> -	defer_count = atomic_long_inc_return(&sdn->defer_count);
> -
> -	if (defer_count >= defer_max)
> +	/* Test before incrementing: the reject path returns below without
> +	 * reaching kick_defer_list_purge(), so a kept increment never flushes.
> +	 */
> +	if (atomic_long_read(&sdn->defer_count) >= defer_max)
>  		goto nodefer;
>  
> +	defer_count = atomic_long_inc_return(&sdn->defer_count);
> +

IIUC, atomic_long_read() before atomic_long_inc_return() needs
two-step coherency transaction, and we could avoid that and
the corner case above by fixing the off-by-one and calling
kick_defer_list_purge() in the slow path:

if (unlikely(defer_count > defer_max)) {
    if (!llist_empty(&sdn->defer_list))
        kick_defer_list_purge(cpu);
    goto nodefer;
}


>  	llist_add(&skb->ll_node, &sdn->defer_list);
>  
>  	if (unlikely(!cpu_online(cpu) || my_cpu != raw_smp_processor_id())) {
> -- 
> 2.55.0
> 

Re: [PATCH net v4] net: do not let defer_count outlive an empty defer_list

[Joseph Kain]

On Fri, Oct 16, 2026 at 11:51 PM <kuniyu@google.com> wrote:
>
> On Thu, 15 Oct 2026 09:22:09 -0700, Joseph Kain wrote:
> > __skb_defer_free_flush() can leave defer_count at or above skb_defer_max
> > with an empty defer_list. That state is absorbing: every producer then
> > takes the nodefer path without queueing, so the list never refills.
> > Deferred freeing stays off for that (cpu, node) until reboot, and
> > cross-CPU frees fall back to the page allocator, which is the contention
> > deferral exists to avoid.
> >
> > skb_attempt_defer_free() increments before testing the limit and does not
> > undo the increment when it rejects, so a rejected free leaves the counter
> > raised with nothing queued.
> >
> > Nothing brings it back down. The reject path returns before
> > kick_defer_list_purge(), so once the counter is stuck no producer
> > schedules a flush on the owner CPU. A CPU that never raises NET_RX itself
> > -- an application CPU doing TX while NIC IRQs are pinned elsewhere and
> > RPS is off -- then never runs the flush at all, so clearing the counter
> > inside the flush cannot help it either. proc_do_skb_defer_max() neither
> > resets the counters nor schedules a flush when the limit changes.
> >
> > It is reachable with no race at all:
> >
> >   sysctl -w net.core.skb_defer_max=1    # frees increment, then reject
> >   sysctl -w net.core.skb_defer_max=128  # does not recover
> >
> > skb_defer_max has .extra1 = SYSCTL_ZERO and no .extra2, so 1 is settable.
> > From v7.1 on, 0 is safe because proc_do_skb_defer_max() flips
> > skb_defer_disable_key and the producer returns before incrementing. On
> > 6.18 through 7.0, which carry the llist conversion but not commit
> > 08dc30de1a40 ("net: add skb_defer_disable_key static key"), setting 0 and
> > restoring the previous value strands the counter the same way -- and 0 is
> > the documented way to turn the feature off.
> >
> > Test the limit before incrementing, so the counter is only ever raised by
> > a producer that goes on to queue. It then cannot outlive the skbs it
> > counts, and the absorbing state is unreachable without depending on a
> > flush ever running on the owner CPU.
>
> I think the state is still reachable when reducing the knob
> if the new value >> 1 is smaller than the current defer_count
> due to the kick condition (==).  This can be fixed by bumping
> the knob to defer_count * 2 though.
>
>
> >
> > This makes the limit advisory. The test no longer reserves the slot it
> > checks, and the recvmsg callers run in preemptible context, so the
> > overshoot is bounded by the number of producers in flight rather than by
> > skb_defer_max; the post-increment test reserved the slot and bounded the
> > queue exactly. The counter is already approximate -- the flush zeroes it
> > without regard for producers in flight -- and the next flush erases the
> > overshoot, where the latch it replaces never recovered at all.
> >
> > Undoing the increment on the reject path instead would be wrong: a flush
> > that zeroes the counter between a producer's increment and its undo
> > leaves the counter negative, and the producer's local is an unsigned
> > long, so every later test reads it as enormous and rejects. That is the
> > same dead end, reached more easily.
> >
> > While here, drain before clearing in the flush. A producer that queues
> > between the clear and the drain currently has its skb drained but its
> > increment kept, which over-counts and costs deferral until the next
> > flush. Draining first matches the ordering __dev_xmit_skb() already uses
> > for the qdisc defer_list.
> >
> > Found by manual code inspection, LLM-assisted, while backporting the
> > llist conversion to 6.1 after hitting the sd->defer_lock contention it
> > removes. The latch itself was not hit in production; it was reproduced by
> > lowering net.core.skb_defer_max.
> >
> > Fixes: 844c9db7f7f5 ("net: use llist for sd->defer_list")
> > Assisted-by: Claude:claude-opus-5
> > Signed-off-by: Joseph Kain <jkain@nuro.ai>
> > ---
> > v4:
> >  - v3's "undo the increment on reject" was wrong: a flush that zeroes the
> >    counter between a producer's increment and its undo leaves the counter
> >    negative, and the producer's local is an unsigned long, so every later
> >    test reads it as enormous and rejects forever. Test the limit before
> >    incrementing instead, so there is no decrement to underflow
> >    (netdev-bot sashiko).
> >
> > Note this changes the effective limit from skb_defer_max - 1 to
> > skb_defer_max, because the test now runs before the increment rather than
> > after it. That matches what Documentation/admin-guide/sysctl/net.rst
> > describes. It also means the half-capacity kick can now fire at
> > skb_defer_max = 1, which commit c09b0cd2cc6c ("net: avoid strange
> > behavior with skb_defer_max == 1") set out to make possible.
> >
> > Built for x86_64 and arm64, with and without CONFIG_PREEMPT_RT. On
> > v7.3-rc6 with skb_defer_max=1 and a cross-CPU TCP load, defer_count
> > reached 1918118 unpatched and did not recover when the default was
> > restored; with this patch it stays within the limit.
> >
> > v3: https://lore.kernel.org/all/20261013205416.3417296-1-jkain@nuro.ai/
> > v2: https://lore.kernel.org/all/20261008220256.1844072-1-jkain@nuro.ai/
> > v1: https://lore.kernel.org/all/20261007180056.1394868-1-jkain@nuro.ai/
> >  net/core/dev.c    | 5 ++++-
> >  net/core/skbuff.c | 9 ++++++---
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 18dc88990510..60d86bc21632 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -6940,8 +6940,11 @@ static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
> >
> >       if (llist_empty(&sdn->defer_list))
> >               return;
> > -     atomic_long_set(&sdn->defer_count, 0);
> >       free_list = llist_del_all(&sdn->defer_list);
> > +     /* Racy on purpose, as in __dev_xmit_skb(): a producer landing between
> > +      * the del_all and this store stays queued but uncounted.
> > +      */
> > +     atomic_long_set(&sdn->defer_count, 0);
> >
> >       llist_for_each_entry_safe(skb, next, free_list, ll_node) {
> >               prefetch(next);
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 4aea06d5167d..25f82b7dcf01 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -7386,11 +7386,14 @@ nodefer:      kfree_skb_napi_cache(skb);
> >       sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + cpu_to_node(my_cpu);
> >
> >       defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
> > -     defer_count = atomic_long_inc_return(&sdn->defer_count);
> > -
> > -     if (defer_count >= defer_max)
> > +     /* Test before incrementing: the reject path returns below without
> > +      * reaching kick_defer_list_purge(), so a kept increment never flushes.
> > +      */
> > +     if (atomic_long_read(&sdn->defer_count) >= defer_max)
> >               goto nodefer;
> >
> > +     defer_count = atomic_long_inc_return(&sdn->defer_count);
> > +
>
> IIUC, atomic_long_read() before atomic_long_inc_return() needs
> two-step coherency transaction, and we could avoid that and
> the corner case above by fixing the off-by-one and calling
> kick_defer_list_purge() in the slow path:
>
> if (unlikely(defer_count > defer_max)) {
>     if (!llist_empty(&sdn->defer_list))
>         kick_defer_list_purge(cpu);
>     goto nodefer;
> }
>
>
> >       llist_add(&skb->ll_node, &sdn->defer_list);
> >
> >       if (unlikely(!cpu_online(cpu) || my_cpu != raw_smp_processor_id())) {
> > --
> > 2.55.0
> >

Thanks kuniyu, I'll follow up on these in v6.

Re: [PATCH net v4] net: do not let defer_count outlive an empty defer_list

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch makes the empty-list latch unreachable, but a
  closely related latch on a non-empty list is still reachable.
- [Low] Two user-visible behaviour changes in skb_attempt_defer_free()
  appear only in the v4 notes below the '---' line, so they will be…
- [Low] Documentation/admin-guide/sysctl/net.rst (lines 362-368) still
  calls skb_defer_max the 'Max size (in skbs) of the per-cpu list of…

--

net: do not let defer_count outlive an empty defer_list

This patch moves the skb_defer_max test in skb_attempt_defer_free() ahead
of the atomic increment. A rejected free then no longer leaves defer_count
raised with nothing queued. It also changes __skb_defer_free_flush() to
drain the list before it zeroes defer_count.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 41beaf625421a..411cbf6754f9e 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -7390,11 +7390,14 @@ nodefer:	kfree_skb_napi_cache(skb);
>  	sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + cpu_to_node(my_cpu);
>  
>  	defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
> -	defer_count = atomic_long_inc_return(&sdn->defer_count);
> -
> -	if (defer_count >= defer_max)
> +	/* Test before incrementing: the reject path returns below without
> +	 * reaching kick_defer_list_purge(), so a kept increment never flushes.
> +	 */
> +	if (atomic_long_read(&sdn->defer_count) >= defer_max)
>  		goto nodefer;

[Severity: Medium]

This fixes the empty-list case. Can a non-empty list still get stuck the
same way?

The reject path here still returns through nodefer without calling
kick_defer_list_purge(). The only kick is an exact-equality test later in
skb_attempt_defer_free():

	kick = (defer_count - 1) == (defer_max >> 1);

proc_do_skb_defer_max() only acts when the value crosses 0:

	if (nval != oval) {
		if (nval)
			static_branch_enable(&skb_defer_disable_key);
		else
			static_branch_disable(&skb_defer_disable_key);
	}

So a change from one positive value to another does not reset
defer_count, does not flush, and does not kick.

If a limit change leaves the count at or past the new half-capacity
point, the kick never matches. The counter then climbs to the limit (or
is already above it), and every later producer rejects without a kick.
For example:

  skb_defer_max=128, count in 51..64 (the 128 kick only fires at 65)
  sysctl -w net.core.skb_defer_max=100   # half is 50, already passed
  count climbs to 100, then every producer rejects
  sysctl -w net.core.skb_defer_max=128   # half is 64, count is 100
  pre-increment 100..127 never equals 64, so it stays stuck at 128

A simpler case: lower the limit from 128 to 32 while 40 skbs are queued.
Every producer then reads 40 >= 32 and rejects.

A preemptible recvmsg producer can also miss the kick. If it samples a
larger defer_max and the limit is lowered before it increments, it
compares against the old half and does not kick.

The owner CPU here is the one the commit message describes.
napi_consume_skb() defers TX-completion frees to skb->alloc_cpu, so a
TX-only application CPU that never raises NET_RX owns these lists.

Deferral for that (cpu, node) stays off. The queued skbs and their pages
stay pinned until something runs net_rx_action() or busy-poll on that
CPU, or the CPU goes offline.

The commit message says:

  "the absorbing state is unreachable without depending on a flush ever
   running on the owner CPU"

Does that hold only for the empty-list state? The sequence above restores
the default value and still does not recover, which is the same symptom
as the reproducer in the commit message.

Would it work to kick on reject when the list is non-empty, rate-limited
through defer_ipi_scheduled or the backlog NAPI SCHED bit? Another option
is to make the kick a threshold-crossing test. A third is to have
proc_do_skb_defer_max() flush or kick the defer nodes when the limit
changes.

>  
> +	defer_count = atomic_long_inc_return(&sdn->defer_count);
> +

[Severity: Low]

This isn't a bug, but the changelog body leaves out two behaviour changes
that come from testing before the increment. They are only described in
the v4 notes below the '---' line, and git am drops that part.

First, the effective per-(cpu, node) limit moves from skb_defer_max - 1
to skb_defer_max, because the old code rejected on the post-increment
value.

Second, the half-capacity kick can now fire at small limits. With
skb_defer_max=1, the old post-increment value of 1 was always rejected,
so no kick was ever sent. Now the first producer after each flush sees a
pre-increment value of 0, which equals 1 >> 1, so it kicks. With
skb_defer_max=2, the kick now fires on the second producer.

Could these two changes go into the commit message itself, for stable
backporters and for anyone tuning skb_defer_max?

>  	llist_add(&skb->ll_node, &sdn->defer_list);

[Severity: Low]

The commit message says "This makes the limit advisory", but
Documentation/admin-guide/sysctl/net.rst still describes skb_defer_max
as:

  Max size (in skbs) of the per-cpu list of skbs being freed
  by the cpu which allocated them.

The atomic_long_read() check and the atomic_long_inc_return() are now
separate steps. Producers that read the same below-limit value all pass
and call llist_add(). The recvmsg callers can also be preempted between
the two steps. The list can therefore exceed skb_defer_max by roughly the
number of producers in flight.

__skb_defer_free_flush() now does:

	free_list = llist_del_all(&sdn->defer_list);
	...
	atomic_long_set(&sdn->defer_count, 0);

Any skb queued between those two steps stays on the list without being
counted, and up to skb_defer_max more can be added on top of it.

The old code was not exact either, and the doc already says per-cpu for
a list that is really per-(cpu, node). Should the sysctl documentation be
updated to say the limit is approximate?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015162209.485940-1-jkain%40nuro.ai