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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.06.2026 17:43, Roger Pau Monne wrote:
> --- /dev/null
> +++ b/tools/tests/numa/.gitignore
> @@ -0,0 +1,2 @@
> +/numa.h
> +/test-numa

Why the leading slashes?

> --- /dev/null
> +++ b/tools/tests/numa/Makefile
> @@ -0,0 +1,47 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +TARGETS := test-numa
> +
> +.PHONY: all
> +all: $(TARGETS)
> +
> +.PHONY: run
> +run: $(TARGETS)
> +ifeq ($(CC),$(HOSTCC))
> +     set -e;             \
> +     for test in $? ; do \
> +             ./$$test ;  \
> +     done
> +else
> +     $(warning HOSTCC != CC, will not run test)
> +endif
> +
> +.PHONY: clean
> +clean:
> +     $(RM) -- *.o $(TARGETS) $(DEPS_RM) numa.h
> +
> +.PHONY: distclean
> +distclean: clean
> +     $(RM) -- *~

I see we remove *~ elsewhere, but not everywhere. I don't, however, know
why we have that, and hence I wonder whether it really wants replicating.

> --- /dev/null
> +++ b/tools/tests/numa/harness.h
> @@ -0,0 +1,184 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Unit tests for NUMA setup.
> + *
> + * Copyright (C) 2026 Cloud Software Group
> + */
> +
> +#ifndef _TEST_HARNESS_
> +#define _TEST_HARNESS_
> +
> +#include <assert.h>
> +#include <errno.h>
> +#include <inttypes.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <xen-tools/bitops.h>
> +#include <xen-tools/common-macros.h>
> +
> +#define CONFIG_DEBUG
> +#define CONFIG_NUMA
> +#define CONFIG_NR_NUMA_NODES 64
> +#define NR_CPUS 256
> +#define MAX_RANGES 128
> +#define PADDR_BITS 52
> +
> +#define __init
> +#define __initdata
> +#define __ro_after_init
> +#define __read_mostly
> +
> +#define printk printf
> +#define XENLOG_INFO ""
> +#define XENLOG_DEBUG ""
> +#define XENLOG_WARNING ""
> +#define KERN_INFO ""
> +#define KERN_ERR ""
> +#define KERN_WARNING ""
> +#define KERN_DEBUG ""
> +
> +#define PAGE_SHIFT    12
> +/* Some libcs define PAGE_SIZE in limits.h. */
> +#undef  PAGE_SIZE
> +#define PAGE_SIZE     (1L << PAGE_SHIFT)
> +#define MAX_ORDER     18 /* 2 * PAGETABLE_ORDER (9) */
> +
> +#define PFN_DOWN(x)   ((x) >> PAGE_SHIFT)
> +#define PFN_UP(x)     (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
> +
> +#define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
> +#define mfn_to_pdx(mfn)   (mfn)
> +#define paddr_to_pdx(pa)  ((pa) >> PAGE_SHIFT)
> +#define mfn_to_maddr(mfn) ((mfn) << PAGE_SHIFT)
> +
> +#define ASSERT assert
> +#define ASSERT_UNREACHABLE() assert(0)
> +
> +/* For the purposes of the testing assume arch NID == Xen NID. */
> +#define numa_node_to_arch_nid(n) (n)
> +
> +typedef uint64_t paddr_t;
> +#define PRIpaddr "016" PRIx64
> +
> +typedef unsigned long mfn_t;
> +typedef uint8_t nodeid_t;
> +
> +#define __set_bit set_bit
> +#define __clear_bit clear_bit
> +
> +static inline unsigned int find_next_bit(
> +    const unsigned long *addr, unsigned int size, unsigned int off)
> +{
> +    unsigned int i;
> +
> +    ASSERT(size <= BITS_PER_LONG);
> +
> +    for ( i = off; i < size; i++ )
> +        if ( !!(*addr & (1UL << i)) )

Why the !! ?

> +            return i;
> +
> +    return size;
> +}
> +
> +#define find_first_bit(b, s) find_next_bit(b, s, 0)
> +
> +/* Minimal cpumask support. */
> +typedef struct cpumask{ DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
> +
> +#define cpumask_clear_cpu(c, m) clear_bit((c), (m)->bits)
> +
> +/* Define the nodemask helpers used. */
> +typedef struct nodemask{ DECLARE_BITMAP(bits, CONFIG_NR_NUMA_NODES); } 
> nodemask_t;
> +
> +#define node_set(node, dst) set_bit((node), (dst).bits)

To aid readability, omit the parentheses around "node"? (More similar cases
further down.)

Jan