[PATCH net-next] net: iterate online nodes in skb_defer_free_flush()

[PATCH net-next] net: iterate online nodes in skb_defer_free_flush()

[Kris Pan]

skb_attempt_defer_free() only queues skbs on the current CPU's node
(numa_node_id() of a running CPU), which is always online, so the
flush loop never needs to visit nodes that are merely possible.

for_each_node() walks node_possible_map.  On machines where the
possible map is much larger than the online map -- e.g. a POWER10
LPAR with 32 possible but 1 online node -- the flush loop touches 31
cold, always-empty per-node lists on every softirq pass.

Use for_each_online_node() to iterate only node_online_map.  Nodes
that go offline are drained by dev_cpu_dead() ("net: flush
skb_defer_nodes in dev_cpu_dead()"), so the flush no longer needs to
visit offline nodes.

Loopback UDP throughput in a QEMU guest with 32 possible / 1 online
nodes (bench_udp, 8 senders, interleaved runs) improves by ~5%.

Fixes: 5628f3fe3b16 ("net: add NUMA awareness to skb_attempt_defer_free()")
Reported-by: kernel test robot <oliver.sang@intel.com>
Suggested-by: Adrian Tomasov <atomasov@redhat.com>
Closes: https://lore.kernel.org/oe-lkp/202512112119.5b9829a-lkp@intel.com
Signed-off-by: Kris Pan <kris.pan@intel.com>
---
 net/core/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index f660fccfc0dbc..807640155e649 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6927,7 +6927,7 @@ static void skb_defer_free_flush(void)
 	struct skb_defer_node *sdn;
 	int node;
 
-	for_each_node(node) {
+	for_each_online_node(node) {
 		sdn = this_cpu_ptr(net_hotdata.skb_defer_nodes) + node;
 		__skb_defer_free_flush(sdn, 1);
 	}
-- 
2.43.0

Re: [PATCH net-next] net: iterate online nodes in skb_defer_free_flush()

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · 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).

- [Medium] The patch changes skb_defer_free_flush() (net/core/dev.c) to
  use for_each_online_node().

--

net: iterate online nodes in skb_defer_free_flush()

This patch changes skb_defer_free_flush() to walk node_online_map instead
of node_possible_map. The aim is to skip per-node defer lists that are
always empty on systems with many possible nodes but few online ones,
such as POWER10 LPARs.

Closes: https://lore.kernel.org/oe-lkp/202512112119.5b9829a-lkp@intel.com

> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0dbc..807640155e649 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6927,7 +6927,7 @@ static void skb_defer_free_flush(void)
>  	struct skb_defer_node *sdn;
>  	int node;
>  
> -	for_each_node(node) {
> +	for_each_online_node(node) {
>  		sdn = this_cpu_ptr(net_hotdata.skb_defer_nodes) + node;
>  		__skb_defer_free_flush(sdn, 1);
>  	}

[Severity: Medium]

Is cpu_to_node() of a running CPU always in node_online_map? The commit
message says:

  skb_attempt_defer_free() only queues skbs on the current CPU's node
  (numa_node_id() of a running CPU), which is always online, ...

skb_attempt_defer_free() chooses the list without checking whether the
node is online:

net/core/skbuff.c:skb_attempt_defer_free() {
    ...
	sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + cpu_to_node(my_cpu);
    ...
}

The only place that brings a CPU's node online is cpu_up():

kernel/cpu.c:cpu_up() {
    ...
	err = try_online_node(cpu_to_node(cpu));
    ...
}

With CONFIG_MEMORY_HOTPLUG=n, try_online_node() is a stub that does
nothing:

include/linux/memory_hotplug.h:
static inline int try_online_node(int nid)
{
	return 0;
}

Consider a pseries LPAR with CONFIG_NUMA=y and CONFIG_MEMORY_HOTPLUG=n,
using the same 32 possible / 1 online layout this patch targets. A DLPAR
CPU add goes through dlpar_online_cpu()->find_and_update_cpu_nid(). That
function only checks node_possible() and relies on cpu_up() to bring the
node online:

arch/powerpc/mm/numa.c:find_and_update_cpu_nid() {
    ...
	if (new_nid < 0 || !node_possible(new_nid))
		new_nid = first_online_node;
	else
		// Associate node <-> cpu, so cpu_up() calls
		// try_online_node() on the right node.
		set_cpu_numa_node(cpu, new_nid);
    ...
}

The CPU can then come online with cpu_to_node() == N while N stays
offline. NODE_DATA and zonelists exist for every possible node, so that
CPU runs normally.

When that CPU defer-frees skbs for a remote alloc_cpu, they go into
per_cpu(skb_defer_nodes, alloc_cpu)[N]. With this change, will
skb_defer_free_flush() on alloc_cpu ever visit index N?

Only __skb_defer_free_flush() resets defer_count. So it looks like up to
sysctl_skb_defer_max - 1 skbs per alloc_cpu (128 is the default max) stay
stranded, along with their page frags and page_pool pages. The only
remaining drain is dev_cpu_dead():

net/core/dev.c:dev_cpu_dead() {
    ...
	for_each_node(node)
		skb_defer_node_flush(per_cpu_ptr(net_hotdata.skb_defer_nodes,
						 oldcpu) + node);
    ...
}

That only runs when a CPU goes offline. Could page_pool_destroy() then
stall at device teardown, the same failure described in commit
06e3f54e8b22 ("net: flush skb_defer_nodes in dev_cpu_dead()")?

Would it be safer to keep for_each_node() here? Another option would be
to track the nodes that have online CPUs instead of relying on
node_online_map.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928014708.3903028-1-kris.pan%40intel.com