Message-ID: <e9079694-1e30-46b6-97e7-b79be01c65a6@linux.ibm.com>
Date: Wed, 6 Aug 2025 18:30:37 +0530
From: Donet Tom <donettom@...ux.ibm.com>
To: Wei Yang <richard.weiyang@...il.com>
Cc: Aboorva Devarajan <aboorvad@...ux.ibm.com>, 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,
        ritesh.list@...il.com
Subject: Re: [PATCH v3 3/7] selftest/mm: Fix ksm_funtional_test failures


On 8/5/25 10:33 PM, Wei Yang wrote:
> On Tue, Aug 05, 2025 at 11:39:15AM +0530, Donet Tom wrote:
>> 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 <donettom@...ux.ibm.com>
>>>>
> [...]
>>>> 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.
>>
> Yes, this one is fixed by calling init_global_file_handles() in child.
>
>> 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
> I assume the behavior described here is after the change to call
> init_global_file_handles() in child.

Yes


>
> Child process inherit the ksm_merging_pages from parent, which is reasonable
> to me. But I am confused why ksm_unmerge() would just reset ksm_merging_pages
> for parent and leave ksm_merging_pages in child process unchanged.
>
> ksm_unmerge() writes to /sys/kernel/mm/ksm/run, which is a system wide sysfs
> interface. I expect it applies to both parent and child.

I am not very familiar with the KSM code, but from what I understand:

The ksm_merging_pages counter is maintained per mm_struct. When
we write to /sys/kernel/mm/ksm/run, unmerging is triggered, and the
counters are updated for all mm_structs present in the ksm_mm_slot list.

A mm_struct gets added to this list  when MADV_MERGEABLE is called.
In the case of the child process, since MADV_MERGEABLE has not been
invoked yet, its mm_struct is not part of the list. As a result,
its ksm_merging_pages counter is not reset.


>> value remained unchanged. That’s why get_my_merging_page() in the child was
>> returning a non-zero value.
>>
> I guess you mean the get_my_merging_page() in __mmap_and_merge_range() return
> a non-zero value. But there is ksm_unmerge() before it. Why this ksm_unmerge()
> couldn't reset the value, but a ksm_unmerge() in parent could.
>
>> 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.
>>
> Also all the tests before test_prctl_fork(), except test_prctl(), calls
>
>    ksft_test_result(!range_maps_duplicates());
>
> If the previous tests succeed, it means there is no duplicate pages. This
> means ksm_merging_pages should be 0 before test_prctl_fork() if other tests
> pass. And the child process would inherit a 0 ksm_merging_pages. (A quick test
> proves it.)


If I understand correctly, all the tests are calling MADV_UNMERGEABLE,
which internally calls break_ksm() in the kernel. This function replaces the
KSM page with an exclusive anonymous page. However, the
ksm_merging_pages counters are not updated at this point.

The function range_maps_duplicates(map, size) checks whether the pages
have been unmerged. Since break_ksm() does perform the unmerge, this
function returns false, and the test passes.

The ksm_merging_pages update happens later via the ksm_scan_thread().
That’s why we observe that ksm_merging_pages values are not reset
immediately after the test finishes.

If we add a sleep(1) after the MADV_UNMERGEABLE call, we can see that
the ksm_merging_pages values are reset after the sleep.

Once the test completes successfully, we can call ksm_unmerge(), which
will immediately reset the ksm_merging_pages value. This way, in the fork
test, the child process will also see the correct value.
>
> So which part of the story I missed?
>

So, during the cleanup phase after a successful test, we can call
ksm_unmerge() to reset the counter. Do you see any issue with
this approach?


>

Powered by blists - more mailing lists