[PATCH 0/3] selftests/mm: assert rmap behave as expected

[PATCH 0/3] selftests/mm: assert rmap behave as expected

[Wei Yang]

As David suggested, currently we don't have a high level test case to
verify the behavior of rmap. This patch set introduce the verification
on rmap by migration.

Patch 1 is a trivial cleanup.
Patch 2 is a preparation to move ksm related operation into vm_util.
Patch 3 is the new test case.

Currently it covers following four scenarios:

  * anonymous page
  * shmem page
  * pagecache page
  * ksm page

RFC->v1:
  * open file in function itself instead of pass fd as paremeter
  * fault in the region by accessing it instead of print content

Wei Yang (3):
  selftests/mm: check a valid fd with negative value
  selftests/mm: put general ksm operation into vm_util
  selftests/mm: assert rmap behave as expected

 MAINTAINERS                                   |   1 +
 tools/testing/selftests/mm/.gitignore         |   1 +
 tools/testing/selftests/mm/Makefile           |   3 +
 .../selftests/mm/ksm_functional_tests.c       |  95 +---
 tools/testing/selftests/mm/rmap.c             | 470 ++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh     |   4 +
 tools/testing/selftests/mm/vm_util.c          | 111 +++++
 tools/testing/selftests/mm/vm_util.h          |   7 +
 8 files changed, 605 insertions(+), 87 deletions(-)
 create mode 100644 tools/testing/selftests/mm/rmap.c

-- 
2.34.1

[PATCH 1/3] selftests/mm: check a valid fd with negative value

[Wei Yang]

fd 0 is usually occupied by stdin, which is not expected to be returned
by open(). But 0 is still a valid fd.

Check the valid fd with negative value like other places.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Harry Yoo <harry.yoo@oracle.com>
---
 tools/testing/selftests/mm/ksm_functional_tests.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index d8bd1911dfc0..6da5c3340e10 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -81,7 +81,7 @@ static long get_my_ksm_zero_pages(void)
 	ssize_t read_size;
 	unsigned long my_ksm_zero_pages;
 
-	if (!proc_self_ksm_stat_fd)
+	if (proc_self_ksm_stat_fd < 0)
 		return 0;
 
 	read_size = pread(proc_self_ksm_stat_fd, buf, sizeof(buf) - 1, 0);
-- 
2.34.1

Re: [PATCH 1/3] selftests/mm: check a valid fd with negative value

[David Hildenbrand]

On 16.07.25 10:27, Wei Yang wrote:
> fd 0 is usually occupied by stdin, which is not expected to be returned
> by open(). But 0 is still a valid fd.
> 
> Check the valid fd with negative value like other places.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> ---
>   tools/testing/selftests/mm/ksm_functional_tests.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
> index d8bd1911dfc0..6da5c3340e10 100644
> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
> @@ -81,7 +81,7 @@ static long get_my_ksm_zero_pages(void)
>   	ssize_t read_size;
>   	unsigned long my_ksm_zero_pages;
>   
> -	if (!proc_self_ksm_stat_fd)
> +	if (proc_self_ksm_stat_fd < 0)
>   		return 0;

Only test_unmerge_zero_pages() calls get_my_ksm_zero_pages(). And there, 
we have

if (proc_self_ksm_stat_fd < 0) {
	ksft_test_result_skip("open(\"/proc/self/ksm_stat\") failed\n");
	return;
}

As the fd is opened after others, we should never get fd=0.

So probably, this should just be handled as part of factoring it out, no 
need for this change upfront AFAIKS.

-- 
Cheers,

David / dhildenb

Re: [PATCH 1/3] selftests/mm: check a valid fd with negative value

[Wei Yang]

On Wed, Jul 16, 2025 at 10:52:21AM +0200, David Hildenbrand wrote:
>On 16.07.25 10:27, Wei Yang wrote:
>> fd 0 is usually occupied by stdin, which is not expected to be returned
>> by open(). But 0 is still a valid fd.
>> 
>> Check the valid fd with negative value like other places.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>> Cc: Rik van Riel <riel@surriel.com>
>> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Harry Yoo <harry.yoo@oracle.com>
>> ---
>>   tools/testing/selftests/mm/ksm_functional_tests.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
>> index d8bd1911dfc0..6da5c3340e10 100644
>> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
>> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
>> @@ -81,7 +81,7 @@ static long get_my_ksm_zero_pages(void)
>>   	ssize_t read_size;
>>   	unsigned long my_ksm_zero_pages;
>> -	if (!proc_self_ksm_stat_fd)
>> +	if (proc_self_ksm_stat_fd < 0)
>>   		return 0;
>
>Only test_unmerge_zero_pages() calls get_my_ksm_zero_pages(). And there, we
>have
>
>if (proc_self_ksm_stat_fd < 0) {
>	ksft_test_result_skip("open(\"/proc/self/ksm_stat\") failed\n");
>	return;
>}
>
>As the fd is opened after others, we should never get fd=0.
>

You are right.

>So probably, this should just be handled as part of factoring it out, no need
>for this change upfront AFAIKS.
>

My intention of this change is, we plan to move get_my_ksm_zero_pages() into a
standalone helper and will open and check the fd there. To make following
change consistent, I split it out.

If you think it is not necessary, will drop this.

>-- 
>Cheers,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me

Re: [PATCH 1/3] selftests/mm: check a valid fd with negative value

[David Hildenbrand]

On 17.07.25 03:18, Wei Yang wrote:
> On Wed, Jul 16, 2025 at 10:52:21AM +0200, David Hildenbrand wrote:
>> On 16.07.25 10:27, Wei Yang wrote:
>>> fd 0 is usually occupied by stdin, which is not expected to be returned
>>> by open(). But 0 is still a valid fd.
>>>
>>> Check the valid fd with negative value like other places.
>>>
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> Cc: Rik van Riel <riel@surriel.com>
>>> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: Harry Yoo <harry.yoo@oracle.com>
>>> ---
>>>    tools/testing/selftests/mm/ksm_functional_tests.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
>>> index d8bd1911dfc0..6da5c3340e10 100644
>>> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
>>> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
>>> @@ -81,7 +81,7 @@ static long get_my_ksm_zero_pages(void)
>>>    	ssize_t read_size;
>>>    	unsigned long my_ksm_zero_pages;
>>> -	if (!proc_self_ksm_stat_fd)
>>> +	if (proc_self_ksm_stat_fd < 0)
>>>    		return 0;
>>
>> Only test_unmerge_zero_pages() calls get_my_ksm_zero_pages(). And there, we
>> have
>>
>> if (proc_self_ksm_stat_fd < 0) {
>> 	ksft_test_result_skip("open(\"/proc/self/ksm_stat\") failed\n");
>> 	return;
>> }
>>
>> As the fd is opened after others, we should never get fd=0.
>>
> 
> You are right.
> 
>> So probably, this should just be handled as part of factoring it out, no need
>> for this change upfront AFAIKS.
>>
> 
> My intention of this change is, we plan to move get_my_ksm_zero_pages() into a
> standalone helper and will open and check the fd there. To make following
> change consistent, I split it out.
> 
> If you think it is not necessary, will drop this.

IMHO, just cleaning it up while factoring out is better.

-- 
Cheers,

David / dhildenb

[PATCH 2/3] selftests/mm: put general ksm operation into vm_util

[Wei Yang]

There are some general ksm operations could be used by other related
test case. Put them into vm_util for common use.

This is a preparation patch for later use.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Suggested-by: David Hildenbrand <david@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Harry Yoo <harry.yoo@oracle.com>

---
v1: open/close fd in function itself instead of pass as parameter
---
 .../selftests/mm/ksm_functional_tests.c       |  95 ++-------------
 tools/testing/selftests/mm/vm_util.c          | 111 ++++++++++++++++++
 tools/testing/selftests/mm/vm_util.h          |   7 ++
 3 files changed, 126 insertions(+), 87 deletions(-)

diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index 6da5c3340e10..0b151f3a29dc 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -38,11 +38,6 @@ enum ksm_merge_mode {
 };
 
 static int mem_fd;
-static int ksm_fd;
-static int ksm_full_scans_fd;
-static int proc_self_ksm_stat_fd;
-static int proc_self_ksm_merging_pages_fd;
-static int ksm_use_zero_pages_fd;
 static int pagemap_fd;
 static size_t pagesize;
 
@@ -73,62 +68,6 @@ static bool range_maps_duplicates(char *addr, unsigned long size)
 	return false;
 }
 
-static long get_my_ksm_zero_pages(void)
-{
-	char buf[200];
-	char *substr_ksm_zero;
-	size_t value_pos;
-	ssize_t read_size;
-	unsigned long my_ksm_zero_pages;
-
-	if (proc_self_ksm_stat_fd < 0)
-		return 0;
-
-	read_size = pread(proc_self_ksm_stat_fd, buf, sizeof(buf) - 1, 0);
-	if (read_size < 0)
-		return -errno;
-
-	buf[read_size] = 0;
-
-	substr_ksm_zero = strstr(buf, "ksm_zero_pages");
-	if (!substr_ksm_zero)
-		return 0;
-
-	value_pos = strcspn(substr_ksm_zero, "0123456789");
-	my_ksm_zero_pages = strtol(substr_ksm_zero + value_pos, NULL, 10);
-
-	return my_ksm_zero_pages;
-}
-
-static long get_my_merging_pages(void)
-{
-	char buf[10];
-	ssize_t ret;
-
-	if (proc_self_ksm_merging_pages_fd < 0)
-		return proc_self_ksm_merging_pages_fd;
-
-	ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0);
-	if (ret <= 0)
-		return -errno;
-	buf[ret] = 0;
-
-	return strtol(buf, NULL, 10);
-}
-
-static long ksm_get_full_scans(void)
-{
-	char buf[10];
-	ssize_t ret;
-
-	ret = pread(ksm_full_scans_fd, buf, sizeof(buf) - 1, 0);
-	if (ret <= 0)
-		return -errno;
-	buf[ret] = 0;
-
-	return strtol(buf, NULL, 10);
-}
-
 static int ksm_merge(void)
 {
 	long start_scans, end_scans;
@@ -137,7 +76,7 @@ static int ksm_merge(void)
 	start_scans = ksm_get_full_scans();
 	if (start_scans < 0)
 		return start_scans;
-	if (write(ksm_fd, "1", 1) != 1)
+	if (ksm_start_and_merge())
 		return -errno;
 	do {
 		end_scans = ksm_get_full_scans();
@@ -150,7 +89,7 @@ static int ksm_merge(void)
 
 static int ksm_unmerge(void)
 {
-	if (write(ksm_fd, "2", 1) != 1)
+	if (ksm_stop_and_unmerge())
 		return -errno;
 	return 0;
 }
@@ -168,7 +107,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot,
 		return err_map;
 	}
 
-	if (get_my_merging_pages() > 0) {
+	if (ksm_get_self_merging_pages() > 0) {
 		ksft_print_msg("Still pages merged\n");
 		return err_map;
 	}
@@ -227,7 +166,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot,
 	 * Check if anything was merged at all. Ignore the zero page that is
 	 * accounted differently (depending on kernel support).
 	 */
-	if (val && !get_my_merging_pages()) {
+	if (val && !ksm_get_self_merging_pages()) {
 		ksft_print_msg("No pages got merged\n");
 		goto unmap;
 	}
@@ -286,15 +225,7 @@ static void test_unmerge_zero_pages(void)
 
 	ksft_print_msg("[RUN] %s\n", __func__);
 
-	if (proc_self_ksm_stat_fd < 0) {
-		ksft_test_result_skip("open(\"/proc/self/ksm_stat\") failed\n");
-		return;
-	}
-	if (ksm_use_zero_pages_fd < 0) {
-		ksft_test_result_skip("open \"/sys/kernel/mm/ksm/use_zero_pages\" failed\n");
-		return;
-	}
-	if (write(ksm_use_zero_pages_fd, "1", 1) != 1) {
+	if (ksm_use_zero_pages()) {
 		ksft_test_result_skip("write \"/sys/kernel/mm/ksm/use_zero_pages\" failed\n");
 		return;
 	}
@@ -306,7 +237,7 @@ static void test_unmerge_zero_pages(void)
 
 	/* Check if ksm_zero_pages is updated correctly after KSM merging */
 	pages_expected = size / pagesize;
-	if (pages_expected != get_my_ksm_zero_pages()) {
+	if (pages_expected != ksm_get_self_zero_pages()) {
 		ksft_test_result_fail("'ksm_zero_pages' updated after merging\n");
 		goto unmap;
 	}
@@ -319,7 +250,7 @@ static void test_unmerge_zero_pages(void)
 
 	/* Check if ksm_zero_pages is updated correctly after unmerging */
 	pages_expected /= 2;
-	if (pages_expected != get_my_ksm_zero_pages()) {
+	if (pages_expected != ksm_get_self_zero_pages()) {
 		ksft_test_result_fail("'ksm_zero_pages' updated after unmerging\n");
 		goto unmap;
 	}
@@ -329,7 +260,7 @@ static void test_unmerge_zero_pages(void)
 		*((unsigned int *)&map[offs]) = offs;
 
 	/* Now we should have no zeropages remaining. */
-	if (get_my_ksm_zero_pages()) {
+	if (ksm_get_self_zero_pages()) {
 		ksft_test_result_fail("'ksm_zero_pages' updated after write fault\n");
 		goto unmap;
 	}
@@ -685,19 +616,9 @@ static void init_global_file_handles(void)
 	mem_fd = open("/proc/self/mem", O_RDWR);
 	if (mem_fd < 0)
 		ksft_exit_fail_msg("opening /proc/self/mem failed\n");
-	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
-	if (ksm_fd < 0)
-		ksft_exit_skip("open(\"/sys/kernel/mm/ksm/run\") failed\n");
-	ksm_full_scans_fd = open("/sys/kernel/mm/ksm/full_scans", O_RDONLY);
-	if (ksm_full_scans_fd < 0)
-		ksft_exit_skip("open(\"/sys/kernel/mm/ksm/full_scans\") failed\n");
 	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
 	if (pagemap_fd < 0)
 		ksft_exit_skip("open(\"/proc/self/pagemap\") failed\n");
-	proc_self_ksm_stat_fd = open("/proc/self/ksm_stat", O_RDONLY);
-	proc_self_ksm_merging_pages_fd = open("/proc/self/ksm_merging_pages",
-						O_RDONLY);
-	ksm_use_zero_pages_fd = open("/sys/kernel/mm/ksm/use_zero_pages", O_RDWR);
 }
 
 int main(int argc, char **argv)
diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index 9dafa7669ef9..14973d957c9a 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -555,3 +555,114 @@ bool detect_huge_zeropage(void)
 	close(fd);
 	return enabled;
 }
+
+long ksm_get_self_zero_pages()
+{
+	int proc_self_ksm_stat_fd;
+	char buf[200];
+	char *substr_ksm_zero;
+	size_t value_pos;
+	ssize_t read_size;
+	unsigned long my_ksm_zero_pages;
+
+	proc_self_ksm_stat_fd = open("/proc/self/ksm_stat", O_RDONLY);
+	if (proc_self_ksm_stat_fd < 0)
+		return 0;
+
+	read_size = pread(proc_self_ksm_stat_fd, buf, sizeof(buf) - 1, 0);
+	close(proc_self_ksm_stat_fd);
+	if (read_size < 0)
+		return -errno;
+
+	buf[read_size] = 0;
+
+	substr_ksm_zero = strstr(buf, "ksm_zero_pages");
+	if (!substr_ksm_zero)
+		return 0;
+
+	value_pos = strcspn(substr_ksm_zero, "0123456789");
+	my_ksm_zero_pages = strtol(substr_ksm_zero + value_pos, NULL, 10);
+
+	return my_ksm_zero_pages;
+}
+
+long ksm_get_self_merging_pages()
+{
+	int proc_self_ksm_merging_pages_fd;
+	char buf[10];
+	ssize_t ret;
+
+	proc_self_ksm_merging_pages_fd = open("/proc/self/ksm_merging_pages",
+						O_RDONLY);
+	if (proc_self_ksm_merging_pages_fd < 0)
+		return proc_self_ksm_merging_pages_fd;
+
+	ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0);
+	close(proc_self_ksm_merging_pages_fd);
+	if (ret <= 0)
+		return -errno;
+	buf[ret] = 0;
+
+	return strtol(buf, NULL, 10);
+}
+
+long ksm_get_full_scans()
+{
+	int ksm_full_scans_fd;
+	char buf[10];
+	ssize_t ret;
+
+	ksm_full_scans_fd = open("/sys/kernel/mm/ksm/full_scans", O_RDONLY);
+	if (ksm_full_scans_fd < 0)
+		return ksm_full_scans_fd;
+
+	ret = pread(ksm_full_scans_fd, buf, sizeof(buf) - 1, 0);
+	close(ksm_full_scans_fd);
+	if (ret <= 0)
+		return -errno;
+	buf[ret] = 0;
+
+	return strtol(buf, NULL, 10);
+}
+
+int ksm_use_zero_pages()
+{
+	int ksm_use_zero_pages_fd;
+	ssize_t ret;
+
+	ksm_use_zero_pages_fd = open("/sys/kernel/mm/ksm/use_zero_pages", O_RDWR);
+	if (ksm_use_zero_pages_fd < 0)
+		return -1;
+
+	ret = write(ksm_use_zero_pages_fd, "1", 1);
+	close(ksm_use_zero_pages_fd);
+	return ret == 1 ? 0 : ret;
+}
+
+int ksm_start_and_merge()
+{
+	int ksm_fd;
+	ssize_t ret;
+
+	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
+	if (ksm_fd < 0)
+		return -1;
+
+	ret = write(ksm_fd, "1", 1);
+	close(ksm_fd);
+	return ret == 1 ?  0 : ret;
+}
+
+int ksm_stop_and_unmerge()
+{
+	int ksm_fd;
+	ssize_t ret;
+
+	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
+	if (ksm_fd < 0)
+		return -1;
+
+	ret = write(ksm_fd, "2", 1);
+	close(ksm_fd);
+	return ret == 1 ?  0 : ret;
+}
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index 2b154c287591..752ae0482df6 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -122,6 +122,13 @@ static inline void log_test_result(int result)
 void *sys_mremap(void *old_address, unsigned long old_size,
 		 unsigned long new_size, int flags, void *new_address);
 
+long ksm_get_self_zero_pages();
+long ksm_get_self_merging_pages();
+long ksm_get_full_scans();
+int ksm_use_zero_pages();
+int ksm_start_and_merge();
+int ksm_stop_and_unmerge();
+
 /*
  * On ppc64 this will only work with radix 2M hugepage size
  */
-- 
2.34.1

Re: [PATCH 2/3] selftests/mm: put general ksm operation into vm_util

[David Hildenbrand]

On 16.07.25 10:27, Wei Yang wrote:
> There are some general ksm operations could be used by other related
> test case. Put them into vm_util for common use.
> 
> This is a preparation patch for later use.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> 
> ---
> v1: open/close fd in function itself instead of pass as parameter
> ---
>   .../selftests/mm/ksm_functional_tests.c       |  95 ++-------------
>   tools/testing/selftests/mm/vm_util.c          | 111 ++++++++++++++++++
>   tools/testing/selftests/mm/vm_util.h          |   7 ++
>   3 files changed, 126 insertions(+), 87 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
> index 6da5c3340e10..0b151f3a29dc 100644
> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
> @@ -38,11 +38,6 @@ enum ksm_merge_mode {
>   };
>   
>   static int mem_fd;
> -static int ksm_fd;
> -static int ksm_full_scans_fd;
> -static int proc_self_ksm_stat_fd;
> -static int proc_self_ksm_merging_pages_fd;
> -static int ksm_use_zero_pages_fd;
>   static int pagemap_fd;
>   static size_t pagesize;
>   
> @@ -73,62 +68,6 @@ static bool range_maps_duplicates(char *addr, unsigned long size)
>   	return false;
>   }
>   
> -static long get_my_ksm_zero_pages(void)
> -{
> -	char buf[200];
> -	char *substr_ksm_zero;
> -	size_t value_pos;
> -	ssize_t read_size;
> -	unsigned long my_ksm_zero_pages;
> -
> -	if (proc_self_ksm_stat_fd < 0)
> -		return 0;
> -
> -	read_size = pread(proc_self_ksm_stat_fd, buf, sizeof(buf) - 1, 0);
> -	if (read_size < 0)
> -		return -errno;
> -
> -	buf[read_size] = 0;
> -
> -	substr_ksm_zero = strstr(buf, "ksm_zero_pages");
> -	if (!substr_ksm_zero)
> -		return 0;
> -
> -	value_pos = strcspn(substr_ksm_zero, "0123456789");
> -	my_ksm_zero_pages = strtol(substr_ksm_zero + value_pos, NULL, 10);
> -
> -	return my_ksm_zero_pages;
> -}
> -
> -static long get_my_merging_pages(void)
> -{
> -	char buf[10];
> -	ssize_t ret;
> -
> -	if (proc_self_ksm_merging_pages_fd < 0)
> -		return proc_self_ksm_merging_pages_fd;
> -
> -	ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0);
> -	if (ret <= 0)
> -		return -errno;
> -	buf[ret] = 0;
> -
> -	return strtol(buf, NULL, 10);
> -}
> -
> -static long ksm_get_full_scans(void)
> -{
> -	char buf[10];
> -	ssize_t ret;
> -
> -	ret = pread(ksm_full_scans_fd, buf, sizeof(buf) - 1, 0);
> -	if (ret <= 0)
> -		return -errno;
> -	buf[ret] = 0;
> -
> -	return strtol(buf, NULL, 10);
> -}
> -
>   static int ksm_merge(void)

Should ksm_merge() get factored out as well?

>   {
>   	long start_scans, end_scans;
> @@ -137,7 +76,7 @@ static int ksm_merge(void)
>   	start_scans = ksm_get_full_scans();
>   	if (start_scans < 0)
>   		return start_scans;
> -	if (write(ksm_fd, "1", 1) != 1)
> +	if (ksm_start_and_merge())
>   		return -errno;
>   	do {
>   		end_scans = ksm_get_full_scans();
> @@ -150,7 +89,7 @@ static int ksm_merge(void)
>   
>   static int ksm_unmerge(void)
>   {
> -	if (write(ksm_fd, "2", 1) != 1)
> +	if (ksm_stop_and_unmerge())
>   		return -errno;
>   	return 0;

What's the reason of gaving ksm_unmerge() and ksm_stop_and_unmerge()?

Probably we should just use ksm_stop_and_unmerge() and remove ksm_unmerge().

See below regarding letting ksm_stop_and_unmerge() and friends return -errno in case of error.

>   }
> @@ -168,7 +107,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot,
>   		return err_map;
>   	}
>   
> -	if (get_my_merging_pages() > 0) {
> +	if (ksm_get_self_merging_pages() > 0) {
>   		ksft_print_msg("Still pages merged\n");
>   		return err_map;
>   	}
> @@ -227,7 +166,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot,
>   	 * Check if anything was merged at all. Ignore the zero page that is
>   	 * accounted differently (depending on kernel support).
>   	 */
> -	if (val && !get_my_merging_pages()) {
> +	if (val && !ksm_get_self_merging_pages()) {
>   		ksft_print_msg("No pages got merged\n");
>   		goto unmap;
>   	}
> @@ -286,15 +225,7 @@ static void test_unmerge_zero_pages(void)
>   
>   	ksft_print_msg("[RUN] %s\n", __func__);
>   
> -	if (proc_self_ksm_stat_fd < 0) {
> -		ksft_test_result_skip("open(\"/proc/self/ksm_stat\") failed\n");
> -		return;
> -	}

See below: probably we should do a test read so we know the file exists and
can be read. So we don't get misleading errors later on older kernels.

if (ksm_get_self_zero_pages() < 0) {
	ksft_test_result_skip("accessing \"/proc/self/ksm_stat\" failed\n");
	return;
}

> -	if (ksm_use_zero_pages_fd < 0) {
> -		ksft_test_result_skip("open \"/sys/kernel/mm/ksm/use_zero_pages\" failed\n");
> -		return;
> -	}
> -	if (write(ksm_use_zero_pages_fd, "1", 1) != 1) {
> +	if (ksm_use_zero_pages()) {
>   		ksft_test_result_skip("write \"/sys/kernel/mm/ksm/use_zero_pages\" failed\n");
>   		return;
>   	}
> @@ -306,7 +237,7 @@ static void test_unmerge_zero_pages(void)
>   
>   	/* Check if ksm_zero_pages is updated correctly after KSM merging */
>   	pages_expected = size / pagesize;
> -	if (pages_expected != get_my_ksm_zero_pages()) {
> +	if (pages_expected != ksm_get_self_zero_pages()) {
>   		ksft_test_result_fail("'ksm_zero_pages' updated after merging\n");
>   		goto unmap;
>   	}
> @@ -319,7 +250,7 @@ static void test_unmerge_zero_pages(void)
>   
>   	/* Check if ksm_zero_pages is updated correctly after unmerging */
>   	pages_expected /= 2;
> -	if (pages_expected != get_my_ksm_zero_pages()) {
> +	if (pages_expected != ksm_get_self_zero_pages()) {
>   		ksft_test_result_fail("'ksm_zero_pages' updated after unmerging\n");
>   		goto unmap;
>   	}
> @@ -329,7 +260,7 @@ static void test_unmerge_zero_pages(void)
>   		*((unsigned int *)&map[offs]) = offs;
>   
>   	/* Now we should have no zeropages remaining. */
> -	if (get_my_ksm_zero_pages()) {
> +	if (ksm_get_self_zero_pages()) {
>   		ksft_test_result_fail("'ksm_zero_pages' updated after write fault\n");
>   		goto unmap;
>   	}
> @@ -685,19 +616,9 @@ static void init_global_file_handles(void)
>   	mem_fd = open("/proc/self/mem", O_RDWR);
>   	if (mem_fd < 0)
>   		ksft_exit_fail_msg("opening /proc/self/mem failed\n");
> -	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
> -	if (ksm_fd < 0)
> -		ksft_exit_skip("open(\"/sys/kernel/mm/ksm/run\") failed\n");
> -	ksm_full_scans_fd = open("/sys/kernel/mm/ksm/full_scans", O_RDONLY);
> -	if (ksm_full_scans_fd < 0)
> -		ksft_exit_skip("open(\"/sys/kernel/mm/ksm/full_scans\") failed\n");

For these skip cases, we should probably do a test access.

if (ksm_stop_and_unmerge() < 0)
	ksft_exit_skip("accessing \"/sys/kernel/mm/ksm/run\\") failed\n");
if (ksm_get_full_scans() < 0)
	ksft_exit_skip("accessing \"/sys/kernel/mm/ksm/full_scans\") failed\n");

So we later now that the files actually do exist and can be opened+written.


>   	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);>   	if (pagemap_fd < 0)
>   		ksft_exit_skip("open(\"/proc/self/pagemap\") failed\n");
> -	proc_self_ksm_stat_fd = open("/proc/self/ksm_stat", O_RDONLY);
> -	proc_self_ksm_merging_pages_fd = open("/proc/self/ksm_merging_pages",
> -						O_RDONLY);
> -	ksm_use_zero_pages_fd = open("/sys/kernel/mm/ksm/use_zero_pages", O_RDWR);
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index 9dafa7669ef9..14973d957c9a 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -555,3 +555,114 @@ bool detect_huge_zeropage(void)
>   	close(fd);
>   	return enabled;
>   }
> +
> +long ksm_get_self_zero_pages()

For all these functions without parameters:

(void)

> +{
> +	int proc_self_ksm_stat_fd;
> +	char buf[200];
> +	char *substr_ksm_zero;
> +	size_t value_pos;
> +	ssize_t read_size;
> +	unsigned long my_ksm_zero_pages;
> +
> +	proc_self_ksm_stat_fd = open("/proc/self/ksm_stat", O_RDONLY);
> +	if (proc_self_ksm_stat_fd < 0)
> +		return 0;
> +
> +	read_size = pread(proc_self_ksm_stat_fd, buf, sizeof(buf) - 1, 0);
> +	close(proc_self_ksm_stat_fd);
> +	if (read_size < 0)
> +		return -errno;
> +
> +	buf[read_size] = 0;
> +
> +	substr_ksm_zero = strstr(buf, "ksm_zero_pages");
> +	if (!substr_ksm_zero)
> +		return 0;
> +
> +	value_pos = strcspn(substr_ksm_zero, "0123456789");
> +	my_ksm_zero_pages = strtol(substr_ksm_zero + value_pos, NULL, 10);
> +
> +	return my_ksm_zero_pages;
> +}
> +
> +long ksm_get_self_merging_pages()
> +{
> +	int proc_self_ksm_merging_pages_fd;
> +	char buf[10];
> +	ssize_t ret;
> +
> +	proc_self_ksm_merging_pages_fd = open("/proc/self/ksm_merging_pages",
> +						O_RDONLY);
> +	if (proc_self_ksm_merging_pages_fd < 0)
> +		return proc_self_ksm_merging_pages_fd;
> +
> +	ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0);
> +	close(proc_self_ksm_merging_pages_fd);
> +	if (ret <= 0)
> +		return -errno;
> +	buf[ret] = 0;
> +
> +	return strtol(buf, NULL, 10);
> +}
> +
> +long ksm_get_full_scans()
> +{
> +	int ksm_full_scans_fd;
> +	char buf[10];
> +	ssize_t ret;
> +
> +	ksm_full_scans_fd = open("/sys/kernel/mm/ksm/full_scans", O_RDONLY);
> +	if (ksm_full_scans_fd < 0)
> +		return ksm_full_scans_fd;
> +
> +	ret = pread(ksm_full_scans_fd, buf, sizeof(buf) - 1, 0);
> +	close(ksm_full_scans_fd);
> +	if (ret <= 0)
> +		return -errno;
> +	buf[ret] = 0;
> +
> +	return strtol(buf, NULL, 10);
> +}
> +
> +int ksm_use_zero_pages()
> +{
> +	int ksm_use_zero_pages_fd;
> +	ssize_t ret;
> +
> +	ksm_use_zero_pages_fd = open("/sys/kernel/mm/ksm/use_zero_pages", O_RDWR);
> +	if (ksm_use_zero_pages_fd < 0)
> +		return -1;

For all these functions, we should not require the caller to lookup errno but
instead return it.

	return -errno;

> +
> +	ret = write(ksm_use_zero_pages_fd, "1", 1);
> +	close(ksm_use_zero_pages_fd);
> +	return ret == 1 ? 0 : ret;

return ret == 1 ? 0 : -errno;

> +}
> +
> +int ksm_start_and_merge()
> +{
> +	int ksm_fd;
> +	ssize_t ret;
> +
> +	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
> +	if (ksm_fd < 0)
> +		return -1;

Same comments regarding errno.

> +
> +	ret = write(ksm_fd, "1", 1);
> +	close(ksm_fd);
> +	return ret == 1 ?  0 : ret;

double space

> +}
> +
> +int ksm_stop_and_unmerge()
> +{
> +	int ksm_fd;
> +	ssize_t ret;
> +
> +	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
> +	if (ksm_fd < 0)
> +		return -1;
> +
> +	ret = write(ksm_fd, "2", 1);
> +	close(ksm_fd);
> +	return ret == 1 ?  0 : ret;

double space



-- 
Cheers,

David / dhildenb

Re: [PATCH 2/3] selftests/mm: put general ksm operation into vm_util

[Wei Yang]

Thanks for the detailed comment.

On Wed, Jul 16, 2025 at 11:20:09AM +0200, David Hildenbrand wrote:
>On 16.07.25 10:27, Wei Yang wrote:
[...]
>> -static long ksm_get_full_scans(void)
>> -{
>> -	char buf[10];
>> -	ssize_t ret;
>> -
>> -	ret = pread(ksm_full_scans_fd, buf, sizeof(buf) - 1, 0);
>> -	if (ret <= 0)
>> -		return -errno;
>> -	buf[ret] = 0;
>> -
>> -	return strtol(buf, NULL, 10);
>> -}
>> -
>>   static int ksm_merge(void)
>
>Should ksm_merge() get factored out as well?
>

Reasonable, will factor out.

While one thing interesting is if it scan 2 full round, the ksm case in patch
3 would fail sometimes. But I don't see the failure in ksm_functional_test
here.

>>   {
>>   	long start_scans, end_scans;
>> @@ -137,7 +76,7 @@ static int ksm_merge(void)
>>   	start_scans = ksm_get_full_scans();
>>   	if (start_scans < 0)
>>   		return start_scans;
>> -	if (write(ksm_fd, "1", 1) != 1)
>> +	if (ksm_start_and_merge())
>>   		return -errno;
>>   	do {
>>   		end_scans = ksm_get_full_scans();
>> @@ -150,7 +89,7 @@ static int ksm_merge(void)
>>   static int ksm_unmerge(void)
>>   {
>> -	if (write(ksm_fd, "2", 1) != 1)
>> +	if (ksm_stop_and_unmerge())
>>   		return -errno;
>>   	return 0;
>
>What's the reason of gaving ksm_unmerge() and ksm_stop_and_unmerge()?
>

My plan is there are two pairs of helper:

    ksm_merge()  <->  ksm_unmerge()
    ksm_start_and_merge() <-> ksm_stop_and_unmerge()

>Probably we should just use ksm_stop_and_unmerge() and remove ksm_unmerge().
>

Looks reasonable, will remove it.

So how about leave three helpers:

   ksm_merge()
   ksm_start()
   ksm_stop_and_unmerge()

Would this be better?

>See below regarding letting ksm_stop_and_unmerge() and friends return -errno in case of error.
>
>>   }
>> @@ -168,7 +107,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot,
>>   		return err_map;
>>   	}
>> -	if (get_my_merging_pages() > 0) {
>> +	if (ksm_get_self_merging_pages() > 0) {
>>   		ksft_print_msg("Still pages merged\n");
>>   		return err_map;
>>   	}
>> @@ -227,7 +166,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot,
>>   	 * Check if anything was merged at all. Ignore the zero page that is
>>   	 * accounted differently (depending on kernel support).
>>   	 */
>> -	if (val && !get_my_merging_pages()) {
>> +	if (val && !ksm_get_self_merging_pages()) {
>>   		ksft_print_msg("No pages got merged\n");
>>   		goto unmap;
>>   	}
>> @@ -286,15 +225,7 @@ static void test_unmerge_zero_pages(void)
>>   	ksft_print_msg("[RUN] %s\n", __func__);
>> -	if (proc_self_ksm_stat_fd < 0) {
>> -		ksft_test_result_skip("open(\"/proc/self/ksm_stat\") failed\n");
>> -		return;
>> -	}
>
>See below: probably we should do a test read so we know the file exists and
>can be read. So we don't get misleading errors later on older kernels.
>
>if (ksm_get_self_zero_pages() < 0) {
>	ksft_test_result_skip("accessing \"/proc/self/ksm_stat\" failed\n");
>	return;
>}

Agree

>
>> -	if (ksm_use_zero_pages_fd < 0) {
>> -		ksft_test_result_skip("open \"/sys/kernel/mm/ksm/use_zero_pages\" failed\n");
>> -		return;
>> -	}
>> -	if (write(ksm_use_zero_pages_fd, "1", 1) != 1) {
>> +	if (ksm_use_zero_pages()) {
>>   		ksft_test_result_skip("write \"/sys/kernel/mm/ksm/use_zero_pages\" failed\n");
>>   		return;
>>   	}
>> @@ -306,7 +237,7 @@ static void test_unmerge_zero_pages(void)
>>   	/* Check if ksm_zero_pages is updated correctly after KSM merging */
>>   	pages_expected = size / pagesize;
>> -	if (pages_expected != get_my_ksm_zero_pages()) {
>> +	if (pages_expected != ksm_get_self_zero_pages()) {
>>   		ksft_test_result_fail("'ksm_zero_pages' updated after merging\n");
>>   		goto unmap;
>>   	}
>> @@ -319,7 +250,7 @@ static void test_unmerge_zero_pages(void)
>>   	/* Check if ksm_zero_pages is updated correctly after unmerging */
>>   	pages_expected /= 2;
>> -	if (pages_expected != get_my_ksm_zero_pages()) {
>> +	if (pages_expected != ksm_get_self_zero_pages()) {
>>   		ksft_test_result_fail("'ksm_zero_pages' updated after unmerging\n");
>>   		goto unmap;
>>   	}
>> @@ -329,7 +260,7 @@ static void test_unmerge_zero_pages(void)
>>   		*((unsigned int *)&map[offs]) = offs;
>>   	/* Now we should have no zeropages remaining. */
>> -	if (get_my_ksm_zero_pages()) {
>> +	if (ksm_get_self_zero_pages()) {
>>   		ksft_test_result_fail("'ksm_zero_pages' updated after write fault\n");
>>   		goto unmap;
>>   	}
>> @@ -685,19 +616,9 @@ static void init_global_file_handles(void)
>>   	mem_fd = open("/proc/self/mem", O_RDWR);
>>   	if (mem_fd < 0)
>>   		ksft_exit_fail_msg("opening /proc/self/mem failed\n");
>> -	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
>> -	if (ksm_fd < 0)
>> -		ksft_exit_skip("open(\"/sys/kernel/mm/ksm/run\") failed\n");
>> -	ksm_full_scans_fd = open("/sys/kernel/mm/ksm/full_scans", O_RDONLY);
>> -	if (ksm_full_scans_fd < 0)
>> -		ksft_exit_skip("open(\"/sys/kernel/mm/ksm/full_scans\") failed\n");
>
>For these skip cases, we should probably do a test access.
>
>if (ksm_stop_and_unmerge() < 0)
>	ksft_exit_skip("accessing \"/sys/kernel/mm/ksm/run\\") failed\n");
>if (ksm_get_full_scans() < 0)
>	ksft_exit_skip("accessing \"/sys/kernel/mm/ksm/full_scans\") failed\n");
>
>So we later now that the files actually do exist and can be opened+written.
>

Agree

>
>>   	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);>   	if (pagemap_fd < 0)
>>   		ksft_exit_skip("open(\"/proc/self/pagemap\") failed\n");
>> -	proc_self_ksm_stat_fd = open("/proc/self/ksm_stat", O_RDONLY);
>> -	proc_self_ksm_merging_pages_fd = open("/proc/self/ksm_merging_pages",

And I think we may do similar thing here.

  if (ksm_get_self_merging_pages() < 0)
	ksft_exit_skip("accessing \"/proc/self/ksm_merging_pages\") failed\n");

>> -						O_RDONLY);
>> -	ksm_use_zero_pages_fd = open("/sys/kernel/mm/ksm/use_zero_pages", O_RDWR);

proc_self_ksm_stat_fd and ksm_use_zero_pages_fd  is only used in
test_unmerge_zero_pages(). So we can test and skip only there.

For proc_self_ksm_merging_pages_fd, it is used in all tests except
test_prctl(). Check ahead maybe helpful.

>>   }
>>   int main(int argc, char **argv)
>> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
>> index 9dafa7669ef9..14973d957c9a 100644
>> --- a/tools/testing/selftests/mm/vm_util.c
>> +++ b/tools/testing/selftests/mm/vm_util.c
>> @@ -555,3 +555,114 @@ bool detect_huge_zeropage(void)
>>   	close(fd);
>>   	return enabled;
>>   }
>> +
>> +long ksm_get_self_zero_pages()
>
>For all these functions without parameters:
>
>(void)
>

Thanks will add it.

>> +{
>> +	int proc_self_ksm_stat_fd;
>> +	char buf[200];
>> +	char *substr_ksm_zero;
>> +	size_t value_pos;
>> +	ssize_t read_size;
>> +	unsigned long my_ksm_zero_pages;
>> +
>> +	proc_self_ksm_stat_fd = open("/proc/self/ksm_stat", O_RDONLY);
>> +	if (proc_self_ksm_stat_fd < 0)
>> +		return 0;
>> +
>> +	read_size = pread(proc_self_ksm_stat_fd, buf, sizeof(buf) - 1, 0);
>> +	close(proc_self_ksm_stat_fd);
>> +	if (read_size < 0)
>> +		return -errno;
>> +
>> +	buf[read_size] = 0;
>> +
>> +	substr_ksm_zero = strstr(buf, "ksm_zero_pages");
>> +	if (!substr_ksm_zero)
>> +		return 0;
>> +
>> +	value_pos = strcspn(substr_ksm_zero, "0123456789");
>> +	my_ksm_zero_pages = strtol(substr_ksm_zero + value_pos, NULL, 10);
>> +
>> +	return my_ksm_zero_pages;
>> +}
>> +
>> +long ksm_get_self_merging_pages()
>> +{
>> +	int proc_self_ksm_merging_pages_fd;
>> +	char buf[10];
>> +	ssize_t ret;
>> +
>> +	proc_self_ksm_merging_pages_fd = open("/proc/self/ksm_merging_pages",
>> +						O_RDONLY);
>> +	if (proc_self_ksm_merging_pages_fd < 0)
>> +		return proc_self_ksm_merging_pages_fd;
>> +
>> +	ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0);
>> +	close(proc_self_ksm_merging_pages_fd);
>> +	if (ret <= 0)
>> +		return -errno;
>> +	buf[ret] = 0;
>> +
>> +	return strtol(buf, NULL, 10);
>> +}
>> +
>> +long ksm_get_full_scans()
>> +{
>> +	int ksm_full_scans_fd;
>> +	char buf[10];
>> +	ssize_t ret;
>> +
>> +	ksm_full_scans_fd = open("/sys/kernel/mm/ksm/full_scans", O_RDONLY);
>> +	if (ksm_full_scans_fd < 0)
>> +		return ksm_full_scans_fd;
>> +
>> +	ret = pread(ksm_full_scans_fd, buf, sizeof(buf) - 1, 0);
>> +	close(ksm_full_scans_fd);
>> +	if (ret <= 0)
>> +		return -errno;
>> +	buf[ret] = 0;
>> +
>> +	return strtol(buf, NULL, 10);
>> +}
>> +
>> +int ksm_use_zero_pages()
>> +{
>> +	int ksm_use_zero_pages_fd;
>> +	ssize_t ret;
>> +
>> +	ksm_use_zero_pages_fd = open("/sys/kernel/mm/ksm/use_zero_pages", O_RDWR);
>> +	if (ksm_use_zero_pages_fd < 0)
>> +		return -1;
>
>For all these functions, we should not require the caller to lookup errno but
>instead return it.
>
>	return -errno;
>

Agree on the -errno thing, will change in all places.

>> +
>> +	ret = write(ksm_use_zero_pages_fd, "1", 1);
>> +	close(ksm_use_zero_pages_fd);
>> +	return ret == 1 ? 0 : ret;
>
>return ret == 1 ? 0 : -errno;
>
>> +}
>> +
>> +int ksm_start_and_merge()
>> +{
>> +	int ksm_fd;
>> +	ssize_t ret;
>> +
>> +	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
>> +	if (ksm_fd < 0)
>> +		return -1;
>
>Same comments regarding errno.
>
>> +
>> +	ret = write(ksm_fd, "1", 1);
>> +	close(ksm_fd);
>> +	return ret == 1 ?  0 : ret;
>
>double space
>
>> +}
>> +
>> +int ksm_stop_and_unmerge()
>> +{
>> +	int ksm_fd;
>> +	ssize_t ret;
>> +
>> +	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
>> +	if (ksm_fd < 0)
>> +		return -1;
>> +
>> +	ret = write(ksm_fd, "2", 1);
>> +	close(ksm_fd);
>> +	return ret == 1 ?  0 : ret;
>
>double space
>

Shame on me :-(

>
>
>-- 
>Cheers,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me

Re: [PATCH 2/3] selftests/mm: put general ksm operation into vm_util

[Wei Yang]

On Thu, Jul 17, 2025 at 02:45:04AM +0000, Wei Yang wrote:
[...]
>
>Looks reasonable, will remove it.
>
>So how about leave three helpers:
>
>   ksm_merge()
>   ksm_start()
>   ksm_stop_and_unmerge()
>
>Would this be better?

Or maybe we just export ksm_merge() and ksm_unmerge(). Leave ksm_start() as an
internal function, since only ksm_merge() use it.

-- 
Wei Yang
Help you, Help me

Re: [PATCH 2/3] selftests/mm: put general ksm operation into vm_util

[David Hildenbrand]

On 17.07.25 05:30, Wei Yang wrote:
> On Thu, Jul 17, 2025 at 02:45:04AM +0000, Wei Yang wrote:
> [...]
>>
>> Looks reasonable, will remove it.
>>
>> So how about leave three helpers:
>>
>>    ksm_merge()
>>    ksm_start()
>>    ksm_stop_and_unmerge()
>>
>> Would this be better?
> 
> Or maybe we just export ksm_merge() and ksm_unmerge(). Leave ksm_start() as an
> internal function, since only ksm_merge() use it.

Either that or start/stop. Whatever you think is best.

-- 
Cheers,

David / dhildenb

Re: [PATCH 2/3] selftests/mm: put general ksm operation into vm_util

[David Hildenbrand]

> My plan is there are two pairs of helper:
> 
>      ksm_merge()  <->  ksm_unmerge()
>      ksm_start_and_merge() <-> ksm_stop_and_unmerge()
> 
>> Probably we should just use ksm_stop_and_unmerge() and remove ksm_unmerge().
>>
> 
> Looks reasonable, will remove it.
> 
> So how about leave three helpers:
> 
>     ksm_merge()
>     ksm_start()
>     ksm_stop_and_unmerge()
> 
> Would this be better?

Is there utility for separating ksm_start() and ksm_merge() in current 
code and with what you are intending to do?

>>> +
>>> +int ksm_stop_and_unmerge()
>>> +{
>>> +	int ksm_fd;
>>> +	ssize_t ret;
>>> +
>>> +	ksm_fd = open("/sys/kernel/mm/ksm/run", O_RDWR);
>>> +	if (ksm_fd < 0)
>>> +		return -1;
>>> +
>>> +	ret = write(ksm_fd, "2", 1);
>>> +	close(ksm_fd);
>>> +	return ret == 1 ?  0 : ret;
>>
>> double space
>>
> 
> Shame on me :-(

Not the end of the world ;)

-- 
Cheers,

David / dhildenb

[PATCH 3/3] selftests/mm: assert rmap behave as expected

[Wei Yang]

As David suggested, currently we don't have a high level test case to
verify the behavior of rmap. This patch introduce the verification on
rmap by migration.

The general idea is if migrate one shared page between processes, this
would be reflected in all related processes. Otherwise, we have problem
in rmap.

Currently it covers following four scenarios:

  * anonymous page
  * shmem page
  * pagecache page
  * ksm page

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Suggested-by: David Hildenbrand <david@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Harry Yoo <harry.yoo@oracle.com>

---
v1: fault in region by just accessing it instead of print it
---
 MAINTAINERS                               |   1 +
 tools/testing/selftests/mm/.gitignore     |   1 +
 tools/testing/selftests/mm/Makefile       |   3 +
 tools/testing/selftests/mm/rmap.c         | 470 ++++++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh |   4 +
 5 files changed, 479 insertions(+)
 create mode 100644 tools/testing/selftests/mm/rmap.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 70df1d8cfaf5..5fb62a275c37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15917,6 +15917,7 @@ S:	Maintained
 F:	include/linux/rmap.h
 F:	mm/page_vma_mapped.c
 F:	mm/rmap.c
+F:	tools/testing/selftests/mm/rmap.c
 
 MEMORY MANAGEMENT - SECRETMEM
 M:	Andrew Morton <akpm@linux-foundation.org>
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
index f2dafa0b700b..c1f3de607ba2 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -57,3 +57,4 @@ pkey_sighandler_tests_32
 pkey_sighandler_tests_64
 guard-regions
 merge
+rmap
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index ae6f994d3add..c7e3a19b5555 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -100,6 +100,7 @@ TEST_GEN_FILES += hugetlb_dio
 TEST_GEN_FILES += droppable
 TEST_GEN_FILES += guard-regions
 TEST_GEN_FILES += merge
+TEST_GEN_FILES += rmap
 
 ifneq ($(ARCH),arm64)
 TEST_GEN_FILES += soft-dirty
@@ -227,6 +228,8 @@ $(OUTPUT)/ksm_tests: LDLIBS += -lnuma
 
 $(OUTPUT)/migration: LDLIBS += -lnuma
 
+$(OUTPUT)/rmap: LDLIBS += -lnuma
+
 local_config.mk local_config.h: check_config.sh
 	/bin/sh ./check_config.sh $(CC)
 
diff --git a/tools/testing/selftests/mm/rmap.c b/tools/testing/selftests/mm/rmap.c
new file mode 100644
index 000000000000..e05cd8a9c1bc
--- /dev/null
+++ b/tools/testing/selftests/mm/rmap.c
@@ -0,0 +1,470 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RMAP functional tests
+ *
+ * Author(s): Wei Yang <richard.weiyang@gmail.com>
+ */
+
+#include "../kselftest_harness.h"
+#include <strings.h>
+#include <pthread.h>
+#include <numa.h>
+#include <numaif.h>
+#include <sys/mman.h>
+#include <sys/prctl.h>
+#include <sys/types.h>
+#include <signal.h>
+#include <time.h>
+#include <sys/sem.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "vm_util.h"
+
+#define TOTAL_LEVEL 5
+#define MAX_CHILDREN 3
+
+#define FAIL_ON_CHECK	(1 << 0)
+#define FAIL_ON_WORK	(1 << 1)
+
+struct sembuf sem_wait = {0, -1, 0};
+struct sembuf sem_signal = {0, 1, 0};
+
+static const char initial_data[] = "Hello, world 0!";
+static const char updated_data[] = "Hello, World 0!";
+
+enum backend_type {
+	ANON,
+	SHM,
+	NORM_FILE,
+};
+
+#define PREFIX "kst_rmap"
+#define MAX_FILENAME_LEN 256
+const char *suffixes[] = {
+	"",
+	"_shm",
+	"_norm",
+};
+
+struct global_data;
+typedef int (*work_fn)(struct global_data *data);
+typedef int (*check_fn)(struct global_data *data);
+typedef void (*prepare_fn)(struct global_data *data);
+
+struct global_data {
+	int worker_level;
+
+	int semid;
+	int pipefd[2];
+
+	unsigned int mapsize;
+	unsigned int rand_seed;
+	char *region;
+
+	prepare_fn do_prepare;
+	work_fn do_work;
+	check_fn do_check;
+
+	enum backend_type backend;
+	char filename[MAX_FILENAME_LEN];
+
+	unsigned long *expected_pfn;
+};
+
+/*
+ * Create a process tree with TOTAL_LEVEL height and at most MAX_CHILDREN
+ * children for each.
+ *
+ * It will randomly select one process as 'worker' process which will
+ * 'do_work' until all processes are created. And all other processes will
+ * wait until 'worker' finish its work.
+ */
+int propagate_children(struct global_data *data)
+{
+	pid_t pid;
+	unsigned int num_child;
+	int status;
+	int ret = 0;
+	int curr_child, worker_child;
+	int curr_level = 1;
+	bool is_worker = true;
+
+repeat:
+	num_child = rand_r(&data->rand_seed) % MAX_CHILDREN + 1;
+	worker_child = is_worker ? rand_r(&data->rand_seed) % num_child : -1;
+
+	for (curr_child = 0; curr_child < num_child; curr_child++) {
+		pid = fork();
+
+		if (pid < 0) {
+			perror("Error: fork\n");
+		} else if (pid == 0) {
+			curr_level++;
+
+			if (curr_child != worker_child)
+				is_worker = false;
+
+			if (curr_level == TOTAL_LEVEL)
+				break;
+
+			data->rand_seed += curr_child;
+			goto repeat;
+		}
+	}
+
+	if (data->do_prepare)
+		data->do_prepare(data);
+
+	close(data->pipefd[1]);
+
+	if (is_worker && curr_level == data->worker_level) {
+		/* This is the worker process, first wait last process created */
+		char buf;
+
+		while (read(data->pipefd[0], &buf, 1) > 0)
+			;
+
+		if (data->do_work)
+			ret = data->do_work(data);
+
+		/* Kick others */
+		semctl(data->semid, 0, IPC_RMID);
+	} else {
+		/* Wait worker finish */
+		semop(data->semid, &sem_wait, 1);
+		if (data->do_check)
+			ret = data->do_check(data);
+	}
+
+	/* Wait all child to quit */
+	while (wait(&status) > 0) {
+		if (WIFEXITED(status))
+			ret |= WEXITSTATUS(status);
+	}
+
+	return ret;
+}
+
+FIXTURE(migrate)
+{
+	struct global_data data;
+};
+
+FIXTURE_SETUP(migrate)
+{
+	struct global_data *data = &self->data;
+
+	ASSERT_EQ(numa_available(), 0);
+	if (numa_bitmask_weight(numa_all_nodes_ptr) <= 1)
+		SKIP(return, "Not enough NUMA nodes available");
+
+	data->mapsize = getpagesize();
+
+	/* Prepare semaphore */
+	data->semid = semget(IPC_PRIVATE, 1, 0666 | IPC_CREAT);
+	ASSERT_NE(data->semid, -1);
+	ASSERT_NE(semctl(data->semid, 0, SETVAL, 0), -1);
+
+	/* Prepare pipe */
+	ASSERT_NE(pipe(data->pipefd), -1);
+
+	data->rand_seed = time(NULL);
+	srand(data->rand_seed);
+
+	data->worker_level = rand() % TOTAL_LEVEL + 1;
+
+	data->do_prepare = NULL;
+	data->do_work = NULL;
+	data->do_check = NULL;
+
+	data->backend = ANON;
+};
+
+FIXTURE_TEARDOWN(migrate)
+{
+	struct global_data *data = &self->data;
+
+	if (data->region != MAP_FAILED)
+		munmap(data->region, data->mapsize);
+	data->region = MAP_FAILED;
+	if (data->expected_pfn != MAP_FAILED)
+		munmap(data->expected_pfn, sizeof(unsigned long));
+	data->expected_pfn = MAP_FAILED;
+	semctl(data->semid, 0, IPC_RMID);
+	data->semid = -1;
+
+	close(data->pipefd[0]);
+
+	data->do_work = NULL;
+	data->do_check = NULL;
+
+	switch (data->backend) {
+	case ANON:
+		break;
+	case SHM:
+		shm_unlink(data->filename);
+		break;
+	case NORM_FILE:
+		unlink(data->filename);
+		break;
+	}
+}
+
+int try_to_move_page(char *region)
+{
+	int ret;
+	int node;
+	int status = 0;
+	volatile unsigned long dummy = 0;
+
+	/*
+	 * Fault in page in case it is not, otherwise move_pages() would
+	 * return -ENOENT.
+	 */
+	dummy = *((unsigned long *)region);
+	/* Prevent the compiler from optimizing out the entire loop: */
+	asm volatile("" : "+r" (dummy));
+
+	ret = move_pages(0, 1, (void **)&region, NULL, &status, MPOL_MF_MOVE_ALL);
+	if (ret != 0)
+		return FAIL_ON_WORK;
+
+	/* Pick up a different target node */
+	for (node = 0; node <= numa_max_node(); node++) {
+		if (numa_bitmask_isbitset(numa_all_nodes_ptr, node) && node != status)
+			break;
+	}
+
+	if (node > numa_max_node()) {
+		ksft_print_msg("Couldn't find available numa node for testing\n");
+		return FAIL_ON_WORK;
+	}
+
+	ret = move_pages(0, 1, (void **)&region, &node, &status, MPOL_MF_MOVE_ALL);
+	if (ret != 0)
+		return FAIL_ON_WORK;
+
+	return 0;
+}
+
+int move_and_update(struct global_data *data)
+{
+	int ret;
+
+	ret = try_to_move_page(data->region);
+	if (ret != 0)
+		return ret;
+
+	/* Change the content */
+	strcpy(data->region, updated_data);
+
+	return ret;
+}
+
+int data_updated(struct global_data *data)
+{
+	if (data->region == MAP_FAILED)
+		return 0;
+
+	if (strncmp((char *)data->region, updated_data, strlen(updated_data)))
+		return FAIL_ON_CHECK;
+	return 0;
+}
+
+TEST_F(migrate, anon)
+{
+	pid_t root_pid;
+	int ret;
+	struct global_data *data = &self->data;
+
+	/* Map a shared area and fault in */
+	data->region = mmap(0, data->mapsize, PROT_READ | PROT_WRITE,
+				MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	ASSERT_NE(data->region, MAP_FAILED);
+	strcpy(data->region, initial_data);
+
+	data->do_work = move_and_update;
+	data->do_check = data_updated;
+
+	root_pid = getpid();
+
+	ret = propagate_children(data);
+
+	if (getpid() == root_pid) {
+		if (ret & FAIL_ON_WORK)
+			SKIP(return, "Failed on moving page");
+
+		ASSERT_EQ(ret, 0);
+	} else {
+		exit(ret);
+	}
+}
+
+TEST_F(migrate, shm)
+{
+	pid_t root_pid;
+	int ret;
+	int shm_fd;
+	struct global_data *data = &self->data;
+
+	snprintf(data->filename, MAX_FILENAME_LEN, "%s%s", PREFIX, suffixes[SHM]);
+	shm_fd = shm_open(data->filename, O_CREAT | O_RDWR, 0666);
+	ASSERT_NE(shm_fd, -1);
+	ftruncate(shm_fd, data->mapsize);
+	data->backend = SHM;
+
+	/* Map a shared area and fault in */
+	data->region = mmap(0, data->mapsize, PROT_READ | PROT_WRITE,
+				MAP_SHARED, shm_fd, 0);
+	ASSERT_NE(data->region, MAP_FAILED);
+	strcpy(data->region, initial_data);
+	close(shm_fd);
+
+	data->do_work = move_and_update;
+	data->do_check = data_updated;
+
+	root_pid = getpid();
+
+	ret = propagate_children(data);
+
+	if (getpid() == root_pid) {
+		if (ret & FAIL_ON_WORK)
+			SKIP(return, "Failed on moving page");
+
+		ASSERT_EQ(ret, 0);
+	} else {
+		exit(ret);
+	}
+}
+
+TEST_F(migrate, file)
+{
+	pid_t root_pid;
+	int ret;
+	int fd;
+	struct global_data *data = &self->data;
+
+	snprintf(data->filename, MAX_FILENAME_LEN, "%s%s", PREFIX, suffixes[NORM_FILE]);
+	fd = open(data->filename, O_CREAT | O_RDWR | O_EXCL, 0666);
+	ASSERT_NE(fd, -1);
+	ftruncate(fd, data->mapsize);
+	data->backend = NORM_FILE;
+
+	/* Map a shared area and fault in */
+	data->region = mmap(0, data->mapsize, PROT_READ | PROT_WRITE,
+				MAP_SHARED, fd, 0);
+	ASSERT_NE(data->region, MAP_FAILED);
+	strcpy(data->region, initial_data);
+	close(fd);
+
+	data->do_work = move_and_update;
+	data->do_check = data_updated;
+
+	root_pid = getpid();
+
+	ret = propagate_children(data);
+
+	if (getpid() == root_pid) {
+		if (ret & FAIL_ON_WORK)
+			SKIP(return, "Failed on moving page");
+
+		ASSERT_EQ(ret, 0);
+	} else {
+		exit(ret);
+	}
+}
+
+void prepare_local_region(struct global_data *data)
+{
+	/* Allocate range and set the same data */
+	data->region = mmap(NULL, data->mapsize, PROT_READ|PROT_WRITE,
+			   MAP_PRIVATE|MAP_ANON, -1, 0);
+	if (data->region == MAP_FAILED)
+		return;
+
+	memset(data->region, 0xcf, data->mapsize);
+}
+
+int merge_and_migrate(struct global_data *data)
+{
+	long start_scans, end_scans;
+	int pagemap_fd;
+	int ret = 0;
+
+	if (data->region == MAP_FAILED)
+		return FAIL_ON_WORK;
+
+	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
+	if (pagemap_fd == -1)
+		return FAIL_ON_WORK;
+
+	/* Wait for three full scans such that any possible merging happened. */
+	start_scans = ksm_get_full_scans();
+	if (start_scans < 0)
+		return FAIL_ON_WORK;
+	if (ksm_start_and_merge())
+		return FAIL_ON_WORK;
+	do {
+		end_scans = ksm_get_full_scans();
+		if (end_scans < 0)
+			return FAIL_ON_WORK;
+	} while (end_scans < start_scans + 3);
+
+	ret = try_to_move_page(data->region);
+
+	*data->expected_pfn = pagemap_get_pfn(pagemap_fd, data->region);
+
+	return ret;
+}
+
+int has_same_pfn(struct global_data *data)
+{
+	unsigned long pfn;
+	int pagemap_fd;
+
+	if (data->region == MAP_FAILED)
+		return 0;
+
+	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
+	if (pagemap_fd == -1)
+		return FAIL_ON_CHECK;
+
+	pfn = pagemap_get_pfn(pagemap_fd, data->region);
+	if (pfn != *data->expected_pfn)
+		return FAIL_ON_CHECK;
+
+	return 0;
+}
+
+TEST_F(migrate, ksm)
+{
+	pid_t root_pid;
+	int ret;
+	struct global_data *data = &self->data;
+
+	ASSERT_EQ(prctl(PR_SET_MEMORY_MERGE, 1, 0, 0, 0), 0);
+	data->do_prepare = prepare_local_region;
+	data->do_work = merge_and_migrate;
+	data->do_check = has_same_pfn;
+
+	data->expected_pfn = mmap(0, sizeof(unsigned long),
+				PROT_READ | PROT_WRITE,
+				MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	ASSERT_NE(data->expected_pfn, MAP_FAILED);
+
+	root_pid = getpid();
+
+	ret = propagate_children(data);
+
+	if (getpid() == root_pid) {
+		if (ret & FAIL_ON_WORK)
+			SKIP(return, "Failed in worker");
+
+		ASSERT_EQ(ret, 0);
+	} else {
+		exit(ret);
+	}
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index a38c984103ce..f6fb8dec6e64 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -83,6 +83,8 @@ separated by spaces:
 	test handling of page fragment allocation and freeing
 - vma_merge
 	test VMA merge cases behave as expected
+- rmap
+	test rmap behave as expected
 
 example: ./run_vmtests.sh -t "hmm mmap ksm"
 EOF
@@ -527,6 +529,8 @@ CATEGORY="page_frag" run_test ./test_page_frag.sh aligned
 
 CATEGORY="page_frag" run_test ./test_page_frag.sh nonaligned
 
+CATEGORY="rmap" run_test ./rmap
+
 echo "SUMMARY: PASS=${count_pass} SKIP=${count_skip} FAIL=${count_fail}" | tap_prefix
 echo "1..${count_total}" | tap_output
 
-- 
2.34.1

Re: [PATCH 3/3] selftests/mm: assert rmap behave as expected

[David Hildenbrand]

On 16.07.25 10:27, Wei Yang wrote:
> As David suggested, currently we don't have a high level test case to
> verify the behavior of rmap. This patch introduce the verification on
> rmap by migration.
> 
> The general idea is if migrate one shared page between processes, this
> would be reflected in all related processes. Otherwise, we have problem
> in rmap.
> 
> Currently it covers following four scenarios:
> 
>    * anonymous page
>    * shmem page
>    * pagecache page
>    * ksm page
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> 
> ---
> v1: fault in region by just accessing it instead of print it
> ---
>   MAINTAINERS                               |   1 +
>   tools/testing/selftests/mm/.gitignore     |   1 +
>   tools/testing/selftests/mm/Makefile       |   3 +
>   tools/testing/selftests/mm/rmap.c         | 470 ++++++++++++++++++++++
>   tools/testing/selftests/mm/run_vmtests.sh |   4 +
>   5 files changed, 479 insertions(+)
>   create mode 100644 tools/testing/selftests/mm/rmap.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 70df1d8cfaf5..5fb62a275c37 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15917,6 +15917,7 @@ S:	Maintained
>   F:	include/linux/rmap.h
>   F:	mm/page_vma_mapped.c
>   F:	mm/rmap.c
> +F:	tools/testing/selftests/mm/rmap.c
>   
>   MEMORY MANAGEMENT - SECRETMEM
>   M:	Andrew Morton <akpm@linux-foundation.org>
> diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
> index f2dafa0b700b..c1f3de607ba2 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -57,3 +57,4 @@ pkey_sighandler_tests_32
>   pkey_sighandler_tests_64
>   guard-regions
>   merge
> +rmap
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index ae6f994d3add..c7e3a19b5555 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -100,6 +100,7 @@ TEST_GEN_FILES += hugetlb_dio
>   TEST_GEN_FILES += droppable
>   TEST_GEN_FILES += guard-regions
>   TEST_GEN_FILES += merge
> +TEST_GEN_FILES += rmap
>   
>   ifneq ($(ARCH),arm64)
>   TEST_GEN_FILES += soft-dirty
> @@ -227,6 +228,8 @@ $(OUTPUT)/ksm_tests: LDLIBS += -lnuma
>   
>   $(OUTPUT)/migration: LDLIBS += -lnuma
>   
> +$(OUTPUT)/rmap: LDLIBS += -lnuma
> +
>   local_config.mk local_config.h: check_config.sh
>   	/bin/sh ./check_config.sh $(CC)
>   
> diff --git a/tools/testing/selftests/mm/rmap.c b/tools/testing/selftests/mm/rmap.c
> new file mode 100644
> index 000000000000..e05cd8a9c1bc
> --- /dev/null
> +++ b/tools/testing/selftests/mm/rmap.c
> @@ -0,0 +1,470 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RMAP functional tests
> + *
> + * Author(s): Wei Yang <richard.weiyang@gmail.com>
> + */
> +
> +#include "../kselftest_harness.h"
> +#include <strings.h>
> +#include <pthread.h>
> +#include <numa.h>
> +#include <numaif.h>
> +#include <sys/mman.h>
> +#include <sys/prctl.h>
> +#include <sys/types.h>
> +#include <signal.h>
> +#include <time.h>
> +#include <sys/sem.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +
> +#include "vm_util.h"
> +
> +#define TOTAL_LEVEL 5
> +#define MAX_CHILDREN 3
> +
> +#define FAIL_ON_CHECK	(1 << 0)
> +#define FAIL_ON_WORK	(1 << 1)
> +
> +struct sembuf sem_wait = {0, -1, 0};
> +struct sembuf sem_signal = {0, 1, 0};
> +
> +static const char initial_data[] = "Hello, world 0!";
> +static const char updated_data[] = "Hello, World 0!";
> +
> +enum backend_type {
> +	ANON,
> +	SHM,
> +	NORM_FILE,
> +};
> +
> +#define PREFIX "kst_rmap"
> +#define MAX_FILENAME_LEN 256
> +const char *suffixes[] = {
> +	"",
> +	"_shm",
> +	"_norm",
> +};
> +
> +struct global_data;
> +typedef int (*work_fn)(struct global_data *data);
> +typedef int (*check_fn)(struct global_data *data);
> +typedef void (*prepare_fn)(struct global_data *data);
> +
> +struct global_data {
> +	int worker_level;
> +
> +	int semid;
> +	int pipefd[2];
> +
> +	unsigned int mapsize;
> +	unsigned int rand_seed;
> +	char *region;
> +
> +	prepare_fn do_prepare;
> +	work_fn do_work;
> +	check_fn do_check;
> +
> +	enum backend_type backend;
> +	char filename[MAX_FILENAME_LEN];
> +
> +	unsigned long *expected_pfn;
> +};
> +
> +/*
> + * Create a process tree with TOTAL_LEVEL height and at most MAX_CHILDREN
> + * children for each.
> + *
> + * It will randomly select one process as 'worker' process which will
> + * 'do_work' until all processes are created. And all other processes will
> + * wait until 'worker' finish its work.
> + */
> +int propagate_children(struct global_data *data)
> +{
> +	pid_t pid;
> +	unsigned int num_child;
> +	int status;
> +	int ret = 0;
> +	int curr_child, worker_child;
> +	int curr_level = 1;
> +	bool is_worker = true;
> +
> +repeat:
> +	num_child = rand_r(&data->rand_seed) % MAX_CHILDREN + 1;
> +	worker_child = is_worker ? rand_r(&data->rand_seed) % num_child : -1;
> +
> +	for (curr_child = 0; curr_child < num_child; curr_child++) {
> +		pid = fork();
> +
> +		if (pid < 0) {
> +			perror("Error: fork\n");
> +		} else if (pid == 0) {
> +			curr_level++;
> +
> +			if (curr_child != worker_child)
> +				is_worker = false;
> +
> +			if (curr_level == TOTAL_LEVEL)
> +				break;
> +
> +			data->rand_seed += curr_child;
> +			goto repeat;
> +		}
> +	}
> +
> +	if (data->do_prepare)
> +		data->do_prepare(data);
> +
> +	close(data->pipefd[1]);
> +
> +	if (is_worker && curr_level == data->worker_level) {
> +		/* This is the worker process, first wait last process created */
> +		char buf;
> +
> +		while (read(data->pipefd[0], &buf, 1) > 0)
> +			;
> +
> +		if (data->do_work)
> +			ret = data->do_work(data);
> +
> +		/* Kick others */
> +		semctl(data->semid, 0, IPC_RMID);
> +	} else {
> +		/* Wait worker finish */
> +		semop(data->semid, &sem_wait, 1);
> +		if (data->do_check)
> +			ret = data->do_check(data);
> +	}
> +
> +	/* Wait all child to quit */
> +	while (wait(&status) > 0) {
> +		if (WIFEXITED(status))
> +			ret |= WEXITSTATUS(status);
> +	}
> +
> +	return ret;
> +}
> +
> +FIXTURE(migrate)
> +{
> +	struct global_data data;
> +};
> +
> +FIXTURE_SETUP(migrate)
> +{
> +	struct global_data *data = &self->data;
> +
> +	ASSERT_EQ(numa_available(), 0);

if (numa_available() < 0)
	SKIP(return, "NUMA not available");

Should that be a skip instead?

> +	if (numa_bitmask_weight(numa_all_nodes_ptr) <= 1)
> +		SKIP(return, "Not enough NUMA nodes available");
> +
 > +	data->mapsize = getpagesize();> +
> +	/* Prepare semaphore */
> +	data->semid = semget(IPC_PRIVATE, 1, 0666 | IPC_CREAT);
> +	ASSERT_NE(data->semid, -1);
> +	ASSERT_NE(semctl(data->semid, 0, SETVAL, 0), -1);
> +
> +	/* Prepare pipe */
> +	ASSERT_NE(pipe(data->pipefd), -1);
> +
> +	data->rand_seed = time(NULL);
> +	srand(data->rand_seed);
> +
> +	data->worker_level = rand() % TOTAL_LEVEL + 1;
> +
> +	data->do_prepare = NULL;
> +	data->do_work = NULL;
> +	data->do_check = NULL;
> +
> +	data->backend = ANON;
> +};
> +
> +FIXTURE_TEARDOWN(migrate)
> +{
> +	struct global_data *data = &self->data;
> +
> +	if (data->region != MAP_FAILED)
> +		munmap(data->region, data->mapsize);
> +	data->region = MAP_FAILED;
> +	if (data->expected_pfn != MAP_FAILED)
> +		munmap(data->expected_pfn, sizeof(unsigned long));
> +	data->expected_pfn = MAP_FAILED;
> +	semctl(data->semid, 0, IPC_RMID);
> +	data->semid = -1;
> +
> +	close(data->pipefd[0]);
> +
> +	data->do_work = NULL;
> +	data->do_check = NULL;
> +
> +	switch (data->backend) {
> +	case ANON:
> +		break;
> +	case SHM:
> +		shm_unlink(data->filename);
> +		break;
> +	case NORM_FILE:
> +		unlink(data->filename);
> +		break;
> +	}
> +}
> +
> +int try_to_move_page(char *region)
> +{
> +	int ret;
> +	int node;
> +	int status = 0;
> +	volatile unsigned long dummy = 0;
> +
> +	/*
> +	 * Fault in page in case it is not, otherwise move_pages() would
> +	 * return -ENOENT.
> +	 */
> +	dummy = *((unsigned long *)region);

Use FORCE_READ() here

https://lkml.kernel.org/r/20250716123126.3851-1-lianux.mm@gmail.com

But, this really must happen in all children before actually performing 
the move in the worker. Otherwise the other processes don't map the page 
and will just ... fault it in later.

> +	/* Prevent the compiler from optimizing out the entire loop: */
> +	asm volatile("" : "+r" (dummy));
> +
> +	ret = move_pages(0, 1, (void **)&region, NULL, &status, MPOL_MF_MOVE_ALL);
> +	if (ret != 0)
> +		return FAIL_ON_WORK;
> +
> +	/* Pick up a different target node */
> +	for (node = 0; node <= numa_max_node(); node++) {
> +		if (numa_bitmask_isbitset(numa_all_nodes_ptr, node) && node != status)
> +			break;
> +	}
> +
> +	if (node > numa_max_node()) {
> +		ksft_print_msg("Couldn't find available numa node for testing\n");
> +		return FAIL_ON_WORK;
> +	}
> +
> +	ret = move_pages(0, 1, (void **)&region, &node, &status, MPOL_MF_MOVE_ALL);
> +	if (ret != 0)
> +		return FAIL_ON_WORK;

Probably, if we don't manage to migrate, we should retry a couple of 
times and then SKIP.

Point is, migration might fail for various reasons (e.g., 2 NUMA nodes 
but one of them doesn't even have memory) etc.

Migration failures might indicate other problems, yes, but false 
failures from the test are suboptimal.

> +
> +	return 0;
> +}
> +
> +int move_and_update(struct global_data *data)
> +{
> +	int ret;
> +
> +	ret = try_to_move_page(data->region);
> +	if (ret != 0)
> +		return ret;
> +
> +	/* Change the content */
> +	strcpy(data->region, updated_data);
> +
> +	return ret;
> +}
> +
> +int data_updated(struct global_data *data)
> +{
> +	if (data->region == MAP_FAILED)
> +		return 0;
> +
> +	if (strncmp((char *)data->region, updated_data, strlen(updated_data)))
> +		return FAIL_ON_CHECK;
> +	return 0;
> +}

I assume checking the PFN is sufficient. No need for the additional data 
content. In particular, with proper anon pages (CoW, see below) that 
doesn't work either way.

> +
> +TEST_F(migrate, anon)
> +{
> +	pid_t root_pid;
> +	int ret;
> +	struct global_data *data = &self->data;
> +
> +	/* Map a shared area and fault in */
> +	data->region = mmap(0, data->mapsize, PROT_READ | PROT_WRITE,
> +				MAP_SHARED | MAP_ANONYMOUS, -1, 0);

That is anon_shmem. We should test proper anon memory (MAP_PRIVATE), 
whereby pages are shared using CoW.

anon_shmem should behave mostly like shmem.

> +	ASSERT_NE(data->region, MAP_FAILED);
> +	strcpy(data->region, initial_data);
> +
> +	data->do_work = move_and_update;
> +	data->do_check = data_updated;
> +
> +	root_pid = getpid();
> +
> +	ret = propagate_children(data);
> +
> +	if (getpid() == root_pid) {
> +		if (ret & FAIL_ON_WORK)
> +			SKIP(return, "Failed on moving page");
> +
> +		ASSERT_EQ(ret, 0);
> +	} else {
> +		exit(ret);
> +	}
> +}

[...]

> +TEST_HARNESS_MAIN
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index a38c984103ce..f6fb8dec6e64 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -83,6 +83,8 @@ separated by spaces:
>   	test handling of page fragment allocation and freeing
>   - vma_merge
>   	test VMA merge cases behave as expected
> +- rmap
> +	test rmap behave as expected

"behaves"

-- 
Cheers,

David / dhildenb

Re: [PATCH 3/3] selftests/mm: assert rmap behave as expected

[Wei Yang]

On Wed, Jul 16, 2025 at 03:34:11PM +0200, David Hildenbrand wrote:
>On 16.07.25 10:27, Wei Yang wrote:
[..]
>> +
>> +FIXTURE(migrate)
>> +{
>> +	struct global_data data;
>> +};
>> +
>> +FIXTURE_SETUP(migrate)
>> +{
>> +	struct global_data *data = &self->data;
>> +
>> +	ASSERT_EQ(numa_available(), 0);
>
>if (numa_available() < 0)
>	SKIP(return, "NUMA not available");
>
>Should that be a skip instead?
>

You are right, will fix it.

>> +	if (numa_bitmask_weight(numa_all_nodes_ptr) <= 1)
>> +		SKIP(return, "Not enough NUMA nodes available");
>> +
>> +	data->mapsize = getpagesize();> +
>> +	/* Prepare semaphore */
>> +	data->semid = semget(IPC_PRIVATE, 1, 0666 | IPC_CREAT);
>> +	ASSERT_NE(data->semid, -1);
>> +	ASSERT_NE(semctl(data->semid, 0, SETVAL, 0), -1);
>> +
>> +	/* Prepare pipe */
>> +	ASSERT_NE(pipe(data->pipefd), -1);
>> +
>> +	data->rand_seed = time(NULL);
>> +	srand(data->rand_seed);
>> +
>> +	data->worker_level = rand() % TOTAL_LEVEL + 1;
>> +
>> +	data->do_prepare = NULL;
>> +	data->do_work = NULL;
>> +	data->do_check = NULL;
>> +
>> +	data->backend = ANON;
>> +};
>> +
>> +FIXTURE_TEARDOWN(migrate)
>> +{
>> +	struct global_data *data = &self->data;
>> +
>> +	if (data->region != MAP_FAILED)
>> +		munmap(data->region, data->mapsize);
>> +	data->region = MAP_FAILED;
>> +	if (data->expected_pfn != MAP_FAILED)
>> +		munmap(data->expected_pfn, sizeof(unsigned long));
>> +	data->expected_pfn = MAP_FAILED;
>> +	semctl(data->semid, 0, IPC_RMID);
>> +	data->semid = -1;
>> +
>> +	close(data->pipefd[0]);
>> +
>> +	data->do_work = NULL;
>> +	data->do_check = NULL;
>> +
>> +	switch (data->backend) {
>> +	case ANON:
>> +		break;
>> +	case SHM:
>> +		shm_unlink(data->filename);
>> +		break;
>> +	case NORM_FILE:
>> +		unlink(data->filename);
>> +		break;
>> +	}
>> +}
>> +
>> +int try_to_move_page(char *region)
>> +{
>> +	int ret;
>> +	int node;
>> +	int status = 0;
>> +	volatile unsigned long dummy = 0;
>> +
>> +	/*
>> +	 * Fault in page in case it is not, otherwise move_pages() would
>> +	 * return -ENOENT.
>> +	 */
>> +	dummy = *((unsigned long *)region);
>
>Use FORCE_READ() here
>
>https://lkml.kernel.org/r/20250716123126.3851-1-lianux.mm@gmail.com
>
>But, this really must happen in all children before actually performing the
>move in the worker. Otherwise the other processes don't map the page and will
>just ... fault it in later.
>

Ok, will access the region in all child before migrate.

>> +	/* Prevent the compiler from optimizing out the entire loop: */
>> +	asm volatile("" : "+r" (dummy));
>> +
>> +	ret = move_pages(0, 1, (void **)&region, NULL, &status, MPOL_MF_MOVE_ALL);
>> +	if (ret != 0)
>> +		return FAIL_ON_WORK;
>> +
>> +	/* Pick up a different target node */
>> +	for (node = 0; node <= numa_max_node(); node++) {
>> +		if (numa_bitmask_isbitset(numa_all_nodes_ptr, node) && node != status)
>> +			break;
>> +	}
>> +
>> +	if (node > numa_max_node()) {
>> +		ksft_print_msg("Couldn't find available numa node for testing\n");
>> +		return FAIL_ON_WORK;
>> +	}
>> +
>> +	ret = move_pages(0, 1, (void **)&region, &node, &status, MPOL_MF_MOVE_ALL);
>> +	if (ret != 0)
>> +		return FAIL_ON_WORK;
>
>Probably, if we don't manage to migrate, we should retry a couple of times
>and then SKIP.
>
>Point is, migration might fail for various reasons (e.g., 2 NUMA nodes but
>one of them doesn't even have memory) etc.
>
>Migration failures might indicate other problems, yes, but false failures
>from the test are suboptimal.
>

Will add retry logic.

>> +
>> +	return 0;
>> +}
>> +
>> +int move_and_update(struct global_data *data)
>> +{
>> +	int ret;
>> +
>> +	ret = try_to_move_page(data->region);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	/* Change the content */
>> +	strcpy(data->region, updated_data);
>> +
>> +	return ret;
>> +}
>> +
>> +int data_updated(struct global_data *data)
>> +{
>> +	if (data->region == MAP_FAILED)
>> +		return 0;
>> +
>> +	if (strncmp((char *)data->region, updated_data, strlen(updated_data)))
>> +		return FAIL_ON_CHECK;
>> +	return 0;
>> +}
>
>I assume checking the PFN is sufficient. No need for the additional data
>content. In particular, with proper anon pages (CoW, see below) that doesn't
>work either way.
>
>> +
>> +TEST_F(migrate, anon)
>> +{
>> +	pid_t root_pid;
>> +	int ret;
>> +	struct global_data *data = &self->data;
>> +
>> +	/* Map a shared area and fault in */
>> +	data->region = mmap(0, data->mapsize, PROT_READ | PROT_WRITE,
>> +				MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>
>That is anon_shmem. We should test proper anon memory (MAP_PRIVATE), whereby
>pages are shared using CoW.
>

So the case should be 

  * mapping (MAP_PRIVATE | MAP_ANONYMOUS)
  * write some content in root parent
  * fault in for each child
  * do migration and record pfn
  * then check pfn is the same in each child

>anon_shmem should behave mostly like shmem.
>
>> +	ASSERT_NE(data->region, MAP_FAILED);
>> +	strcpy(data->region, initial_data);
>> +
>> +	data->do_work = move_and_update;
>> +	data->do_check = data_updated;
>> +
>> +	root_pid = getpid();
>> +
>> +	ret = propagate_children(data);
>> +
>> +	if (getpid() == root_pid) {
>> +		if (ret & FAIL_ON_WORK)
>> +			SKIP(return, "Failed on moving page");
>> +
>> +		ASSERT_EQ(ret, 0);
>> +	} else {
>> +		exit(ret);
>> +	}
>> +}
>
>[...]
>
>> +TEST_HARNESS_MAIN
>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>> index a38c984103ce..f6fb8dec6e64 100755
>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>> @@ -83,6 +83,8 @@ separated by spaces:
>>   	test handling of page fragment allocation and freeing
>>   - vma_merge
>>   	test VMA merge cases behave as expected
>> +- rmap
>> +	test rmap behave as expected
>
>"behaves"
>
>-- 
>Cheers,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me

Re: [PATCH 3/3] selftests/mm: assert rmap behave as expected

[Wei Yang]

On Thu, Jul 17, 2025 at 03:17:48AM +0000, Wei Yang wrote:
>On Wed, Jul 16, 2025 at 03:34:11PM +0200, David Hildenbrand wrote:
>>On 16.07.25 10:27, Wei Yang wrote:
>[..]
>>> +
>>> +FIXTURE(migrate)
>>> +{
>>> +	struct global_data data;
>>> +};
>>> +
>>> +FIXTURE_SETUP(migrate)
>>> +{
>>> +	struct global_data *data = &self->data;
>>> +
>>> +	ASSERT_EQ(numa_available(), 0);
>>
>>if (numa_available() < 0)
>>	SKIP(return, "NUMA not available");
>>
>>Should that be a skip instead?
>>
>
>You are right, will fix it.
>
>>> +	if (numa_bitmask_weight(numa_all_nodes_ptr) <= 1)
>>> +		SKIP(return, "Not enough NUMA nodes available");
>>> +
>>> +	data->mapsize = getpagesize();> +
>>> +	/* Prepare semaphore */
>>> +	data->semid = semget(IPC_PRIVATE, 1, 0666 | IPC_CREAT);
>>> +	ASSERT_NE(data->semid, -1);
>>> +	ASSERT_NE(semctl(data->semid, 0, SETVAL, 0), -1);
>>> +
>>> +	/* Prepare pipe */
>>> +	ASSERT_NE(pipe(data->pipefd), -1);
>>> +
>>> +	data->rand_seed = time(NULL);
>>> +	srand(data->rand_seed);
>>> +
>>> +	data->worker_level = rand() % TOTAL_LEVEL + 1;
>>> +
>>> +	data->do_prepare = NULL;
>>> +	data->do_work = NULL;
>>> +	data->do_check = NULL;
>>> +
>>> +	data->backend = ANON;
>>> +};
>>> +
>>> +FIXTURE_TEARDOWN(migrate)
>>> +{
>>> +	struct global_data *data = &self->data;
>>> +
>>> +	if (data->region != MAP_FAILED)
>>> +		munmap(data->region, data->mapsize);
>>> +	data->region = MAP_FAILED;
>>> +	if (data->expected_pfn != MAP_FAILED)
>>> +		munmap(data->expected_pfn, sizeof(unsigned long));
>>> +	data->expected_pfn = MAP_FAILED;
>>> +	semctl(data->semid, 0, IPC_RMID);
>>> +	data->semid = -1;
>>> +
>>> +	close(data->pipefd[0]);
>>> +
>>> +	data->do_work = NULL;
>>> +	data->do_check = NULL;
>>> +
>>> +	switch (data->backend) {
>>> +	case ANON:
>>> +		break;
>>> +	case SHM:
>>> +		shm_unlink(data->filename);
>>> +		break;
>>> +	case NORM_FILE:
>>> +		unlink(data->filename);
>>> +		break;
>>> +	}
>>> +}
>>> +
>>> +int try_to_move_page(char *region)
>>> +{
>>> +	int ret;
>>> +	int node;
>>> +	int status = 0;
>>> +	volatile unsigned long dummy = 0;
>>> +
>>> +	/*
>>> +	 * Fault in page in case it is not, otherwise move_pages() would
>>> +	 * return -ENOENT.
>>> +	 */
>>> +	dummy = *((unsigned long *)region);
>>
>>Use FORCE_READ() here
>>
>>https://lkml.kernel.org/r/20250716123126.3851-1-lianux.mm@gmail.com
>>
>>But, this really must happen in all children before actually performing the
>>move in the worker. Otherwise the other processes don't map the page and will
>>just ... fault it in later.
>>
>
>Ok, will access the region in all child before migrate.
>
>>> +	/* Prevent the compiler from optimizing out the entire loop: */
>>> +	asm volatile("" : "+r" (dummy));
>>> +
>>> +	ret = move_pages(0, 1, (void **)&region, NULL, &status, MPOL_MF_MOVE_ALL);
>>> +	if (ret != 0)
>>> +		return FAIL_ON_WORK;
>>> +
>>> +	/* Pick up a different target node */
>>> +	for (node = 0; node <= numa_max_node(); node++) {
>>> +		if (numa_bitmask_isbitset(numa_all_nodes_ptr, node) && node != status)
>>> +			break;
>>> +	}
>>> +
>>> +	if (node > numa_max_node()) {
>>> +		ksft_print_msg("Couldn't find available numa node for testing\n");
>>> +		return FAIL_ON_WORK;
>>> +	}
>>> +
>>> +	ret = move_pages(0, 1, (void **)&region, &node, &status, MPOL_MF_MOVE_ALL);
>>> +	if (ret != 0)
>>> +		return FAIL_ON_WORK;
>>
>>Probably, if we don't manage to migrate, we should retry a couple of times
>>and then SKIP.
>>
>>Point is, migration might fail for various reasons (e.g., 2 NUMA nodes but
>>one of them doesn't even have memory) etc.
>>
>>Migration failures might indicate other problems, yes, but false failures
>>from the test are suboptimal.
>>
>
>Will add retry logic.
>
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +int move_and_update(struct global_data *data)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = try_to_move_page(data->region);
>>> +	if (ret != 0)
>>> +		return ret;
>>> +
>>> +	/* Change the content */
>>> +	strcpy(data->region, updated_data);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +int data_updated(struct global_data *data)
>>> +{
>>> +	if (data->region == MAP_FAILED)
>>> +		return 0;
>>> +
>>> +	if (strncmp((char *)data->region, updated_data, strlen(updated_data)))
>>> +		return FAIL_ON_CHECK;
>>> +	return 0;
>>> +}
>>
>>I assume checking the PFN is sufficient. No need for the additional data
>>content. In particular, with proper anon pages (CoW, see below) that doesn't
>>work either way.
>>
>>> +
>>> +TEST_F(migrate, anon)
>>> +{
>>> +	pid_t root_pid;
>>> +	int ret;
>>> +	struct global_data *data = &self->data;
>>> +
>>> +	/* Map a shared area and fault in */
>>> +	data->region = mmap(0, data->mapsize, PROT_READ | PROT_WRITE,
>>> +				MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>>
>>That is anon_shmem. We should test proper anon memory (MAP_PRIVATE), whereby
>>pages are shared using CoW.
>>
>
>So the case should be 
>
>  * mapping (MAP_PRIVATE | MAP_ANONYMOUS)
>  * write some content in root parent
>  * fault in for each child
>  * do migration and record pfn
>  * then check pfn is the same in each child
>

Hi, David

Is my understanding correct? If so, may I send a new version?

-- 
Wei Yang
Help you, Help me

Re: [PATCH 3/3] selftests/mm: assert rmap behave as expected

[David Hildenbrand]

Sorry,

>> So the case should be
>>
>>   * mapping (MAP_PRIVATE | MAP_ANONYMOUS)
>>   * write some content in root parent
>>   * fault in for each child

That is not required, but a FORCE_READ() would also not be harmful (if 
it would be done by your common code)

>>   * do migration and record pfn
>>   * then check pfn is the same in each child

Yes, makes sense.

A new version makes sense, so we can have a look what the new state 
looks like.

-- 
Cheers,

David / dhildenb