2020-10-10 09:47:56

by Tianxianting

[permalink] [raw]
Subject: [PATCH] bpf: Avoid allocing memory on memoryless numa node

In architecture like powerpc, we can have cpus without any local memory
attached to it. In such cases the node does not have real memory.

Use local_memory_node(), which is guaranteed to have memory.
local_memory_node is a noop in other architectures that does not support
memoryless nodes.

Signed-off-by: Xianting Tian <[email protected]>
---
kernel/bpf/cpumap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 6386b7bb9..2c885c00a 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -423,7 +423,7 @@ __cpu_map_entry_alloc(struct bpf_cpumap_val *value, u32 cpu, int map_id)
struct xdp_bulk_queue *bq;

/* Have map->numa_node, but choose node of redirect target CPU */
- numa = cpu_to_node(cpu);
+ numa = local_memory_node(cpu_to_node(cpu));

rcpu = kzalloc_node(sizeof(*rcpu), gfp, numa);
if (!rcpu)
--
2.17.1


2020-10-12 02:25:04

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH] bpf: Avoid allocing memory on memoryless numa node

On Sat, Oct 10, 2020 at 1:55 AM Xianting Tian <[email protected]> wrote:
>
> In architecture like powerpc, we can have cpus without any local memory
> attached to it. In such cases the node does not have real memory.
>
> Use local_memory_node(), which is guaranteed to have memory.
> local_memory_node is a noop in other architectures that does not support
> memoryless nodes.
...
> /* Have map->numa_node, but choose node of redirect target CPU */
> - numa = cpu_to_node(cpu);
> + numa = local_memory_node(cpu_to_node(cpu));

There are so many calls to cpu_to_node() throughout the kernel.
Are you going to convert all of them one patch at a time to the above sequence?
Why not do this CONFIG_HAVE_MEMORYLESS_NODES
in cpu_to_node() instead?
and save the churn.

2020-10-12 02:25:24

by Tianxianting

[permalink] [raw]
Subject: RE: [PATCH] bpf: Avoid allocing memory on memoryless numa node

Thanks Alexei for your suggestion,
I will try to do it.

-----Original Message-----
From: Alexei Starovoitov [mailto:[email protected]]
Sent: Monday, October 12, 2020 9:21 AM
To: tianxianting (RD) <[email protected]>
Cc: Alexei Starovoitov <[email protected]>; Daniel Borkmann <[email protected]>; David S. Miller <[email protected]>; Jakub Kicinski <[email protected]>; Jesper Dangaard Brouer <[email protected]>; John Fastabend <[email protected]>; Martin KaFai Lau <[email protected]>; Song Liu <[email protected]>; Yonghong Song <[email protected]>; Andrii Nakryiko <[email protected]>; KP Singh <[email protected]>; Network Development <[email protected]>; bpf <[email protected]>; LKML <[email protected]>
Subject: Re: [PATCH] bpf: Avoid allocing memory on memoryless numa node

On Sat, Oct 10, 2020 at 1:55 AM Xianting Tian <[email protected]> wrote:
>
> In architecture like powerpc, we can have cpus without any local
> memory attached to it. In such cases the node does not have real memory.
>
> Use local_memory_node(), which is guaranteed to have memory.
> local_memory_node is a noop in other architectures that does not
> support memoryless nodes.
...
> /* Have map->numa_node, but choose node of redirect target CPU */
> - numa = cpu_to_node(cpu);
> + numa = local_memory_node(cpu_to_node(cpu));

There are so many calls to cpu_to_node() throughout the kernel.
Are you going to convert all of them one patch at a time to the above sequence?
Why not do this CONFIG_HAVE_MEMORYLESS_NODES in cpu_to_node() instead?
and save the churn.