On Tue, Sep 01, 2026, Shivank Garg wrote:
> guest_memfd_test assumes that nodes 0 and 1 exist and have memory.
> is_multi_numa_node_system() only checks that the maximum node ID is
> nonzero, which is not enough for sparse or memoryless nodes.
>
> Select the required nodes from MPOL_F_MEMS_ALLOWED instead. Use the full
> nodemask width plus one to mbind(), and let test_mbind() run when only
> one memory node is available.
Please split this into at least three patches.
1. Refactor guest_memfd_test.c to prepare for using nodes other than 0 and 1.
2. Fix test_mbind().
3. Fix test_numa_allocation().
4. If necessary, do additional cleanups in numaif.h
As is, this is extremely difficult to review, e.g. without staring intently, I
can't tell what's refactoring and what's actually a functional change.
Actually, looking at the xAPIC IPI test more, what you proposed in patch 3 is in
the general direction of what we want, but needs to be more than just a wrapper
for get_mempolicy() to be useful. Specificaly, if it fills the mask *and*
returns
the number of nodes found, then it's more generically useful. And if we also
add
an API to get the next (exclusive) node, then we can cut down on the amount of
copy+paste without forcing tests to use the "array of one-bit nodemasks"
approach
that the xAPIC test uses.
E.g. the below get_numa_node_ids() is basically just copy+paste from the xAPIC
test, except that it returns an array of nodes instead of an array of nodemasks.
The fact that you felt compelled to copy+paste instead of adding an API is quite
telling: using an array of nodemasks/nodes is inflexible and really only works
if
a test wants to target exactly one node. It's also annoying to extract to a
generic
API without ending up with a brittle API. E.g. if the API where to take the a
mask
and an array, it would either have to be a macro or take a struct to ensure the
array can hold all possible masks.
I'm planning on adding these in the series to also add MAXNODE_FOR_MASK().
static inline int kvm_get_numa_memory_nodes(unsigned long *nodemask)
{
int r;
*nodemask = 0;
r = get_mempolicy(NULL, nodemask, MAXNODE_FOR_MASK(*nodemask), 0,
MPOL_F_MEMS_ALLOWED);
TEST_ASSERT(!r || errno == ENOSYS || errno == EPERM,
"Unexpected get_mempolicy() failure");
return __builtin_popcountl(*nodemask);
}
/*
* Return the node ID of the next NUMA node in the mask, starting at @from+1.
* Guarantees a node is found, and that the found node is not @from. Pass -1
* to find the first node in the mask.
*/
static inline int kvm_get_next_numa_node(unsigned long nodemask, int from)
{
const unsigned long nr_bits = BITS_PER_TYPE(nodemask);
int to;
to = find_next_bit(&nodemask, nr_bits, from + 1);
if (to == nr_bits)
to = find_next_bit(&nodemask, nr_bits, 0);
TEST_ASSERT(to != nr_bits && to != from,
"Unabled to find second NUMA node (from = %d, to = %d)",
from, to);
return to;
}
> The sysfs helpers for finding maxnode are no longer needed.
This is an observation, not a proper changelog sentence.
> Signed-off-by: Shivank Garg <[email protected]>
> ---
> tools/testing/selftests/kvm/guest_memfd_test.c | 86
> +++++++++++++++++---------
> tools/testing/selftests/kvm/include/numaif.h | 52 ----------------
> 2 files changed, 58 insertions(+), 80 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c
> b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 2233d871a38f..aee80dda6229 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -76,33 +76,53 @@ static void test_mmap_supported(int fd, size_t total_size)
> kvm_munmap(mem, total_size);
> }
>
> +/*
> + * Fill @nids with the first @nr_nids nodes in the allowed mask.
> + * Return false if the mask contains fewer than @nr_nids nodes.
> + */
> +static bool get_numa_node_ids(int *nids, int nr_nids)
> +{
> + unsigned long nodemask = get_numa_mem_nodes();
> + unsigned long nid;
> + int nr_found = 0;
> +
> + for_each_set_bit(nid, &nodemask, BITS_PER_TYPE(nodemask)) {
> + nids[nr_found++] = nid;
> + if (nr_found == nr_nids)
> + return true;
> + }
> +
> + return false;
> +}
> +
> static void test_mbind(int fd, size_t total_size)
> {
> - const unsigned long nodemask_0 = 1; /* nid: 0 */
> - unsigned long nodemask = 0;
> - unsigned long maxnode = BITS_PER_TYPE(nodemask);
> + unsigned long nodemask, bind_nodemask;
> + unsigned long maxnode = BITS_PER_TYPE(nodemask) + 1;
> int policy;
> char *mem;
> + int nid;
> int ret;
>
> - if (!is_multi_numa_node_system())
> + if (!get_numa_node_ids(&nid, 1))
This is not functionally equivalent. The existing test requires multiple NUMA
nodes, whereas this will now succeed if there's exactly one node. That could be
totally fine, but it needs to be isolated and explained in its own patch.
