Re: [PATCH for-4.22 v2 4/5] tests/numa: add unit tests for NUMA setup logic

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jun 05, 2026 at 05:10:23PM +0200, Anthony PERARD wrote:
> On Wed, Jun 03, 2026 at 09:18:35PM +0200, Roger Pau Monne wrote:
> > diff --git a/tools/tests/numa/Makefile b/tools/tests/numa/Makefile
> > new file mode 100644
> > index 000000000000..5235f9d6297f
> > --- /dev/null
> > +++ b/tools/tests/numa/Makefile
> > +
> > +.PHONY: uninstall
> > +uninstall:
> > +   $(RM) -- $(patsubst %,$(DESTDIR)$(LIBEXEC)/tests/%,$(TARGETS))
> 
> There's a simpler way to write this, with
>     $(addprefix $(DESTDIR)$(LIBEXEC)/tests/,$(TARGETS))
> But that's ok to.

I've merely c&p what we had in other Makefiles, and didn't think much
about the specific runes.  I will leave that one alone if you don't
mind, we could always do a sweep change of this pattern in
tools/tests.

> > +
> > +numa.h: $(XEN_ROOT)/xen/include/xen/numa.h
> > +   sed -e '/^#[[:space:]]*include/d' <$< >$@
> > +
> > +CFLAGS += -D__XEN_TOOLS__
> > +CFLAGS += $(APPEND_CFLAGS)
> > +CFLAGS += $(CFLAGS_xeninclude)
> > +
> > +test-numa: test-numa.c numa.h
> > +   $(CC) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_$*.o) -o $@ $< $(APPEND_CFLAGS)
> 
> $* should be undefined here.
> So we have $(CFLAGS_.o), but that variable doesn't exit either.
> You could remove $(CFLAGS_$*.o), it's not use here.

Ack.

> Also, $(APPEND_CFLAGS) is added twice, once via $(CFLAGS) and a second
> time on the command line. I think the one added to $(CFLAGS) should be
> removed.

Done, will remove that from CLFAGS then.

> > diff --git a/tools/tests/numa/test-numa.c b/tools/tests/numa/test-numa.c
> > new file mode 100644
> > index 000000000000..bced68d4d7f1
> > --- /dev/null
> > +++ b/tools/tests/numa/test-numa.c
> > +
> > +        for ( j = 0;
> > +              j < ARRAY_SIZE(tests[i].affinity) && 
> > tests[i].affinity[j].end;
> 
> Why do you test the value `.end` ? ARRAY_SIZE is likely enough as the
> test array is static. Same thing later, with the `ram` array.

To skip processing empty/uninitialized entries (ie: ones with start
== end == 0).

> 
> 
> Anyway, it's all look good enough to me:
> Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks, will do the above changes and do one last pre-push CI run.

Roger.