On 8/4/25 2:41 PM, Wei Yang wrote:
On Tue, Jul 29, 2025 at 11:03:59AM +0530, Aboorva Devarajan wrote:
From: Donet Tom <donet...@linux.ibm.com>

This patch fixed 2 issues.

1) After fork() in test_prctl_fork, the child process uses the file
descriptors from the parent process to read ksm_stat and
ksm_merging_pages. This results in incorrect values being read (parent
process ksm_stat and ksm_merging_pages will be read in child), causing
the test to fail.

This patch calls init_global_file_handles() in the child process to
ensure that the current process's file descriptors are used to read
ksm_stat and ksm_merging_pages.

2) All tests currently call ksm_merge to trigger page merging.
To ensure the system remains in a consistent state for subsequent
tests, it is better to call ksm_unmerge during the test cleanup phase.

In the test_prctl_fork test, after a fork(), reading ksm_merging_pages
in the child process returns a non-zero value because a previous test
performed a merge, and the child's memory state is inherited from the
parent.

Although the child process calls ksm_unmerge, the ksm_merging_pages
counter in the parent is reset to zero, while the child's counter
remains unchanged. This discrepancy causes the test to fail.

To avoid this issue, each test should call ksm_unmerge during cleanup
to ensure the counter is reset and the system is in a clean state for
subsequent tests.

execv argument is an array of pointers to null-terminated strings.
In this patch we also added NULL in the execv argument.

Fixes: 6c47de3be3a0 ("selftest/mm: ksm_functional_tests: extend test case for ksm 
fork/exec")
Co-developed-by: Aboorva Devarajan <aboor...@linux.ibm.com>
Signed-off-by: Aboorva Devarajan <aboor...@linux.ibm.com>
Signed-off-by: Donet Tom <donet...@linux.ibm.com>
---
tools/testing/selftests/mm/ksm_functional_tests.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c 
b/tools/testing/selftests/mm/ksm_functional_tests.c
index d8bd1911dfc0..996dc6645570 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -46,6 +46,8 @@ static int ksm_use_zero_pages_fd;
static int pagemap_fd;
static size_t pagesize;

+static void init_global_file_handles(void);
+
static bool range_maps_duplicates(char *addr, unsigned long size)
{
        unsigned long offs_a, offs_b, pfn_a, pfn_b;
@@ -274,6 +276,7 @@ static void test_unmerge(void)
        ksft_test_result(!range_maps_duplicates(map, size),
                         "Pages were unmerged\n");
unmap:
+       ksm_unmerge();
In __mmap_and_merge_range(), we call ksm_unmerge(). Why this one not help?

Not very familiar with ksm stuff. Would you mind giving more on how this fix
the failure you see?


The issue I was facing here was test_prctl_fork was failing.

# [RUN] test_prctl_fork
# Still pages merged
#

This issue occurred because the previous test performed a merge, causing
the value of /proc/self/ksm_merging_pages to reflect the number of
deduplicated pages. After that, a fork() was called. Post-fork, the child process
inherited the parent's ksm_merging_pages value.

Then, the child process invoked __mmap_and_merge_range(), which resulted
in unmerging the pages and resetting the value. However, since the parent process
had performed the merge, its ksm_merging_pages value also got reset to 0.
Meanwhile, the child process had not performed any merge itself, so the inherited
value remained unchanged. That’s why get_my_merging_page() in the child was
returning a non-zero value.

Initially, I fixed the issue by calling ksm_unmerge() before the fork(), and that
resolved the problem. Later, I decided it would be cleaner to move the
ksm_unmerge() call to the test cleanup phase.



        munmap(map, size);
}

@@ -338,6 +341,7 @@ static void test_unmerge_zero_pages(void)
        ksft_test_result(!range_maps_duplicates(map, size),
                        "KSM zero pages were unmerged\n");
unmap:
+       ksm_unmerge();
        munmap(map, size);
}

@@ -366,6 +370,7 @@ static void test_unmerge_discarded(void)
        ksft_test_result(!range_maps_duplicates(map, size),
                         "Pages were unmerged\n");
unmap:
+       ksm_unmerge();
        munmap(map, size);
}

@@ -452,6 +457,7 @@ static void test_unmerge_uffd_wp(void)
close_uffd:
        close(uffd);
unmap:
+       ksm_unmerge();
        munmap(map, size);
}
#endif
@@ -515,6 +521,7 @@ static int test_child_ksm(void)
        else if (map == MAP_MERGE_SKIP)
                return -3;

+       ksm_unmerge();
        munmap(map, size);
        return 0;
}
@@ -548,6 +555,7 @@ static void test_prctl_fork(void)

        child_pid = fork();
        if (!child_pid) {
+               init_global_file_handles();
Would this leave fd in parent as orphan?

                exit(test_child_ksm());
        } else if (child_pid < 0) {
                ksft_test_result_fail("fork() failed\n");
@@ -595,7 +603,7 @@ static void test_prctl_fork_exec(void)
                return;
        } else if (child_pid == 0) {
                char *prg_name = "./ksm_functional_tests";
-               char *argv_for_program[] = { prg_name, FORK_EXEC_CHILD_PRG_NAME 
};
+               char *argv_for_program[] = { prg_name, 
FORK_EXEC_CHILD_PRG_NAME, NULL };

                execv(prg_name, argv_for_program);
                return;
@@ -644,6 +652,7 @@ static void test_prctl_unmerge(void)
        ksft_test_result(!range_maps_duplicates(map, size),
                         "Pages were unmerged\n");
unmap:
+       ksm_unmerge();
        munmap(map, size);
}

@@ -677,6 +686,7 @@ static void test_prot_none(void)
        ksft_test_result(!range_maps_duplicates(map, size),
                         "Pages were unmerged\n");
unmap:
+       ksm_unmerge();
        munmap(map, size);
}

--
2.47.1


Reply via email to