[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250804091141.ifwryfmgjepwrog4@master>
Date: Mon, 4 Aug 2025 09:11:41 +0000
From: Wei Yang <richard.weiyang@...il.com>
To: Aboorva Devarajan <aboorvad@...ux.ibm.com>
Cc: akpm@...ux-foundation.org, Liam.Howlett@...cle.com,
lorenzo.stoakes@...cle.com, shuah@...nel.org, pfalcato@...e.de,
david@...hat.com, ziy@...dia.com, baolin.wang@...ux.alibaba.com,
npache@...hat.com, ryan.roberts@....com, dev.jain@....com,
baohua@...nel.org, linux-mm@...ck.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
donettom@...ux.ibm.com, ritesh.list@...il.com
Subject: Re: [PATCH v3 3/7] selftest/mm: Fix ksm_funtional_test failures
On Tue, Jul 29, 2025 at 11:03:59AM +0530, Aboorva Devarajan wrote:
>From: Donet Tom <donettom@...ux.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 <aboorvad@...ux.ibm.com>
>Signed-off-by: Aboorva Devarajan <aboorvad@...ux.ibm.com>
>Signed-off-by: Donet Tom <donettom@...ux.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?
> 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
>
--
Wei Yang
Help you, Help me
Powered by blists - more mailing lists