Re: [PATCH 2/2] selftests/bpf: Select NUMA node of current CPU to create map

[Saket Kumar Bhaskar <skb99@xxxxxxxxxxxxx>]

On Sat, Jan 25, 2025 at 09:02:37AM -0800, Alexei Starovoitov wrote:
> On Sat, Jan 25, 2025 at 7:25 AM Saket Kumar Bhaskar <skb99@xxxxxxxxxxxxx> wrote:
> >
> > On powerpc, a CPU does not necessarily originate from NUMA node 0.
> > This contrasts with architectures like x86, where CPU 0 is not
> > hot-pluggable, making NUMA node 0 a consistently valid node.
> > This discrepancy can lead to failures when creating a map on NUMA
> > node 0, which is initialized by default, if no CPUs are allocated
> > from NUMA node 0.
> >
> > This patch fixes the issue by setting NUMA node for map creation
> > to NUMA node of the current CPU.
> >
> > Fixes: 96eabe7a40aa ("bpf: Allow selecting numa node during map creation")
> > Signed-off-by: Saket Kumar Bhaskar <skb99@xxxxxxxxxxxxx>
> > ---
> >  tools/testing/selftests/bpf/Makefile                      | 2 +-
> >  tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 0a016cd71..c7a996f53 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -47,7 +47,7 @@ CFLAGS += -g $(OPT_FLAGS) -rdynamic                                   \
> >           -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR)          \
> >           -I$(TOOLSINCDIR) -I$(TOOLSARCHINCDIR) -I$(APIDIR) -I$(OUTPUT)
> >  LDFLAGS += $(SAN_LDFLAGS)
> > -LDLIBS += $(LIBELF_LIBS) -lz -lrt -lpthread
> > +LDLIBS += $(LIBELF_LIBS) -lz -lrt -lpthread -lnuma
> >
> >  PCAP_CFLAGS    := $(shell $(PKG_CONFIG) --cflags libpcap 2>/dev/null && echo "-DTRAFFIC_MONITOR=1")
> >  PCAP_LIBS      := $(shell $(PKG_CONFIG) --libs libpcap 2>/dev/null)
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> > index cc184e442..d241d22b8 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> > @@ -4,6 +4,7 @@
> >  #include <sys/syscall.h>
> >  #include <limits.h>
> >  #include <test_progs.h>
> > +#include <numa.h>
> >  #include "bloom_filter_map.skel.h"
> >
> >  static void test_fail_cases(void)
> > @@ -69,6 +70,7 @@ static void test_success_cases(void)
> >
> >         /* Create a map */
> >         opts.map_flags = BPF_F_ZERO_SEED | BPF_F_NUMA_NODE;
> > +       opts.numa_node = numa_node_of_cpu(sched_getcpu()); // Get the NUMA node of the current CPU
> 
> let's not introduce new library deps.
> Will NUMA_NO_NODE work ?
> 
Yes this change worked:

diff --git a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
index d241d22b8..527825939 100644
--- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
@@ -4,9 +4,12 @@
 #include <sys/syscall.h>
 #include <limits.h>
 #include <test_progs.h>
-#include <numa.h>
 #include "bloom_filter_map.skel.h"
 
+#ifndef NUMA_NO_NODE
+#define        NUMA_NO_NODE    (-1)
+#endif
+
 static void test_fail_cases(void)
 {
        LIBBPF_OPTS(bpf_map_create_opts, opts);
@@ -70,7 +73,7 @@ static void test_success_cases(void)
 
        /* Create a map */
        opts.map_flags = BPF_F_ZERO_SEED | BPF_F_NUMA_NODE;
-       opts.numa_node = numa_node_of_cpu(sched_getcpu()); // Get the NUMA node of the current CPU
+       opts.numa_node = NUMA_NO_NODE;
        fd = bpf_map_create(BPF_MAP_TYPE_BLOOM_FILTER, NULL, 0, sizeof(value), 100, &opts);
        if (!ASSERT_GE(fd, 0, "bpf_map_create bloom filter success case"))
                return;

I will send out v2.
> Note c++ comments are not allowed.
> 
Acknowledged..

Thanks,
Saket
> pw-bot: cr