Re: [PATCH v2 2/3] selftests/mm: fix ksm NUMA merge test for systems with memoryless NUMA nodes

[Sayali Patil <sayalip@linux.ibm.com>]

On 30/06/26 15:02, Sayali Patil wrote:
> The KSM NUMA merge test allocates identical pages on different NUMA
> nodes and verifies KSM behavior with merge_across_nodes enabled and
> disabled.
> 
> On systems with memoryless NUMA nodes, for example:
>   #numactl  -H
>        available: 2 nodes (0,4)
>        .....
>        node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
>        node 0 size: 14825 MB
>        node 0 free: 1382 MB
>        node 4 cpus:
>        node 4 size: 0 MB
>        node 4 free: 0 MB
> 
> the test may attempt to allocate memory on a node without memory,
> causing numa_alloc_onnode() to fail and resulting in a spurious test
> failure.
> 
> The test currently checks numa_num_configured_nodes() to determine
> whether sufficient NUMA nodes are available. However, configured nodes
> do not necessarily have memory.
> 
> Reuse the existing get_first_mem_node() and get_next_mem_node()
> helpers to locate NUMA nodes that actually contain memory, and skip
> the test when fewer than two such nodes are available.
> 
> Before patch:
>         ---------------------------
> 	running ./ksm_tests -N -m 1
>         ---------------------------
>          mbind: Invalid argument
>          ok 1 KSM NUMA merging
> 	Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
>          [PASS]
>         ok 1 ksm_tests -N -m 1
>         ---------------------------
>          running ./ksm_tests -N -m 0
>         ---------------------------
>          mbind: Invalid argument
>          not ok 1 KSM NUMA merging
> 	Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
>          [FAIL]
>         not ok 2 ksm_tests -N -m 0 # exit=1
> 
> After patch:
>         ---------------------------
>          running ./ksm_tests -N -m 1
>         ---------------------------
>          At least 2 NUMA nodes with memory must be available
> 	ok 1
> 	SKIP KSM NUMA merging
> 	Totals: pass:0 fail:0 xfail:0 xpass:0 skip:1 error:0
>          [PASS]
>          ok 1 ksm_tests -N -m 1
>         ---------------------------
>          running ./ksm_tests -N -m 0
>         ---------------------------
>          At least 2 NUMA nodes with memory must be available
> 	ok 1
> 	SKIP KSM NUMA merging
> 	Totals: pass:0 fail:0 xfail:0 xpass:0 skip:1 error:0
>          [PASS]
>          ok 2 ksm_tests -N -m 0
> 
> Fixes: e3820ab252dd ("selftest/vm: fix ksm selftest to run with different NUMA topologies")
> Co-developed-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Sayali Patil <sayalip@linux.ibm.com>
> ---
>   tools/testing/selftests/mm/ksm_tests.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/ksm_tests.c b/tools/testing/selftests/mm/ksm_tests.c
> index a050f4840cfa..2ebbb544c671 100644
> --- a/tools/testing/selftests/mm/ksm_tests.c
> +++ b/tools/testing/selftests/mm/ksm_tests.c
> @@ -440,9 +440,9 @@ static int get_next_mem_node(int node)
>   		mem_node = i % (max_node + 1);
>   		node_size = numa_node_size(mem_node, NULL);
>   		if (node_size > 0)
> -			break;
> +			return mem_node;
>   	}
> -	return mem_node;
> +	return -ENODEV;
>   }
>   
>   static int get_first_mem_node(void)
> @@ -455,8 +455,8 @@ static int check_ksm_numa_merge(int merge_type, int mapping, int prot, int timeo
>   {
>   	void *numa1_map_ptr, *numa2_map_ptr;
>   	struct timespec start_time;
> +	int first_node, second_node;
>   	int page_count = 2;
> -	int first_node;
>   
>   	if (clock_gettime(CLOCK_MONOTONIC_RAW, &start_time)) {
>   		ksft_perror("clock_gettime");
> @@ -467,17 +467,19 @@ static int check_ksm_numa_merge(int merge_type, int mapping, int prot, int timeo
>   		ksft_print_msg("NUMA support not enabled\n");
>   		return KSFT_SKIP;
>   	}
> -	if (numa_num_configured_nodes() <= 1) {
> -		ksft_print_msg("At least 2 NUMA nodes must be available\n");
> +	first_node = get_first_mem_node();
> +	second_node = get_next_mem_node(first_node);
> +
> +	if (second_node < 0) {
> +		ksft_print_msg("At least 2 NUMA nodes with memory must be available\n");
>   		return KSFT_SKIP;
>   	}
>   	if (ksm_write_sysfs(KSM_FP("merge_across_nodes"), merge_across_nodes))
>   		return KSFT_FAIL;
>   
>   	/* allocate 2 pages in 2 different NUMA nodes and fill them with the same data */
> -	first_node = get_first_mem_node();
>   	numa1_map_ptr = numa_alloc_onnode(page_size, first_node);
> -	numa2_map_ptr = numa_alloc_onnode(page_size, get_next_mem_node(first_node));
> +	numa2_map_ptr = numa_alloc_onnode(page_size, second_node);
>   	if (!numa1_map_ptr || !numa2_map_ptr) {
>   		ksft_perror("numa_alloc_onnode");
>   		return KSFT_FAIL;

AI review comment:
> If get_first_mem_node() doesn't find a node with memory, will it
> return -ENODEV and pass it directly into get_next_mem_node()?
> Looking at get_next_mem_node() with a negative node parameter:
> tools/testing/selftests/mm/ksm_tests.c:get_next_mem_node() {
>    ...
>    for (i = node + 1; i <= max_node + node; i++) {
>        mem_node = i % (max_node + 1);
>        node_size = numa_node_size(mem_node, NULL);
>    ...
> }
> Because the modulo operator preserves the sign of a negative dividend,
> starting the loop with a negative node value causes mem_node to become
> negative. This means a negative node ID is then passed into libnuma's
> numa_node_size() function.
> Could we check if first_node is valid before attempting to find the
> second node?

The additional check for get_first_mem_node() is not needed. After 
numa_available() succeeds, a running system should always have at least 
one NUMA node with memory, so get_first_mem_node() is expected to return 
a valid node ID. The actual prerequisite for this test is the presence 
of two NUMA nodes with memory, which is already validated by checking 
the return value of get_next_mem_node().

Thanks,
Sayali