[PATCH net v7] net: stop defer_count outliving the deferred-free list

[PATCH net v7] net: stop defer_count outliving the deferred-free list

[Joseph Kain]

After the llist conversion, rejected frees retain their counter
increments. A producer racing the counter reset and list detach can
also leave a count for an skb already drained. Deferred freeing can
then remain disabled while the list is empty.

Undo rejected reservations and subtract only the number of skbs
drained. Allow admission up to the configured limit. Retain the
halfway notification and also notify when the reserved count reaches
the limit, covering reservations published after an earlier purge.

Fixes: 844c9db7f7f5 ("net: use llist for sd->defer_list")
Signed-off-by: Joseph Kain <jkain@nuro.ai>
Assisted-by: LLM
---
For 6.18-7.1, apply the drain change inside each NUMA-node iteration
of skb_defer_free_flush(), using sdn->defer_list and sdn->defer_count.
Initialize n to zero for each node.
---
 net/core/dev.c    |  8 +++++++-
 net/core/skbuff.c | 17 ++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 18dc88990510..7843ef511d7e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6937,16 +6937,22 @@ static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
 {
 	struct llist_node *free_list;
 	struct sk_buff *skb, *next;
+	long n = 0;
 
 	if (llist_empty(&sdn->defer_list))
 		return;
-	atomic_long_set(&sdn->defer_count, 0);
 	free_list = llist_del_all(&sdn->defer_list);
 
 	llist_for_each_entry_safe(skb, next, free_list, ll_node) {
 		prefetch(next);
 		napi_consume_skb(skb, budget);
+		n++;
 	}
+	/* Subtract exactly what was drained. Never store an absolute value:
+	 * mixing one with the relative updates in skb_attempt_defer_free()
+	 * lets the counter go negative, and it is read as unsigned there.
+	 */
+	atomic_long_sub(n, &sdn->defer_count);
 }
 
 void skb_defer_node_flush(struct skb_defer_node *sdn)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4aea06d5167d..b1d3fd783283 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -7388,8 +7388,14 @@ nodefer:	kfree_skb_napi_cache(skb);
 	defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
 	defer_count = atomic_long_inc_return(&sdn->defer_count);
 
-	if (defer_count >= defer_max)
+	if (defer_count > defer_max) {
+		/* Undo the reservation. An llist_add() does not release one --
+		 * the skb stays charged until a flush subtracts the batch that
+		 * contains it -- so a rejected free must undo its own.
+		 */
+		atomic_long_dec(&sdn->defer_count);
 		goto nodefer;
+	}
 
 	llist_add(&skb->ll_node, &sdn->defer_list);
 
@@ -7398,8 +7404,13 @@ nodefer:	kfree_skb_napi_cache(skb);
 		return;
 	}
 
-	/* Send an IPI every time queue reaches half capacity. */
-	kick = (defer_count - 1) == (defer_max >> 1);
+	/* Half capacity, plus the skb that fills the queue. Equality alone can
+	 * be consumed early -- if the producer holding the kick value publishes
+	 * first, its purge drains only that skb and the reservations published
+	 * after it match nothing -- which would leave a full queue with nothing
+	 * pending.
+	 */
+	kick = (defer_count - 1) == (defer_max >> 1) || defer_count == defer_max;
 
 	/* Make sure to trigger NET_RX_SOFTIRQ on the remote CPU
 	 * if we are unlucky enough (this seems very unlikely).
-- 
2.55.0

Re: [PATCH net v7] net: stop defer_count outliving the deferred-free list

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v7] net: stop defer_count outliving the deferred-free list

[Joseph Kain]

On Thu, Oct 29, 2026 at 2:13 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.


Thanks, both are fair, and I should have carried this over from v6.

Found by manual code inspection, LLM-assisted, while backporting
844c9db7f7f5 ("net: use llist for sd->defer_list") to a 6.1 vendor
kernel.

Not triggered in production. The absorbing state is reachable with no
race at all, and was reproduced deterministically in a VM: set
net.core.skb_defer_max=1, drive cross-CPU receive traffic until
defer_count climbs past the old limit, then restore it.

There is no crash or splat — the failure is silent. On an unpatched
7.3-rc6 kernel defer_count reached 2036497 at the lowered limit and
3995713 after restoring 128, still climbing, and zero skbs were queued
over the following 794847 skb_attempt_defer_free() calls: deferred
freeing was off for that (cpu, node) and cross-CPU frees fell back to
the page allocator. With this patch the counter peaks at 1, returns to
0, and 4282 purges were requested over 797687 calls.

If another revision is needed for other reasons I will fold this into
the commit message.

Re: [PATCH net v7] net: stop defer_count outliving the deferred-free list

[netdev-bot+sashiko]

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

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

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] __skb_defer_free_flush() (net/core/dev.c) now holds the whole
  drained batch's reservation on sdn->defer_count until every detached…

Pre-existing issues:
- [Medium] A runtime change to net.core.skb_defer_max can leave a
  per-(cpu, node) defer queue at or above the new limit with no purge…
- [Low] Documentation/admin-guide/sysctl/net.rst (lines 362-368)
  describes skb_defer_max as the "Max size (in skbs) of the per-cpu list…

--

net: stop defer_count outliving the deferred-free list

This commit changes the deferred skb free accounting. A rejected free now
undoes its own defer_count increment, and __skb_defer_free_flush()
subtracts the number of drained skbs instead of resetting the counter to
zero. It also allows admission up to defer_max and adds a remote kick when
the reserved count reaches defer_max.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 3ac700d37e596..f5ed0ecf534ac 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7073,16 +7073,22 @@ static void __skb_defer_free_flush(struct skb_defer_node *sdn, int budget)
>  {
>  	struct llist_node *free_list;
>  	struct sk_buff *skb, *next;
> +	long n = 0;
>  
>  	if (llist_empty(&sdn->defer_list))
>  		return;
> -	atomic_long_set(&sdn->defer_count, 0);
>  	free_list = llist_del_all(&sdn->defer_list);
>  
>  	llist_for_each_entry_safe(skb, next, free_list, ll_node) {
>  		prefetch(next);
>  		napi_consume_skb(skb, budget);
> +		n++;
>  	}
> +	/* Subtract exactly what was drained. Never store an absolute value:
> +	 * mixing one with the relative updates in skb_attempt_defer_free()
> +	 * lets the counter go negative, and it is read as unsigned there.
> +	 */
> +	atomic_long_sub(n, &sdn->defer_count);

[Severity: Medium]

With the subtraction moved after the napi_consume_skb() loop, does
deferral stay disabled for the whole drain?

The list is empty after llist_del_all(), but sdn->defer_count still
counts every detached skb until atomic_long_sub() runs. Suppose a full
queue (defer_max skbs, default 128) is flushed through
net_rx_action()->skb_defer_free_flush()->__skb_defer_free_flush(). The
counter then stays at about defer_max for the entire free loop.

A producer that calls skb_attempt_defer_free() for the same (alloc cpu,
node) pair during that window would hit:

	defer_count = atomic_long_inc_return(&sdn->defer_count);

	if (defer_count > defer_max) {
		atomic_long_dec(&sdn->defer_count);
		goto nodefer;
	}

It would then fall back to kfree_skb_napi_cache(), which is the
cross-CPU free that deferral is meant to avoid.

The earlier versions both released the budget before freeing. The
spinlock version did atomic_set(&sd->defer_count, 0) under defer_lock
before the free loop. Commit 844c9db7f7f5 did atomic_long_set() before
llist_del_all().

The detached skbs are no longer on the list. Could n be computed first
and subtracted before the free loop? A counting pass over free_list would
do it, and would also warm the cache lines that napi_consume_skb()
touches. Neither the commit message nor the new comment mentions this
change to the admission window.

>  }
>  
>  void skb_defer_node_flush(struct skb_defer_node *sdn)
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 48f6764117243..68192e477ff1f 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -7499,8 +7499,14 @@ nodefer:	kfree_skb_napi_cache(skb);
>  	defer_max = READ_ONCE(net_hotdata.sysctl_skb_defer_max);
>  	defer_count = atomic_long_inc_return(&sdn->defer_count);
>  
> -	if (defer_count >= defer_max)
> +	if (defer_count > defer_max) {
> +		/* Undo the reservation. An llist_add() does not release one --
> +		 * the skb stays charged until a flush subtracts the batch that
> +		 * contains it -- so a rejected free must undo its own.
> +		 */
> +		atomic_long_dec(&sdn->defer_count);
>  		goto nodefer;
> +	}

[Severity: Low]

This isn't a bug introduced by this patch, 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 limit is actually applied separately to each (allocating cpu,
freeing node) list:

	sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + cpu_to_node(my_cpu);

net_dev_init() allocates nr_node_ids of these lists per CPU. One
allocating CPU can therefore have up to defer_max * nr_node_ids skbs
pending.

The documentation also doesn't say that writing 0 disables deferral
entirely, which proc_do_skb_defer_max() does through
skb_defer_disable_key.

The switch from >= to > makes the per-list bound match "Max size". The
per-node scope and the meaning of 0 still differ from the text. Should
net.rst be updated to describe them?

>  
>  	llist_add(&skb->ll_node, &sdn->defer_list);
>  
> @@ -7509,8 +7515,13 @@ nodefer:	kfree_skb_napi_cache(skb);
>  		return;
>  	}
>  
> -	/* Send an IPI every time queue reaches half capacity. */
> -	kick = (defer_count - 1) == (defer_max >> 1);
> +	/* Half capacity, plus the skb that fills the queue. Equality alone can
> +	 * be consumed early -- if the producer holding the kick value publishes
> +	 * first, its purge drains only that skb and the reservations published
> +	 * after it match nothing -- which would leave a full queue with nothing
> +	 * pending.
> +	 */
> +	kick = (defer_count - 1) == (defer_max >> 1) || defer_count == defer_max;

[Severity: Medium]

This is a pre-existing issue, but can a runtime change to
net.core.skb_defer_max still leave a per-(cpu, node) queue at or above
the new limit with no purge pending?

Both kick conditions are equality tests against the defer_max that was
read earlier with READ_ONCE(). The rejection branch above returns
without kicking.

proc_do_skb_defer_max() only toggles skb_defer_disable_key when the value
changes to or from 0. It doesn't drain or kick the existing lists:

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

Take a limit of 128 with 31 skbs queued, which is below the
half-capacity kick, so nothing is pending. If the limit is lowered to 16,
every later producer gets 32 > 16, undoes its reservation and returns.
The count never passes through 9 or 16, so neither kick fires.

In another case, a producer reads 128, the limit is lowered to 32, and
that producer then reserves 32. Both of its checks (31 == 64 and
32 == 128) fail. Later producers get 33 > 32 and are rejected.

In both cases deferral for that pair stays off. The queued skbs stay
pinned until the allocating CPU runs net_rx_action() or
napi_threaded_poll_loop() for some unrelated reason. On a CPU that only
allocates TX skbs, that may never happen.

The new full-capacity kick only guarantees a pending purge for a full
queue while the limit stays constant. Should the rejection path also
kick, or should the sysctl handler flush the existing lists?

[ ... ]

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

Re: [PATCH net v7] net: stop defer_count outliving the deferred-free list

[Joseph Kain]

On Mon, Nov 2, 2026 at 1:21 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> With the subtraction moved after the napi_consume_skb() loop, does
> deferral stay disabled for the whole drain?

Yes, for a full batch. The detached skbs remain charged until the
freeing loop completes; smaller batches can leave capacity for
further admissions. I should have explained this admission-window
change.

Preliminary non-PREEMPT_RT x86 VM tests recorded no limit rejections
in either variant at skb_defer_max=128. Results at 16 and 32 showed
mixed rejection and purge counts. Throughput and CPU cost were not
measured, so these results do not establish a performance advantage
for either ordering.

I propose retaining the current single-pass drain for now. If
another revision is needed, I will explicitly document that the
detached batch remains charged until freeing completes.

> This is a pre-existing issue, but can a runtime change to
> net.core.skb_defer_max still leave a per-(cpu, node) queue at or above
> the new limit with no purge pending?

Agreed, and as you say it is pre-existing: neither the reject path nor
proc_do_skb_defer_max() kicks, before or after this patch. Both of your
examples look right to me. It is out of scope for this patch, which is
about counts outliving the list.