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

[PATCH net] 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, and
the flush keeps returning early without clearing. 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.

Two changes from commit 844c9db7f7f5 ("net: use llist for sd->defer_list")
combine to produce it.

skb_attempt_defer_free() now 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.

The flush clears defer_count before llist_del_all() rather than together
with it under defer_lock as it did before. A remote producer that queues
between the two has its skb drained by that same llist_del_all() but its
increment retained, so the counter drifts upward until it reaches the
limit.

Neither needs PREEMPT_RT. The drift needs no preemption at all: the
flush normally runs in NAPI poll on the CPU that owns the list, while
skb_attempt_defer_free() only ever defers to a remote CPU, so producers
cannot be excluded by disabling preemption on the flusher. PREEMPT_RT
only widens the window, because local_bh_disable() does not disable
preemption, so the flusher can be scheduled out between the clear and
the drain.

The latch is also 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.
0 is safe, because proc_do_skb_defer_max() flips skb_defer_disable_key
and the producer returns before incrementing, but that guard covers 0
only. Recovering from the latched state needs a skb_defer_max above the
accumulated count, which an operator has no way to determine.

Drain before clearing, so the error can only under-read, is bounded by
the producers in flight, and self-heals on the next flush. Clear the
counter on the empty-list path as well, so no count can outlive an empty
list and an already-stranded counter recovers.

llist_empty() has already loaded that cache line, so the added read is
free; the unlikely() keeps the store off the hot path, where it would
otherwise bounce the line against remote producers.

Found on an arm64 PREEMPT_RT system running the two commits backported to
6.1. With skb_defer_max lowered, defer_count reached 788,888 on one host
and 122,399 on another with the list empty throughout, and restoring the
default did not re-enable deferral.

Fixes: 844c9db7f7f5 ("net: use llist for sd->defer_list")
Signed-off-by: Joseph Kain <jkain@nuro.ai>
---
 net/core/dev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 18dc88990510..043a7f9166a9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6938,10 +6938,13 @@ static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
 	struct llist_node *free_list;
 	struct sk_buff *skb, *next;
 
-	if (llist_empty(&sdn->defer_list))
+	if (llist_empty(&sdn->defer_list)) {
+		if (unlikely(atomic_long_read(&sdn->defer_count)))
+			atomic_long_set(&sdn->defer_count, 0);
 		return;
-	atomic_long_set(&sdn->defer_count, 0);
+	}
 	free_list = llist_del_all(&sdn->defer_list);
+	atomic_long_set(&sdn->defer_count, 0);
 
 	llist_for_each_entry_safe(skb, next, free_list, ll_node) {
 		prefetch(next);
-- 
2.55.0

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

[Eric Dumazet]

Le mer. 7 oct. 2026 à 20:03, Joseph Kain <jkain@nuro.ai> a écrit :
>
> __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, and
> the flush keeps returning early without clearing. 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.
>
> Two changes from commit 844c9db7f7f5 ("net: use llist for sd->defer_list")
> combine to produce it.
>
> skb_attempt_defer_free() now 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.
>
> The flush clears defer_count before llist_del_all() rather than together
> with it under defer_lock as it did before. A remote producer that queues
> between the two has its skb drained by that same llist_del_all() but its
> increment retained, so the counter drifts upward until it reaches the
> limit.

This is not cumulative: the next flush finding a non-empty list resets
the counter. To get stuck, (skb_defer_max - 1) increments must land
between the clear and the drain of a single flush. With small limits this
needs nothing special, but with the default of 128 the flusher has to be
interrupted or preempted at that point. Please reword this part and the
"needs no preemption at all" paragraph.

>
> Neither needs PREEMPT_RT. The drift needs no preemption at all: the
> flush normally runs in NAPI poll on the CPU that owns the list, while
> skb_attempt_defer_free() only ever defers to a remote CPU, so producers
> cannot be excluded by disabling preemption on the flusher. PREEMPT_RT
> only widens the window, because local_bh_disable() does not disable
> preemption, so the flusher can be scheduled out between the clear and
> the drain.
>
> The latch is also 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.
> 0 is safe, because proc_do_skb_defer_max() flips skb_defer_disable_key
> and the producer returns before incrementing, but that guard covers 0
> only. Recovering from the latched state needs a skb_defer_max above the
> accumulated count, which an operator has no way to determine.
>
> Drain before clearing, so the error can only under-read, is bounded by
> the producers in flight, and self-heals on the next flush. Clear the
> counter on the empty-list path as well, so no count can outlive an empty
> list and an already-stranded counter recovers.
>
> llist_empty() has already loaded that cache line, so the added read is
> free; the unlikely() keeps the store off the hot path, where it would
> otherwise bounce the line against remote producers.
>
> Found on an arm64 PREEMPT_RT system running the two commits backported to
> 6.1. With skb_defer_max lowered, defer_count reached 788,888 on one host
> and 122,399 on another with the list empty throughout, and restoring the
> default did not re-enable deferral.

Not sure if this paragraph is needed?

Lowered to which value? Have you seen this with the default?

If the value was 0, note that kernels without commit 08dc30de1a40
("net: add skb_defer_disable_key static key"), i.e. 6.18 to 7.0, have the
same issue as your skb_defer_max=1 example: setting 0 and restoring the
previous value leaves deferral disabled. This is worth mentioning, since
0 is the documented way to turn the feature off.

Please send a v2 with these changes (but please wait ~24 hours before V2)

>
> Fixes: 844c9db7f7f5 ("net: use llist for sd->defer_list")
> Signed-off-by: Joseph Kain <jkain@nuro.ai>

This is your first linux contribution, are you sure you have not used an LLM?

( See Assisted-by: LLM) tag in Documentation/process/coding-assistants.rst

pw-bot: cr

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

[Joseph Kain]

(resending with the list cc'd.  My previous reply went to Eric only)

On Wed, Oct 7, 2026 at 1:23=E2=80=AFPM Eric Dumazet <edumazet@kernel.org> w=
rote:
>
> Le mer. 7 oct. 2026 =C3=A0 20:03, Joseph Kain <jkain@nuro.ai> a =C3=A9cri=
t :
> >
> > __skb_defer_free_flush() can leave defer_count at or above skb_defer_ma=
x
> > with an empty defer_list. That state is absorbing: every producer then
> > takes the nodefer path without queueing, so the list never refills, and
> > the flush keeps returning early without clearing. Deferred freeing stay=
s
> > 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.
> >
> > Two changes from commit 844c9db7f7f5 ("net: use llist for sd->defer_lis=
t")
> > combine to produce it.
> >
> > skb_attempt_defer_free() now increments before testing the limit and do=
es
> > not undo the increment when it rejects, so a rejected free leaves the
> > counter raised with nothing queued.
> >
> > The flush clears defer_count before llist_del_all() rather than togethe=
r
> > with it under defer_lock as it did before. A remote producer that queue=
s
> > between the two has its skb drained by that same llist_del_all() but it=
s
> > increment retained, so the counter drifts upward until it reaches the
> > limit.
>
> This is not cumulative: the next flush finding a non-empty list resets
> the counter. To get stuck, (skb_defer_max - 1) increments must land
> between the clear and the drain of a single flush. With small limits this
> needs nothing special, but with the default of 128 the flusher has to be
> interrupted or preempted at that point. Please reword this part and the
> "needs no preemption at all" paragraph.

Thanks, you are right; any later flush finding a non-empty list clears the
counter.  I will rewrite this portion in the upcoming v2.

>
> >
> > Neither needs PREEMPT_RT. The drift needs no preemption at all: the
> > flush normally runs in NAPI poll on the CPU that owns the list, while
> > skb_attempt_defer_free() only ever defers to a remote CPU, so producers
> > cannot be excluded by disabling preemption on the flusher. PREEMPT_RT
> > only widens the window, because local_bh_disable() does not disable
> > preemption, so the flusher can be scheduled out between the clear and
> > the drain.
> >
> > The latch is also reachable with no race at all:
> >
> >   sysctl -w net.core.skb_defer_max=3D1    # frees increment, then rejec=
t
> >   sysctl -w net.core.skb_defer_max=3D128  # does not recover
> >
> > skb_defer_max has .extra1 =3D SYSCTL_ZERO and no .extra2, so 1 is setta=
ble.
> > 0 is safe, because proc_do_skb_defer_max() flips skb_defer_disable_key
> > and the producer returns before incrementing, but that guard covers 0
> > only. Recovering from the latched state needs a skb_defer_max above the
> > accumulated count, which an operator has no way to determine.
> >
> > Drain before clearing, so the error can only under-read, is bounded by
> > the producers in flight, and self-heals on the next flush. Clear the
> > counter on the empty-list path as well, so no count can outlive an empt=
y
> > list and an already-stranded counter recovers.
> >
> > llist_empty() has already loaded that cache line, so the added read is
> > free; the unlikely() keeps the store off the hot path, where it would
> > otherwise bounce the line against remote producers.
> >
> > Found on an arm64 PREEMPT_RT system running the two commits backported =
to
> > 6.1. With skb_defer_max lowered, defer_count reached 788,888 on one hos=
t
> > and 122,399 on another with the list empty throughout, and restoring th=
e
> > default did not re-enable deferral.
>
> Not sure if this paragraph is needed?
>
> Lowered to which value? Have you seen this with the default?
>
> If the value was 0, note that kernels without commit 08dc30de1a40
> ("net: add skb_defer_disable_key static key"), i.e. 6.18 to 7.0, have the
> same issue as your skb_defer_max=3D1 example: setting 0 and restoring the
> previous value leaves deferral disabled. This is worth mentioning, since
> 0 is the documented way to turn the feature off.

Thanks, I will add that. The backport I reproduced on also predates the
 static key, so this is the case I hit.

>
> Please send a v2 with these changes (but please wait ~24 hours before V2)
>
> >
> > Fixes: 844c9db7f7f5 ("net: use llist for sd->defer_list")
> > Signed-off-by: Joseph Kain <jkain@nuro.ai>
>
> This is your first linux contribution, are you sure you have not used an =
LLM?
>
> ( See Assisted-by: LLM) tag in Documentation/process/coding-assistants.rs=
t

Oh, I certainly have used an LLM but missed the documentation.  I
will include the Assisted-tag in v2.


>
> pw-bot: cr

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

[netdev-bot+sashiko]

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

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

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).

- [High] The fix depends on __skb_defer_free_flush() running on the CPU
  that owns the list.

--

net: do not let defer_count outlive an empty defer_list

The patch reorders __skb_defer_free_flush() so that defer_count is cleared
after llist_del_all() instead of before it. It also clears a non-zero
defer_count when the defer_list is found empty. The goal is to recover a
(cpu, node) whose counter is stranded at or above skb_defer_max.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index e76762e29360e..e847e243a692e 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6938,10 +6938,13 @@ static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
>  	struct llist_node *free_list;
>  	struct sk_buff *skb, *next;
>  
> -	if (llist_empty(&sdn->defer_list))
> +	if (llist_empty(&sdn->defer_list)) {
> +		if (unlikely(atomic_long_read(&sdn->defer_count)))
> +			atomic_long_set(&sdn->defer_count, 0);
>  		return;

[Severity: High]
Does this reset run in the stranded state?

__skb_defer_free_flush() only runs for this_cpu's nodes. Its callers are
net_rx_action(), threaded NAPI, busy poll and CPU hotplug. The producer
side, skb_attempt_defer_free() in net/core/skbuff.c, increments and
rejects before it reaches the only kick:

net/core/skbuff.c:skb_attempt_defer_free() {
    ...
	defer_count = atomic_long_inc_return(&sdn->defer_count);

	if (defer_count >= defer_max)
		goto nodefer;

	llist_add(&skb->ll_node, &sdn->defer_list);
    ...
	kick = (defer_count - 1) == (defer_max >> 1);
    ...
	if (unlikely(kick))
		kick_defer_list_purge(cpu);
}

Once defer_count >= defer_max, every producer goes through nodefer and
returns. None of them calls kick_defer_list_purge(), so nothing raises
NET_RX_SOFTIRQ on the owner CPU.

napi_consume_skb() sends every alien TX-completion skb here. The owner can
be an application CPU that allocates TX skbs but does no NAPI, RPS/RFS or
backlog processing itself. On such a CPU, does the commit's own reproducer
still latch?

  sysctl -w net.core.skb_defer_max=1    # every free rejected, count climbs
  sysctl -w net.core.skb_defer_max=128  # count already >= 128, no kick

In that case the new reset in __skb_defer_free_flush() never runs, and
deferral stays off for that (cpu, node).

The same thing seems to happen in two other cases. With defer_max=2, one
skb gets queued but the kick condition (defer_count - 1) == 1 never matches
before the reject. It also happens whenever skb_defer_max is lowered below
the current count. proc_do_skb_defer_max() does not reset the per-node
counters or request a flush when a positive value is restored.

The commit message says:

  "Clear the counter on the empty-list path as well, so no count can
  outlive an empty list and an already-stranded counter recovers."

Is that only true when the owner CPU runs NET_RX for unrelated reasons?

Would a producer-side change also be needed to close this? Two options are
undoing the increment with atomic_long_dec() on the reject path, or kicking
or flushing the owner on reject.

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

[ ... ]

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