* [PATCH net v6] net: schedule a purge when the defer queue rejects a free
@ 2026-10-19 17:42 Joseph Kain
2026-10-19 17:58 ` Eric Dumazet
2026-10-20 17:43 ` sashiko-bot
0 siblings, 2 replies; 7+ messages in thread
From: Joseph Kain @ 2026-10-19 17:42 UTC (permalink / raw)
To: netdev
Cc: edumazet, davem, kuba, pabeni, horms, kuniyu, kerneljasonxing,
bigeasy, clrkwllms, rostedt, linux-rt-devel, linux-kernel,
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 keeps
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.
Kick the purge from the reject path instead. The reject is the one place
that observes a full queue, so scheduling the flush there is what stops
the counter outliving the list, and it does not depend on the owner CPU
ever raising NET_RX on its own. kick_defer_list_purge() already coalesces
through sd->defer_ipi_scheduled, so a burst of rejects costs one IPI.
This also covers the case where skb_defer_max is lowered below the
current defer_count. The kick condition tests equality with half the
limit, so a reduction can move that point behind the counter and no
later producer ever reaches it; rejecting without a kick then strands a
non-empty list the same way. The reject-path kick recovers both.
While here, test the limit with > rather than >=, so the effective limit
is skb_defer_max rather than skb_defer_max - 1, matching what
Documentation/admin-guide/sysctl/net.rst describes.
That also makes the half-capacity kick reachable at small limits. At
skb_defer_max = 1 nothing was ever queued, so no kick was ever sent, and
at 2 the equality was never hit; both now kick. That is what commit
c09b0cd2cc6c ("net: avoid strange behavior with skb_defer_max == 1") set
out to allow.
Also 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")
Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Joseph Kain <jkain@nuro.ai>
---
v6:
- Keep the increment and kick the purge from the reject path, rather
than testing before incrementing as v4 did. One atomic instead of a
read followed by an RMW, the increment still reserves the slot so the
limit stays exact, and it also recovers the case where skb_defer_max
is lowered below the current defer_count, which v4 did not
(Kuniyuki Iwashima; also reported against v3 and v4 by
netdev-bot sashiko, whose suggested fix this is).
- Describe the two resulting behaviour changes -- the effective limit
and the kick at small limits -- in the commit message rather than
only here, since git am drops this part (netdev-bot sashiko).
Measured on v7.3-rc6 in a VM, 8 cpus, cross-CPU TCP load, sampling
defer_count via bpftrace. Dropping net.core.skb_defer_max from 128 to 1
and then restoring 128:
7b63ef2d55f2 (unpatched) defer_count climbed 701404 -> 3244783 and
stayed at 3244783 after restoring 128
this patch defer_count stayed within 0..1 throughout
and returned to 0
With skb_defer_max=1 and a cross-CPU TCP load, kick_defer_list_purge()
is called 0 times before this patch and ~756k times after, over ~750k
skb_attempt_defer_free() calls; at skb_defer_max=2, 0 before and ~163k
after.
Built for x86_64 and arm64, with and without CONFIG_PREEMPT_RT.
v4: https://lore.kernel.org/all/20261015162209.485940-1-jkain@nuro.ai/
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 | 8 +++++++-
2 files changed, 11 insertions(+), 2 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..643666d9f4c9 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 (unlikely(defer_count > defer_max)) {
+ /* The kept increment only comes back down when a flush zeroes
+ * it, and this is the one path that sees a full queue.
+ */
+ if (!llist_empty(&sdn->defer_list))
+ kick_defer_list_purge(cpu);
goto nodefer;
+ }
llist_add(&skb->ll_node, &sdn->defer_list);
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net v6] net: schedule a purge when the defer queue rejects a free
@ 2026-10-19 17:44 Joseph Kain
2026-10-20 17:44 ` sashiko-bot
0 siblings, 1 reply; 7+ messages in thread
From: Joseph Kain @ 2026-10-19 17:44 UTC (permalink / raw)
To: netdev
Cc: edumazet, davem, kuba, pabeni, horms, kuniyu, kerneljasonxing,
bigeasy, clrkwllms, rostedt, linux-rt-devel, linux-kernel,
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 keeps
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.
Kick the purge from the reject path instead. The reject is the one place
that observes a full queue, so scheduling the flush there is what stops
the counter outliving the list, and it does not depend on the owner CPU
ever raising NET_RX on its own. kick_defer_list_purge() already coalesces
through sd->defer_ipi_scheduled, so a burst of rejects costs one IPI.
This also covers the case where skb_defer_max is lowered below the
current defer_count. The kick condition tests equality with half the
limit, so a reduction can move that point behind the counter and no
later producer ever reaches it; rejecting without a kick then strands a
non-empty list the same way. The reject-path kick recovers both.
While here, test the limit with > rather than >=, so the effective limit
is skb_defer_max rather than skb_defer_max - 1, matching what
Documentation/admin-guide/sysctl/net.rst describes.
That also makes the half-capacity kick reachable at small limits. At
skb_defer_max = 1 nothing was ever queued, so no kick was ever sent, and
at 2 the equality was never hit; both now kick. That is what commit
c09b0cd2cc6c ("net: avoid strange behavior with skb_defer_max == 1") set
out to allow.
Also 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")
Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Joseph Kain <jkain@nuro.ai>
---
v6:
- Keep the increment and kick the purge from the reject path, rather
than testing before incrementing as v4 did. One atomic instead of a
read followed by an RMW, the increment still reserves the slot so the
limit stays exact, and it also recovers the case where skb_defer_max
is lowered below the current defer_count, which v4 did not
(Kuniyuki Iwashima; also reported against v3 and v4 by
netdev-bot sashiko, whose suggested fix this is).
- Describe the two resulting behaviour changes -- the effective limit
and the kick at small limits -- in the commit message rather than
only here, since git am drops this part (netdev-bot sashiko).
Measured on v7.3-rc6 in a VM, 8 cpus, cross-CPU TCP load, sampling
defer_count via bpftrace. Dropping net.core.skb_defer_max from 128 to 1
and then restoring 128:
7b63ef2d55f2 (unpatched) defer_count climbed 701404 -> 3244783 and
stayed at 3244783 after restoring 128
this patch defer_count stayed within 0..1 throughout
and returned to 0
With skb_defer_max=1 and a cross-CPU TCP load, kick_defer_list_purge()
is called 0 times before this patch and ~756k times after, over ~750k
skb_attempt_defer_free() calls; at skb_defer_max=2, 0 before and ~163k
after.
Built for x86_64 and arm64, with and without CONFIG_PREEMPT_RT.
v4: https://lore.kernel.org/all/20261015162209.485940-1-jkain@nuro.ai/
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 | 8 +++++++-
2 files changed, 11 insertions(+), 2 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..643666d9f4c9 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 (unlikely(defer_count > defer_max)) {
+ /* The kept increment only comes back down when a flush zeroes
+ * it, and this is the one path that sees a full queue.
+ */
+ if (!llist_empty(&sdn->defer_list))
+ kick_defer_list_purge(cpu);
goto nodefer;
+ }
llist_add(&skb->ll_node, &sdn->defer_list);
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net v6] net: schedule a purge when the defer queue rejects a free
2026-10-19 17:42 [PATCH net v6] net: schedule a purge when the defer queue rejects a free Joseph Kain
@ 2026-10-19 17:58 ` Eric Dumazet
2026-10-19 18:47 ` Joseph Kain
2026-10-20 20:55 ` Joseph Kain
2026-10-20 17:43 ` sashiko-bot
1 sibling, 2 replies; 7+ messages in thread
From: Eric Dumazet @ 2026-10-19 17:58 UTC (permalink / raw)
To: Joseph Kain
Cc: netdev, davem, kuba, pabeni, horms, kuniyu, kerneljasonxing,
bigeasy, clrkwllms, rostedt, linux-rt-devel, linux-kernel
Le lun. 19 oct. 2026 à 19:43, 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.
> 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 keeps
> 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.
>
> Kick the purge from the reject path instead. The reject is the one place
> that observes a full queue, so scheduling the flush there is what stops
> the counter outliving the list, and it does not depend on the owner CPU
> ever raising NET_RX on its own. kick_defer_list_purge() already coalesces
> through sd->defer_ipi_scheduled, so a burst of rejects costs one IPI.
>
> This also covers the case where skb_defer_max is lowered below the
> current defer_count. The kick condition tests equality with half the
> limit, so a reduction can move that point behind the counter and no
> later producer ever reaches it; rejecting without a kick then strands a
> non-empty list the same way. The reject-path kick recovers both.
>
> While here, test the limit with > rather than >=, so the effective limit
> is skb_defer_max rather than skb_defer_max - 1, matching what
> Documentation/admin-guide/sysctl/net.rst describes.
>
> That also makes the half-capacity kick reachable at small limits. At
> skb_defer_max = 1 nothing was ever queued, so no kick was ever sent, and
> at 2 the equality was never hit; both now kick. That is what commit
> c09b0cd2cc6c ("net: avoid strange behavior with skb_defer_max == 1") set
> out to allow.
>
> Also 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")
> Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Joseph Kain <jkain@nuro.ai>
> ---
Say not to very verbose changelog, that nobody will read.
Since commit 844c9db7f7f5 ("net: use llist for sd->defer_list"),
skb_attempt_defer_free() increments sdn->defer_count before testing
it against skb_defer_max, and keeps the increment when it rejects.
__skb_defer_free_flush() returns early on an empty list without
clearing the counter.
With skb_defer_max == 1, every free is rejected and nothing is ever
queued, so the counter grows without bound and is never reset.
Restoring a larger skb_defer_max does not recover: deferred freeing
stays disabled for that (cpu, node) until reboot.
sysctl -w net.core.skb_defer_max=1
sysctl -w net.core.skb_defer_max=128 # does not recover
Test the limit with '>' instead of '>=', so that the first skb after
a reset is always queued, and a counter above the limit implies a
non-empty list that a flush will clear. This also makes the effective
limit match the documentation, and makes the half-capacity kick
reachable for skb_defer_max of 1 and 2.
When rejecting, kick the owner CPU if the list is not empty, so that
lowering skb_defer_max below the current count cannot leave the list
waiting for a NET_RX on the owner CPU that may never come.
kick_defer_list_purge() coalesces through sd->defer_ipi_scheduled.
Also clear the counter after llist_del_all() rather than before,
as __dev_xmit_skb() does, so a racing producer is under-counted
rather than over-counted.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v6] net: schedule a purge when the defer queue rejects a free
2026-10-19 17:58 ` Eric Dumazet
@ 2026-10-19 18:47 ` Joseph Kain
2026-10-20 20:55 ` Joseph Kain
1 sibling, 0 replies; 7+ messages in thread
From: Joseph Kain @ 2026-10-19 18:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, davem, kuba, pabeni, horms, kuniyu, kerneljasonxing,
bigeasy, clrkwllms, rostedt, linux-rt-devel, linux-kernel
On Mon, Oct 19, 2026 at 10:58 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Say not to very verbose changelog, that nobody will read.
>
> Since commit 844c9db7f7f5 ("net: use llist for sd->defer_list"),
> skb_attempt_defer_free() increments sdn->defer_count before testing
> it against skb_defer_max, and keeps the increment when it rejects.
> __skb_defer_free_flush() returns early on an empty list without
> clearing the counter.
[...]
Thanks, I'll use this changelog in v7.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v6] net: schedule a purge when the defer queue rejects a free
2026-10-19 17:42 [PATCH net v6] net: schedule a purge when the defer queue rejects a free Joseph Kain
2026-10-19 17:58 ` Eric Dumazet
@ 2026-10-20 17:43 ` sashiko-bot
1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-20 17:43 UTC (permalink / raw)
To: Joseph Kain; +Cc: linux-rt-devel, linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] net: skb_attempt_defer_free() still latches when skb_defer_max hits 0
- [Medium] net: repeated IPIs and lock contention on skb_attempt_defer_free() reject path
--
commit 4683493f7039cdaa5b1372dd8cadf3a6b5ffb8f0
Author: Joseph Kain <jkain@nuro.ai>
net: schedule a purge when the defer queue rejects a free
This commit schedules a defer list purge from the reject path in
skb_attempt_defer_free(), changes the limit comparison from >= to >, and
reorders llist_del_all() before resetting defer_count in
__skb_defer_free_flush() so that rejected frees do not leave defer_count
permanently stuck above skb_defer_max.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index e76762e29360e..a5912b728ddd7 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 41beaf625421a..596f99811bacd 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -7392,8 +7392,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 (unlikely(defer_count > defer_max)) {
> + /* The kept increment only comes back down when a flush zeroes
> + * it, and this is the one path that sees a full queue.
> + */
> + if (!llist_empty(&sdn->defer_list))
> + kick_defer_list_purge(cpu);
> goto nodefer;
> + }
[Severity: Medium]
Can sdn->defer_count still permanently latch above defer_max with an empty
sdn->defer_list when net.core.skb_defer_max is set to 0 and later restored to
a positive value?
Both the new reject path in skb_attempt_defer_free() and
__skb_defer_free_flush() rely on sdn->defer_list being non-empty whenever
sdn->defer_count > defer_max:
net/core/skbuff.c:skb_attempt_defer_free() {
...
if (unlikely(defer_count > defer_max)) {
if (!llist_empty(&sdn->defer_list))
kick_defer_list_purge(cpu);
goto nodefer;
}
}
net/core/dev.c:__skb_defer_free_flush() {
if (llist_empty(&sdn->defer_list))
return;
free_list = llist_del_all(&sdn->defer_list);
atomic_long_set(&sdn->defer_count, 0);
...
}
When a producer observes defer_max == 0, even the first increment
(defer_count == 1) satisfies defer_count > defer_max and jumps to nodefer
without adding an skb to sdn->defer_list.
The commit message notes that from v7.1 on, 0 is safe because
proc_do_skb_defer_max() flips skb_defer_disable_key before producers
increment. However, in proc_do_skb_defer_max():
net/core/sysctl_net_core.c:proc_do_skb_defer_max() {
oval = !net_hotdata.sysctl_skb_defer_max;
ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
nval = !net_hotdata.sysctl_skb_defer_max;
if (nval != oval) {
if (nval)
static_branch_enable(&skb_defer_disable_key);
else
static_branch_disable(&skb_defer_disable_key);
}
}
proc_dointvec_minmax() writes 0 to net_hotdata.sysctl_skb_defer_max before
static_branch_enable() acquires the jump_label locks and patches kernel text
across all CPUs. During that window (and deterministically on 6.18 through
7.0 stable trees targeted by Fixes: 844c9db7f7f5, which do not have
skb_defer_disable_key), concurrent callers of skb_attempt_defer_free() pass
the static key check, read defer_max == 0, and increment sdn->defer_count
while rejecting before llist_add().
Once sdn->defer_list is empty and sdn->defer_count has accumulated to a value
at or above the restored net.core.skb_defer_max limit, every subsequent
producer sees defer_count > defer_max and llist_empty(&sdn->defer_list),
skips kick_defer_list_purge(), and even an autonomous __skb_defer_free_flush()
on the owner CPU returns early without zeroing sdn->defer_count.
[Severity: Medium]
Does calling kick_defer_list_purge(cpu) on every rejected skb when
!llist_empty(&sdn->defer_list) cause repeated cross-CPU IPIs and lock
contention during a burst of rejects?
The commit message states:
kick_defer_list_purge() already coalesces through sd->defer_ipi_scheduled,
so a burst of rejects costs one IPI.
In net/core/dev.c, the IPI callback trigger_rx_softirq() runs in hardirq
context and clears sd->defer_ipi_scheduled immediately after raising
NET_RX_SOFTIRQ, before net_rx_action() and skb_defer_free_flush() run in
softirq context to drain sdn->defer_list:
net/core/dev.c:trigger_rx_softirq() {
struct softnet_data *sd = data;
__raise_softirq_irqoff(NET_RX_SOFTIRQ);
smp_store_release(&sd->defer_ipi_scheduled, 0);
}
net/core/dev.c:kick_defer_list_purge() {
struct softnet_data *sd = &per_cpu(softnet_data, cpu);
unsigned long flags;
if (use_backlog_threads()) {
backlog_lock_irq_save(sd, &flags);
if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
__napi_schedule_irqoff(&sd->backlog);
backlog_unlock_irq_restore(sd, flags);
} else if (!cmpxchg(&sd->defer_ipi_scheduled, 0, 1)) {
if (smp_call_function_single_async(cpu, &sd->defer_csd))
WRITE_ONCE(sd->defer_ipi_scheduled, 0);
}
}
Previously, kick_defer_list_purge() was gated on the exact equality
(defer_count - 1) == (defer_max >> 1), so it was invoked at most once per
(cpu, node) per flush cycle.
With this change, whenever the target CPU has hardirqs enabled but has not yet
reached skb_defer_free_flush() (for example, while executing in a
local_bh_disable() section, polling NAPI, or when softirqs are deferred to
ksoftirqd), each IPI immediately clears sd->defer_ipi_scheduled in hardirq
context while sdn->defer_list is still non-empty. Won't subsequent rejected
skbs on remote CPUs then repeatedly win cmpxchg(&sd->defer_ipi_scheduled, 0, 1)
and send back-to-back IPIs, or contend on backlog_lock_irq_save() on every
rejected skb when use_backlog_threads() is enabled?
>
> llist_add(&skb->ll_node, &sdn->defer_list);
>
[ ... ]
完美 — wait no extra text! Let me make sure I don't output anything after the blank line! Let me remove that accidental thought line before finishing! Wait—I am already in the output stream! Oh! Let me be careful. I cannot erase what was emitted, so I will just end with a blank line now.
$]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019174258.1375583-1-jkain@nuro.ai?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v6] net: schedule a purge when the defer queue rejects a free
2026-10-19 17:44 Joseph Kain
@ 2026-10-20 17:44 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-20 17:44 UTC (permalink / raw)
To: Joseph Kain; +Cc: linux-rt-devel, linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] net: skb_attempt_defer_free() still latches defer_count on skb_defer_max=0
- [Medium] net: repeated IPIs and lock contention in skb_attempt_defer_free()
--
commit 6c549c105bac5ff899022b2e84c0969ec08de899
Author: Joseph Kain <jkain@nuro.ai>
net: schedule a purge when the defer queue rejects a free
This commit schedules a defer list purge from the reject path in
skb_attempt_defer_free() when defer_count exceeds defer_max, changes the
limit check from >= to >, and moves the defer_count reset after
llist_del_all() in __skb_defer_free_flush().
> diff --git a/net/core/dev.c b/net/core/dev.c
> index e76762e29360e..a5912b728ddd7 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 41beaf625421a..596f99811bacd 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -7392,8 +7392,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 (unlikely(defer_count > defer_max)) {
> + /* The kept increment only comes back down when a flush zeroes
> + * it, and this is the one path that sees a full queue.
> + */
> + if (!llist_empty(&sdn->defer_list))
> + kick_defer_list_purge(cpu);
> goto nodefer;
> + }
[Severity: Medium]
Can setting net.core.skb_defer_max to 0 and later restoring a positive
limit still latch sdn->defer_count in the absorbing state?
In net/core/sysctl_net_core.c:proc_do_skb_defer_max(),
proc_dointvec_minmax() writes 0 to net_hotdata.sysctl_skb_defer_max
before static_branch_enable(&skb_defer_disable_key) patches the jump
label across CPUs:
oval = !net_hotdata.sysctl_skb_defer_max;
ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
nval = !net_hotdata.sysctl_skb_defer_max;
if (nval != oval) {
if (nval)
static_branch_enable(&skb_defer_disable_key);
else
static_branch_disable(&skb_defer_disable_key);
}
Additionally, preemptible callers in tcp_eat_recv_skb(),
skb_consume_udp(), or mptcp_eat_recv_skb() may already have passed the
skb_defer_disable_key check in skb_attempt_defer_free(), and on 6.18
through 7.0 stable kernels targeted by Fixes: 844c9db7f7f5,
skb_defer_disable_key is not present at all.
When defer_max is 0 and sdn->defer_list is empty (either initially or
after a flush drains existing entries), each producer in
skb_attempt_defer_free() increments sdn->defer_count to 1, 2, ..., N,
sees defer_count > 0, skips kick_defer_list_purge(cpu) because
llist_empty(&sdn->defer_list) is true, and jumps to nodefer without
adding to sdn->defer_list.
Because __skb_defer_free_flush() returns early when
llist_empty(&sdn->defer_list) is true without zeroing sdn->defer_count,
restoring net.core.skb_defer_max to a value <= N leaves sdn->defer_count
at or above defer_max with an empty sdn->defer_list, permanently
disabling deferred freeing for that (cpu, node) pair.
Also, if sdn->defer_list has fewer than (defer_max >> 1) entries queued
when net.core.skb_defer_max is set to 0, enabling skb_defer_disable_key
causes subsequent producers to return before the reject path while
proc_do_skb_defer_max() neither kicks nor flushes pending nodes, which
can strand those queued SKBs if the owner CPU does not raise
NET_RX_SOFTIRQ on its own.
[Severity: Medium]
Does calling kick_defer_list_purge(cpu) on every rejected SKB cause
repeated IPIs and lock contention when the target CPU has BH disabled or
uses backlog threads?
The commit message notes that kick_defer_list_purge() coalesces through
sd->defer_ipi_scheduled so a burst of rejects costs one IPI. However, in
net/core/dev.c:trigger_rx_softirq(), sd->defer_ipi_scheduled is cleared
in hardirq IPI context immediately after raising NET_RX_SOFTIRQ, before
net_rx_action() and skb_defer_free_flush() run:
static void trigger_rx_softirq(void *data)
{
struct softnet_data *sd = data;
__raise_softirq_irqoff(NET_RX_SOFTIRQ);
smp_store_release(&sd->defer_ipi_scheduled, 0);
}
If the target alloc_cpu is running with BH disabled (for example under
bh_lock_sock(), local_bh_disable(), or napi_poll()), or if softirq
processing is deferred to ksoftirqd, hardirqs remain enabled on that
CPU. Each CSD IPI interrupts the target CPU in hardirq context and clears
sd->defer_ipi_scheduled to 0 while sdn->defer_list remains non-empty,
allowing subsequent rejected frees in skb_attempt_defer_free() to send
repeated IPIs until skb_defer_free_flush() actually drains the list.
Furthermore, when use_backlog_threads() is true (such as on
CONFIG_PREEMPT_RT), kick_defer_list_purge() unconditionally acquires
sd->input_pkt_queue.lock via backlog_lock_irq_save() on every rejected
free even when NAPI_STATE_SCHED is already set on sd->backlog.state:
if (use_backlog_threads()) {
backlog_lock_irq_save(sd, &flags);
if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
__napi_schedule_irqoff(&sd->backlog);
backlog_unlock_irq_restore(sd, flags);
}
>
> llist_add(&skb->ll_node, &sdn->defer_list);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019174400.1377093-1-jkain@nuro.ai?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v6] net: schedule a purge when the defer queue rejects a free
2026-10-19 17:58 ` Eric Dumazet
2026-10-19 18:47 ` Joseph Kain
@ 2026-10-20 20:55 ` Joseph Kain
1 sibling, 0 replies; 7+ messages in thread
From: Joseph Kain @ 2026-10-20 20:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, davem, kuba, pabeni, horms, kuniyu, kerneljasonxing,
bigeasy, clrkwllms, rostedt, linux-rt-devel, linux-kernel
On Mon, Oct 19, 2026 at 10:58 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Say not to very verbose changelog, that nobody will read.
>
> Since commit 844c9db7f7f5 ("net: use llist for sd->defer_list"),
> skb_attempt_defer_free() increments sdn->defer_count before testing
> it against skb_defer_max, and keeps the increment when it rejects.
> __skb_defer_free_flush() returns early on an empty list without
> clearing the counter.
>
Review bots found that:
1. skb_defer_max=0 still latches.
proc_do_skb_defer_max() publishes the new value before it calls
static_branch_enable(), so a producer can read skb_defer_max=0,
increment, and reject with an empty list, which skips the kick.
__skb_defer_free_flush() then returns early without clearing. Restoring
any limit at or below the count that accumulated leaves every later
producer rejecting on an empty list. On 6.18 through 7.0, which the
Fixes: tag targets, there is no skb_defer_disable_key at all, so there it
is deterministic rather than a race.
2. The kick does not coalesce the way I said it did.
trigger_rx_softirq() clears sd->defer_ipi_scheduled as soon as the IPI
runs, before the list is drained, so if the owner has BHs disabled the
next rejection sends another IPI. With use_backlog_threads(),
kick_defer_list_purge() also takes sd->input_pkt_queue.lock before it
tests NAPI_STATE_SCHED, and on PREEMPT_RT that is a remote
spin_lock_irqsave() per rejected free. This series started from
sd->defer_lock contention on this same path, so I would rather not ship
that.
For v7 I plan to restore the empty-list clear in the flush which covers 1
(but was dropped in v3). I’ll keep a kick for the case where the
owner never raises NET_RX on its own, but bound it.
The bound is what I am unsure of. Kicking only on the first rejection
after the limit is crossed costs one IPI per overflow, but nothing
retries if that one does not land. Would you prefer that, or something
that keeps retrying?
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-20 20:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 17:42 [PATCH net v6] net: schedule a purge when the defer queue rejects a free Joseph Kain
2026-10-19 17:58 ` Eric Dumazet
2026-10-19 18:47 ` Joseph Kain
2026-10-20 20:55 ` Joseph Kain
2026-10-20 17:43 ` sashiko-bot
-- strict thread matches above, loose matches on Subject: below --
2026-10-19 17:44 Joseph Kain
2026-10-20 17:44 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.