Re: [PATCH RFC v2 4/4] KVM: selftests: exercise guest_memfd folio migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardcoding of NUMA node IDs 0 and 1 makes the test brittle and prone to false failures on systems with sparse, non-contiguous, or memoryless nodes.
--

--- Patch [4]: [PATCH RFC v2 4/4] KVM: selftests: exercise guest_memfd folio migration ---
commit 3681825fc67afc2ab3cf2b50c6cec1297cae94b5
Author: Shivank Garg <shivankg@amd.com>

KVM: selftests: exercise guest_memfd folio migration

This commit adds a migration test to guest_memfd_test to verify that
guest_memfd folios can be migrated across NUMA nodes using move_pages().
It allocates folios on node 0, migrates them to node 1, verifies the
contents, and then migrates them back.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
[ ... ]
> +static void test_migrate_folio(int fd, size_t total_size)
> +{
> +	const unsigned long nodemask_0 = 1; /* nid: 0 */
> +	unsigned long maxnode = BITS_PER_TYPE(nodemask_0);
> +	int page_count = total_size / page_size;
> +	int *status, *nodes;
> +	void **addr;
> +	char *mem;
> +	int i;
> +
> +	if (!is_multi_numa_node_system())
> +		return;
> +
> +	mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +
> +	addr = calloc(page_count, sizeof(*addr));
> +	status = calloc(page_count, sizeof(*status));
> +	nodes = calloc(page_count, sizeof(*nodes));
> +	TEST_ASSERT(addr && status && nodes, "Failed to allocate page arrays");
> +
> +	/* Allocate all folios on node 0 and fill each with a known pattern. */
> +	kvm_mbind(mem, total_size, MPOL_BIND, &nodemask_0, maxnode, 0);
> +	for (i = 0; i < page_count; i++) {
> +		memset(mem + i * page_size, (char)(i & 0xff), page_size);
> +		addr[i] = mem + i * page_size;
> +	}
> +
> +	kvm_move_pages(0, page_count, addr, NULL, status, 0);
> +	for (i = 0; i < page_count; i++)
> +		TEST_ASSERT(status[i] == 0, "Page %d should be on node 0", i);
> +
> +	/* Migrate node 0 -> 1, then check both the location and the data. */
> +	for (i = 0; i < page_count; i++)
> +		nodes[i] = 1;

[Severity: Medium]
Could this hardcoding of NUMA node IDs cause false positive test failures?

If a system has sparse, non-contiguous, or memoryless nodes (for example,
nodes 0 and 2 exist, but node 1 is absent), is_multi_numa_node_system()
will return true, but the initial allocation on node 0 or the migration to
node 1 could fail.

Would it be better to dynamically discover available memory nodes in
test_migrate_folio() instead of assuming nodes 0 and 1 always exist and
have memory available?

> +	kvm_move_pages(0, page_count, addr, nodes, status, MPOL_MF_MOVE);
> +
> +	kvm_move_pages(0, page_count, addr, NULL, status, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-shivank-gmem-migrate-v2-0-269ac1f84e2b@amd.com?part=4