linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] mm/ksm: Fix incorrect accounting of KSM counters during fork.
@ 2025-09-15 15:03 Donet Tom
  2025-09-15 15:03 ` [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct " Donet Tom
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Donet Tom @ 2025-09-15 15:03 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze,
	Donet Tom

The first patch in this series fixes the incorrect accounting of KSM
counters such as ksm_merging_pages, ksm_rmap_items, and the global
ksm_zero_pages during fork.

The following two patches add selftests to verify that the
ksm_merging_pages counter and the global ksm_zero_pages counter are
updated correctly during fork.

Test Results
============
Without the first patch
-----------------------
# [RUN] test_fork_ksm_merging_page_count
not ok 10 ksm_merging_page in child: 32
# [RUN] test_fork_global_ksm_zero_pages_count
not ok 11 Incorrect global ksm zero page counter after fork

With the first patch
--------------------
# [RUN] test_fork_ksm_merging_page_count
ok 10 ksm_merging_pages is not inherited after fork
# [RUN] test_fork_global_ksm_zero_pages_count
ok 11 Global ksm zero page count is correct after fork

Changes:

v1 -> v2 :
 - Rebased to mm-new branch
 - Fixed incorrect global ksm_zero_pages counter issue during fork and
 added a selftest to verify it.

v1: https://lore.kernel.org/all/2e662107e01417bf9af23bc7f52863cd538419be.1756211338.git.donettom@linux.ibm.com/


Donet Tom (3):
  mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  selftests/mm: Added fork inheritance test for ksm_merging_pages
    counter
  selftests/mm: Added fork test to verify global ksm_zero_pages counter
    behavior

 include/linux/ksm.h                           |   8 +-
 .../selftests/mm/ksm_functional_tests.c       | 114 +++++++++++++++++-
 2 files changed, 120 insertions(+), 2 deletions(-)

-- 
2.51.0



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-15 15:03 [PATCH v2 0/3] mm/ksm: Fix incorrect accounting of KSM counters during fork Donet Tom
@ 2025-09-15 15:03 ` Donet Tom
  2025-09-15 23:42   ` Andrew Morton
                     ` (2 more replies)
  2025-09-15 15:03 ` [PATCH v2 2/3] selftests/mm: Added fork inheritance test for ksm_merging_pages counter Donet Tom
  2025-09-15 15:03 ` [PATCH v2 3/3] selftests/mm: Added fork test to verify global ksm_zero_pages counter behavior Donet Tom
  2 siblings, 3 replies; 17+ messages in thread
From: Donet Tom @ 2025-09-15 15:03 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze,
	Donet Tom, stable

Currently, the KSM-related counters in `mm_struct`, such as
`ksm_merging_pages`, `ksm_rmap_items`, and `ksm_zero_pages`, are
inherited by the child process during fork. This results in inconsistent
accounting.

When a process uses KSM, identical pages are merged and an rmap item is
created for each merged page. The `ksm_merging_pages` and
`ksm_rmap_items` counters are updated accordingly. However, after a
fork, these counters are copied to the child while the corresponding
rmap items are not. As a result, when the child later triggers an
unmerge, there are no rmap items present in the child, so the counters
remain stale, leading to incorrect accounting.

A similar issue exists with `ksm_zero_pages`, which maintains both a
global counter and a per-process counter. During fork, the per-process
counter is inherited by the child, but the global counter is not
incremented. Since the child also references zero pages, the global
counter should be updated as well. Otherwise, during zero-page unmerge,
both the global and per-process counters are decremented, causing the
global counter to become inconsistent.

To fix this, ksm_merging_pages and ksm_rmap_items are reset to 0
during fork, and the global ksm_zero_pages counter is updated with the
per-process ksm_zero_pages value inherited by the child. This ensures
that KSM statistics remain accurate and reflect the activity of each
process correctly.

Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")
Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
cc: stable@vger.kernel.org # v6.6
Signed-off-by: Donet Tom <donettom@linux.ibm.com>
---
 include/linux/ksm.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/ksm.h b/include/linux/ksm.h
index 22e67ca7cba3..067538fc4d58 100644
--- a/include/linux/ksm.h
+++ b/include/linux/ksm.h
@@ -56,8 +56,14 @@ static inline long mm_ksm_zero_pages(struct mm_struct *mm)
 static inline void ksm_fork(struct mm_struct *mm, struct mm_struct *oldmm)
 {
 	/* Adding mm to ksm is best effort on fork. */
-	if (mm_flags_test(MMF_VM_MERGEABLE, oldmm))
+	if (mm_flags_test(MMF_VM_MERGEABLE, oldmm)) {
+		long nr_ksm_zero_pages = atomic_long_read(&mm->ksm_zero_pages);
+
+		mm->ksm_merging_pages = 0;
+		mm->ksm_rmap_items = 0;
+		atomic_long_add(nr_ksm_zero_pages, &ksm_zero_pages);
 		__ksm_enter(mm);
+	}
 }
 
 static inline int ksm_execve(struct mm_struct *mm)
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/3] selftests/mm: Added fork inheritance test for ksm_merging_pages counter
  2025-09-15 15:03 [PATCH v2 0/3] mm/ksm: Fix incorrect accounting of KSM counters during fork Donet Tom
  2025-09-15 15:03 ` [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct " Donet Tom
@ 2025-09-15 15:03 ` Donet Tom
  2025-09-17 13:15   ` David Hildenbrand
  2025-09-15 15:03 ` [PATCH v2 3/3] selftests/mm: Added fork test to verify global ksm_zero_pages counter behavior Donet Tom
  2 siblings, 1 reply; 17+ messages in thread
From: Donet Tom @ 2025-09-15 15:03 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze,
	Donet Tom

Added a new selftest to verify whether the `ksm_merging_pages` counter
in `mm_struct` is not inherited by a child process after fork. This helps
ensure correctness of KSM accounting across process creation.

Signed-off-by: Donet Tom <donettom@linux.ibm.com>
---
 .../selftests/mm/ksm_functional_tests.c       | 42 ++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index 712f43c87736..645cefba2126 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -602,6 +602,45 @@ static void test_prot_none(void)
 	munmap(map, size);
 }
 
+static void test_fork_ksm_merging_page_count(void)
+{
+	const unsigned int size = 2 * MiB;
+	char *map;
+	pid_t child_pid;
+	int status;
+
+	ksft_print_msg("[RUN] %s\n", __func__);
+
+	map = mmap_and_merge_range(0xcf, size, PROT_READ | PROT_WRITE, KSM_MERGE_MADVISE);
+	if (map == MAP_FAILED)
+		return;
+
+	child_pid = fork();
+	if (!child_pid) {
+		init_global_file_handles();
+		exit(ksm_get_self_merging_pages());
+	} else if (child_pid < 0) {
+		ksft_test_result_fail("fork() failed\n");
+		return;
+	}
+
+	if (waitpid(child_pid, &status, 0) < 0) {
+		ksft_test_result_fail("waitpid() failed\n");
+		return;
+	}
+
+	status = WEXITSTATUS(status);
+	if (status) {
+		ksft_test_result_fail("ksm_merging_page in child: %d\n", status);
+		return;
+	}
+
+	ksft_test_result_pass("ksm_merging_pages is not inherited after fork\n");
+
+	ksm_stop();
+	munmap(map, size);
+}
+
 static void init_global_file_handles(void)
 {
 	mem_fd = open("/proc/self/mem", O_RDWR);
@@ -620,7 +659,7 @@ static void init_global_file_handles(void)
 
 int main(int argc, char **argv)
 {
-	unsigned int tests = 8;
+	unsigned int tests = 9;
 	int err;
 
 	if (argc > 1 && !strcmp(argv[1], FORK_EXEC_CHILD_PRG_NAME)) {
@@ -652,6 +691,7 @@ int main(int argc, char **argv)
 	test_prctl_fork();
 	test_prctl_fork_exec();
 	test_prctl_unmerge();
+	test_fork_ksm_merging_page_count();
 
 	err = ksft_get_fail_cnt();
 	if (err)
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/3] selftests/mm: Added fork test to verify global ksm_zero_pages counter behavior
  2025-09-15 15:03 [PATCH v2 0/3] mm/ksm: Fix incorrect accounting of KSM counters during fork Donet Tom
  2025-09-15 15:03 ` [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct " Donet Tom
  2025-09-15 15:03 ` [PATCH v2 2/3] selftests/mm: Added fork inheritance test for ksm_merging_pages counter Donet Tom
@ 2025-09-15 15:03 ` Donet Tom
  2025-09-17 13:19   ` David Hildenbrand
  2 siblings, 1 reply; 17+ messages in thread
From: Donet Tom @ 2025-09-15 15:03 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze,
	Donet Tom

Added a selftest to verify the behavior of the global KSM zero-page
counter during fork. When a process forks, the per-process zero-page
counter is inherited by the child, and the global counter should be
updated with this inherited value. This test ensures that the global
counter is correctly updated after fork.

Signed-off-by: Donet Tom <donettom@linux.ibm.com>
---
 .../selftests/mm/ksm_functional_tests.c       | 74 ++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index 645cefba2126..f23597ac8066 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -602,6 +602,77 @@ static void test_prot_none(void)
 	munmap(map, size);
 }
 
+long ksm_get_global_ksm_zero_pages(void)
+{
+	int global_ksm_zero_pages_fd;
+	char buf[10];
+	ssize_t ret;
+
+	global_ksm_zero_pages_fd = open("/sys/kernel/mm/ksm/ksm_zero_pages",
+								O_RDONLY);
+	if (global_ksm_zero_pages_fd < 0)
+		return -errno;
+
+	ret = pread(global_ksm_zero_pages_fd, buf, sizeof(buf) - 1, 0);
+	close(global_ksm_zero_pages_fd);
+	if (ret <= 0)
+		return -errno;
+	buf[ret] = 0;
+
+	return strtol(buf, NULL, 10);
+}
+
+static void test_fork_global_ksm_zero_pages_count(void)
+{
+	const unsigned int size = 2 * MiB;
+	char *map;
+	pid_t child_pid;
+	int status;
+	long g_zpages_before = 0, g_zpages_after = 0;
+
+	ksft_print_msg("[RUN] %s\n", __func__);
+
+	/* Unmerge all pages before test */
+	if (ksm_stop() < 0) {
+		ksft_test_result_fail("KSM unmerging failed\n");
+		return;
+	}
+	/* Get the global zero page count before test */
+	g_zpages_before = ksm_get_global_ksm_zero_pages();
+	/* Let KSM deduplicate zero pages. */
+	map = mmap_and_merge_range(0x00, size, PROT_READ | PROT_WRITE, KSM_MERGE_MADVISE);
+	if (map == MAP_FAILED)
+		return;
+
+	child_pid = fork();
+	if (!child_pid) {
+		exit(ksm_stop());
+	} else if (child_pid < 0) {
+		ksft_test_result_fail("fork() failed\n");
+		return;
+	}
+	if (waitpid(child_pid, &status, 0) < 0) {
+		ksft_test_result_fail("waitpid() failed\n");
+		return;
+	}
+	status = WEXITSTATUS(status);
+	if (status < 0) {
+		ksft_test_result_fail("KSM unmerging failed in child\n");
+		return;
+	}
+
+	/* Verify global zero-page count remains unchanged */
+	g_zpages_after = ksm_get_global_ksm_zero_pages();
+	if (g_zpages_before != g_zpages_after) {
+		ksft_test_result_fail("Incorrect global ksm zero page count after fork\n");
+		return;
+	}
+
+	ksft_test_result_pass("Global ksm zero page count is correct after fork\n");
+	ksm_stop();
+	munmap(map, size);
+}
+
 static void test_fork_ksm_merging_page_count(void)
 {
 	const unsigned int size = 2 * MiB;
@@ -659,7 +730,7 @@ static void init_global_file_handles(void)
 
 int main(int argc, char **argv)
 {
-	unsigned int tests = 9;
+	unsigned int tests = 10;
 	int err;
 
 	if (argc > 1 && !strcmp(argv[1], FORK_EXEC_CHILD_PRG_NAME)) {
@@ -692,6 +763,7 @@ int main(int argc, char **argv)
 	test_prctl_fork_exec();
 	test_prctl_unmerge();
 	test_fork_ksm_merging_page_count();
+	test_fork_global_ksm_zero_pages_count();
 
 	err = ksft_get_fail_cnt();
 	if (err)
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-15 15:03 ` [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct " Donet Tom
@ 2025-09-15 23:42   ` Andrew Morton
  2025-09-16  2:14     ` Joe Perches
  2025-09-16  4:33     ` Sasha Levin
  2025-09-17 10:38   ` David Hildenbrand
  2025-09-17 12:27   ` Chengming Zhou
  2 siblings, 2 replies; 17+ messages in thread
From: Andrew Morton @ 2025-09-15 23:42 UTC (permalink / raw)
  To: Donet Tom
  Cc: David Hildenbrand, Ritesh Harjani, Xu Xin, Chengming Zhou,
	Wei Yang, Aboorva Devarajan, linux-mm, linux-kernel,
	Giorgi Tchankvetadze, stable, Joe Perches

On Mon, 15 Sep 2025 20:33:04 +0530 Donet Tom <donettom@linux.ibm.com> wrote:

> Currently, the KSM-related counters in `mm_struct`, such as
> `ksm_merging_pages`, `ksm_rmap_items`, and `ksm_zero_pages`, are
> inherited by the child process during fork. This results in inconsistent
> accounting.
> 
> When a process uses KSM, identical pages are merged and an rmap item is
> created for each merged page. The `ksm_merging_pages` and
> `ksm_rmap_items` counters are updated accordingly. However, after a
> fork, these counters are copied to the child while the corresponding
> rmap items are not. As a result, when the child later triggers an
> unmerge, there are no rmap items present in the child, so the counters
> remain stale, leading to incorrect accounting.
> 
> A similar issue exists with `ksm_zero_pages`, which maintains both a
> global counter and a per-process counter. During fork, the per-process
> counter is inherited by the child, but the global counter is not
> incremented. Since the child also references zero pages, the global
> counter should be updated as well. Otherwise, during zero-page unmerge,
> both the global and per-process counters are decremented, causing the
> global counter to become inconsistent.
> 
> To fix this, ksm_merging_pages and ksm_rmap_items are reset to 0
> during fork, and the global ksm_zero_pages counter is updated with the
> per-process ksm_zero_pages value inherited by the child. This ensures
> that KSM statistics remain accurate and reflect the activity of each
> process correctly.
> 
> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")

Linux-v5.19

> Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")

Linux-v6.1

> Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")

Linux-v6.10

> cc: stable@vger.kernel.org # v6.6

So how was Linux-v6.6 arrived at?

I think the most important use for Fixes: is to tell the -stable
maintainers which kernel version(s) we believe should receive the
patch.  So listing multiple Fixes: targets just causes confusion.

Maybe the -stable maintainers parse the "# v6.6" thing, I don't know. 
But providing them with a single Fixes: recommendation is a good thing
either way.

So can you please revisit this and suggest a single Fixes: target which
clarifies your reccomendation?

Thanks.

(Cc Joe.  Should checkpatch say something about this)?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-15 23:42   ` Andrew Morton
@ 2025-09-16  2:14     ` Joe Perches
  2025-09-16  2:54       ` Andrew Morton
  2025-09-16  4:33     ` Sasha Levin
  1 sibling, 1 reply; 17+ messages in thread
From: Joe Perches @ 2025-09-16  2:14 UTC (permalink / raw)
  To: Andrew Morton, Donet Tom
  Cc: David Hildenbrand, Ritesh Harjani, Xu Xin, Chengming Zhou,
	Wei Yang, Aboorva Devarajan, linux-mm, linux-kernel,
	Giorgi Tchankvetadze, stable

On Mon, 2025-09-15 at 16:42 -0700, Andrew Morton wrote:
> On Mon, 15 Sep 2025 20:33:04 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
[]
> > Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
> 
> Linux-v5.19
> 
> > Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")
> 
> Linux-v6.1
> 
> > Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
> 
> Linux-v6.10
> 
> > cc: stable@vger.kernel.org # v6.6
> 
> So how was Linux-v6.6 arrived at?
[]
> (Cc Joe.  Should checkpatch say something about this)?

Probably not. Parsing variants of versions seems, umm, difficult.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-16  2:14     ` Joe Perches
@ 2025-09-16  2:54       ` Andrew Morton
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Morton @ 2025-09-16  2:54 UTC (permalink / raw)
  To: Joe Perches
  Cc: Donet Tom, David Hildenbrand, Ritesh Harjani, Xu Xin,
	Chengming Zhou, Wei Yang, Aboorva Devarajan, linux-mm,
	linux-kernel, Giorgi Tchankvetadze, stable

On Mon, 15 Sep 2025 19:14:47 -0700 Joe Perches <joe@perches.com> wrote:

> On Mon, 2025-09-15 at 16:42 -0700, Andrew Morton wrote:
> > On Mon, 15 Sep 2025 20:33:04 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
> []
> > > Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
> > 
> > Linux-v5.19
> > 
> > > Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")
> > 
> > Linux-v6.1
> > 
> > > Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
> > 
> > Linux-v6.10
> > 
> > > cc: stable@vger.kernel.org # v6.6
> > 
> > So how was Linux-v6.6 arrived at?
> []
> > (Cc Joe.  Should checkpatch say something about this)?
> 
> Probably not. Parsing variants of versions seems, umm, difficult.

I was thinking simpler.  If the Fixes: count exceeds one, ask
"wtf are you asking of the -stable maintainers"?



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-15 23:42   ` Andrew Morton
  2025-09-16  2:14     ` Joe Perches
@ 2025-09-16  4:33     ` Sasha Levin
  2025-09-16  4:41       ` Andrew Morton
  2025-09-16  5:50       ` Donet Tom
  1 sibling, 2 replies; 17+ messages in thread
From: Sasha Levin @ 2025-09-16  4:33 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Donet Tom, David Hildenbrand, Ritesh Harjani, Xu Xin,
	Chengming Zhou, Wei Yang, Aboorva Devarajan, linux-mm,
	linux-kernel, Giorgi Tchankvetadze, stable, Joe Perches

On Mon, Sep 15, 2025 at 04:42:48PM -0700, Andrew Morton wrote:
>On Mon, 15 Sep 2025 20:33:04 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
>
>> Currently, the KSM-related counters in `mm_struct`, such as
>> `ksm_merging_pages`, `ksm_rmap_items`, and `ksm_zero_pages`, are
>> inherited by the child process during fork. This results in inconsistent
>> accounting.
>>
>> When a process uses KSM, identical pages are merged and an rmap item is
>> created for each merged page. The `ksm_merging_pages` and
>> `ksm_rmap_items` counters are updated accordingly. However, after a
>> fork, these counters are copied to the child while the corresponding
>> rmap items are not. As a result, when the child later triggers an
>> unmerge, there are no rmap items present in the child, so the counters
>> remain stale, leading to incorrect accounting.
>>
>> A similar issue exists with `ksm_zero_pages`, which maintains both a
>> global counter and a per-process counter. During fork, the per-process
>> counter is inherited by the child, but the global counter is not
>> incremented. Since the child also references zero pages, the global
>> counter should be updated as well. Otherwise, during zero-page unmerge,
>> both the global and per-process counters are decremented, causing the
>> global counter to become inconsistent.
>>
>> To fix this, ksm_merging_pages and ksm_rmap_items are reset to 0
>> during fork, and the global ksm_zero_pages counter is updated with the
>> per-process ksm_zero_pages value inherited by the child. This ensures
>> that KSM statistics remain accurate and reflect the activity of each
>> process correctly.
>>
>> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
>
>Linux-v5.19
>
>> Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")
>
>Linux-v6.1
>
>> Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
>
>Linux-v6.10
>
>> cc: stable@vger.kernel.org # v6.6
>
>So how was Linux-v6.6 arrived at?

e2942062e01d is in v6.6, not in v6.10 - I suspect that this is why the "# v6.6"
part was added.

>I think the most important use for Fixes: is to tell the -stable
>maintainers which kernel version(s) we believe should receive the
>patch.  So listing multiple Fixes: targets just causes confusion.

Right - there's no way of communicating if all the commits listed in multiple
Fixes tags should exist in the tree, or any one of them, for the new fix to be
applicable.

-- 
Thanks,
Sasha


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-16  4:33     ` Sasha Levin
@ 2025-09-16  4:41       ` Andrew Morton
  2025-09-16 12:45         ` Sasha Levin
  2025-09-16  5:50       ` Donet Tom
  1 sibling, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2025-09-16  4:41 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Donet Tom, David Hildenbrand, Ritesh Harjani, Xu Xin,
	Chengming Zhou, Wei Yang, Aboorva Devarajan, linux-mm,
	linux-kernel, Giorgi Tchankvetadze, stable, Joe Perches

On Tue, 16 Sep 2025 00:33:09 -0400 Sasha Levin <sashal@kernel.org> wrote:

> On Mon, Sep 15, 2025 at 04:42:48PM -0700, Andrew Morton wrote:
> >On Mon, 15 Sep 2025 20:33:04 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
> >
> >> Currently, the KSM-related counters in `mm_struct`, such as
> >> `ksm_merging_pages`, `ksm_rmap_items`, and `ksm_zero_pages`, are
> >> inherited by the child process during fork. This results in inconsistent
> >> accounting.
> >>
> >> When a process uses KSM, identical pages are merged and an rmap item is
> >> created for each merged page. The `ksm_merging_pages` and
> >> `ksm_rmap_items` counters are updated accordingly. However, after a
> >> fork, these counters are copied to the child while the corresponding
> >> rmap items are not. As a result, when the child later triggers an
> >> unmerge, there are no rmap items present in the child, so the counters
> >> remain stale, leading to incorrect accounting.
> >>
> >> A similar issue exists with `ksm_zero_pages`, which maintains both a
> >> global counter and a per-process counter. During fork, the per-process
> >> counter is inherited by the child, but the global counter is not
> >> incremented. Since the child also references zero pages, the global
> >> counter should be updated as well. Otherwise, during zero-page unmerge,
> >> both the global and per-process counters are decremented, causing the
> >> global counter to become inconsistent.
> >>
> >> To fix this, ksm_merging_pages and ksm_rmap_items are reset to 0
> >> during fork, and the global ksm_zero_pages counter is updated with the
> >> per-process ksm_zero_pages value inherited by the child. This ensures
> >> that KSM statistics remain accurate and reflect the activity of each
> >> process correctly.
> >>
> >> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
> >
> >Linux-v5.19
> >
> >> Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")
> >
> >Linux-v6.1
> >
> >> Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
> >
> >Linux-v6.10
> >
> >> cc: stable@vger.kernel.org # v6.6
> >
> >So how was Linux-v6.6 arrived at?
> 
> e2942062e01d is in v6.6, not in v6.10 - I suspect that this is why the "# v6.6"
> part was added.

OK.

> 
> >I think the most important use for Fixes: is to tell the -stable
> >maintainers which kernel version(s) we believe should receive the
> >patch.  So listing multiple Fixes: targets just causes confusion.
> 
> Right - there's no way of communicating if all the commits listed in multiple
> Fixes tags should exist in the tree, or any one of them, for the new fix to be
> applicable.

So what should we do in this situation?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-16  4:33     ` Sasha Levin
  2025-09-16  4:41       ` Andrew Morton
@ 2025-09-16  5:50       ` Donet Tom
  1 sibling, 0 replies; 17+ messages in thread
From: Donet Tom @ 2025-09-16  5:50 UTC (permalink / raw)
  To: Sasha Levin, Andrew Morton
  Cc: David Hildenbrand, Ritesh Harjani, Xu Xin, Chengming Zhou,
	Wei Yang, Aboorva Devarajan, linux-mm, linux-kernel,
	Giorgi Tchankvetadze, stable, Joe Perches


On 9/16/25 10:03 AM, Sasha Levin wrote:
> On Mon, Sep 15, 2025 at 04:42:48PM -0700, Andrew Morton wrote:
>> On Mon, 15 Sep 2025 20:33:04 +0530 Donet Tom <donettom@linux.ibm.com> 
>> wrote:
>>
>>> Currently, the KSM-related counters in `mm_struct`, such as
>>> `ksm_merging_pages`, `ksm_rmap_items`, and `ksm_zero_pages`, are
>>> inherited by the child process during fork. This results in 
>>> inconsistent
>>> accounting.
>>>
>>> When a process uses KSM, identical pages are merged and an rmap item is
>>> created for each merged page. The `ksm_merging_pages` and
>>> `ksm_rmap_items` counters are updated accordingly. However, after a
>>> fork, these counters are copied to the child while the corresponding
>>> rmap items are not. As a result, when the child later triggers an
>>> unmerge, there are no rmap items present in the child, so the counters
>>> remain stale, leading to incorrect accounting.
>>>
>>> A similar issue exists with `ksm_zero_pages`, which maintains both a
>>> global counter and a per-process counter. During fork, the per-process
>>> counter is inherited by the child, but the global counter is not
>>> incremented. Since the child also references zero pages, the global
>>> counter should be updated as well. Otherwise, during zero-page unmerge,
>>> both the global and per-process counters are decremented, causing the
>>> global counter to become inconsistent.
>>>
>>> To fix this, ksm_merging_pages and ksm_rmap_items are reset to 0
>>> during fork, and the global ksm_zero_pages counter is updated with the
>>> per-process ksm_zero_pages value inherited by the child. This ensures
>>> that KSM statistics remain accurate and reflect the activity of each
>>> process correctly.
>>>
>>> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
>>
>> Linux-v5.19
>>
>>> Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each 
>>> process")
>>
>> Linux-v6.1
>>
>>> Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
>>
>> Linux-v6.10
>>
>>> cc: stable@vger.kernel.org # v6.6
>>
>> So how was Linux-v6.6 arrived at?
>
> e2942062e01d is in v6.6, not in v6.10 - I suspect that this is why the 
> "# v6.6"
> part was added.


Yes, e2942062e01d is in v6.6, which is why I mentioned that we need to 
backport it up to v6.6.


>
>> I think the most important use for Fixes: is to tell the -stable
>> maintainers which kernel version(s) we believe should receive the
>> patch.  So listing multiple Fixes: targets just causes confusion.
>
> Right - there's no way of communicating if all the commits listed in 
> multiple
> Fixes tags should exist in the tree, or any one of them, for the new 
> fix to be
> applicable.
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-16  4:41       ` Andrew Morton
@ 2025-09-16 12:45         ` Sasha Levin
  0 siblings, 0 replies; 17+ messages in thread
From: Sasha Levin @ 2025-09-16 12:45 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Donet Tom, David Hildenbrand, Ritesh Harjani, Xu Xin,
	Chengming Zhou, Wei Yang, Aboorva Devarajan, linux-mm,
	linux-kernel, Giorgi Tchankvetadze, stable, Joe Perches

On Mon, Sep 15, 2025 at 09:41:17PM -0700, Andrew Morton wrote:
>On Tue, 16 Sep 2025 00:33:09 -0400 Sasha Levin <sashal@kernel.org> wrote:
>> On Mon, Sep 15, 2025 at 04:42:48PM -0700, Andrew Morton wrote:
>> >I think the most important use for Fixes: is to tell the -stable
>> >maintainers which kernel version(s) we believe should receive the
>> >patch.  So listing multiple Fixes: targets just causes confusion.
>>
>> Right - there's no way of communicating if all the commits listed in multiple
>> Fixes tags should exist in the tree, or any one of them, for the new fix to be
>> applicable.
>
>So what should we do in this situation?

For us, ideally point to the oldest commit fixed by the new commit with a
Fixes: tag, and then a note along with the stable tag if this is a more complex
scenario that we need to consider.

-- 
Thanks,
Sasha


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-15 15:03 ` [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct " Donet Tom
  2025-09-15 23:42   ` Andrew Morton
@ 2025-09-17 10:38   ` David Hildenbrand
  2025-09-17 12:27   ` Chengming Zhou
  2 siblings, 0 replies; 17+ messages in thread
From: David Hildenbrand @ 2025-09-17 10:38 UTC (permalink / raw)
  To: Donet Tom, Andrew Morton
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze,
	stable

On 15.09.25 17:03, Donet Tom wrote:
> Currently, the KSM-related counters in `mm_struct`, such as
> `ksm_merging_pages`, `ksm_rmap_items`, and `ksm_zero_pages`, are
> inherited by the child process during fork. This results in inconsistent
> accounting.
> 
> When a process uses KSM, identical pages are merged and an rmap item is
> created for each merged page. The `ksm_merging_pages` and
> `ksm_rmap_items` counters are updated accordingly. However, after a
> fork, these counters are copied to the child while the corresponding
> rmap items are not. As a result, when the child later triggers an
> unmerge, there are no rmap items present in the child, so the counters
> remain stale, leading to incorrect accounting.
> 
> A similar issue exists with `ksm_zero_pages`, which maintains both a
> global counter and a per-process counter. During fork, the per-process
> counter is inherited by the child, but the global counter is not
> incremented. Since the child also references zero pages, the global
> counter should be updated as well. Otherwise, during zero-page unmerge,
> both the global and per-process counters are decremented, causing the
> global counter to become inconsistent.
> 
> To fix this, ksm_merging_pages and ksm_rmap_items are reset to 0
> during fork, and the global ksm_zero_pages counter is updated with the
> per-process ksm_zero_pages value inherited by the child. This ensures
> that KSM statistics remain accurate and reflect the activity of each
> process correctly.
> 
> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
> Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")
> Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
> cc: stable@vger.kernel.org # v6.6
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---
>   include/linux/ksm.h | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/ksm.h b/include/linux/ksm.h
> index 22e67ca7cba3..067538fc4d58 100644
> --- a/include/linux/ksm.h
> +++ b/include/linux/ksm.h
> @@ -56,8 +56,14 @@ static inline long mm_ksm_zero_pages(struct mm_struct *mm)
>   static inline void ksm_fork(struct mm_struct *mm, struct mm_struct *oldmm)
>   {
>   	/* Adding mm to ksm is best effort on fork. */
> -	if (mm_flags_test(MMF_VM_MERGEABLE, oldmm))
> +	if (mm_flags_test(MMF_VM_MERGEABLE, oldmm)) {
> +		long nr_ksm_zero_pages = atomic_long_read(&mm->ksm_zero_pages);
> +
> +		mm->ksm_merging_pages = 0;
> +		mm->ksm_rmap_items = 0;
> +		atomic_long_add(nr_ksm_zero_pages, &ksm_zero_pages);
>   		__ksm_enter(mm);

That LGTM. KSM is all weird in combination with fork(), but that's 
something for another day to improve I guess.

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct during fork
  2025-09-15 15:03 ` [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct " Donet Tom
  2025-09-15 23:42   ` Andrew Morton
  2025-09-17 10:38   ` David Hildenbrand
@ 2025-09-17 12:27   ` Chengming Zhou
  2 siblings, 0 replies; 17+ messages in thread
From: Chengming Zhou @ 2025-09-17 12:27 UTC (permalink / raw)
  To: Donet Tom, Andrew Morton, David Hildenbrand
  Cc: Ritesh Harjani, Xu Xin, Wei Yang, Aboorva Devarajan, linux-mm,
	linux-kernel, Giorgi Tchankvetadze, stable

On 2025/9/15 23:03, Donet Tom wrote:
> Currently, the KSM-related counters in `mm_struct`, such as
> `ksm_merging_pages`, `ksm_rmap_items`, and `ksm_zero_pages`, are
> inherited by the child process during fork. This results in inconsistent
> accounting.
> 
> When a process uses KSM, identical pages are merged and an rmap item is
> created for each merged page. The `ksm_merging_pages` and
> `ksm_rmap_items` counters are updated accordingly. However, after a
> fork, these counters are copied to the child while the corresponding
> rmap items are not. As a result, when the child later triggers an
> unmerge, there are no rmap items present in the child, so the counters
> remain stale, leading to incorrect accounting.
> 
> A similar issue exists with `ksm_zero_pages`, which maintains both a
> global counter and a per-process counter. During fork, the per-process
> counter is inherited by the child, but the global counter is not
> incremented. Since the child also references zero pages, the global
> counter should be updated as well. Otherwise, during zero-page unmerge,
> both the global and per-process counters are decremented, causing the
> global counter to become inconsistent.
> 
> To fix this, ksm_merging_pages and ksm_rmap_items are reset to 0
> during fork, and the global ksm_zero_pages counter is updated with the
> per-process ksm_zero_pages value inherited by the child. This ensures
> that KSM statistics remain accurate and reflect the activity of each
> process correctly.
> 
> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
> Fixes: cb4df4cae4f2 ("ksm: count allocated ksm rmap_items for each process")
> Fixes: e2942062e01d ("ksm: count all zero pages placed by KSM")
> cc: stable@vger.kernel.org # v6.6
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>

Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev>

Thanks.

> ---
>   include/linux/ksm.h | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/ksm.h b/include/linux/ksm.h
> index 22e67ca7cba3..067538fc4d58 100644
> --- a/include/linux/ksm.h
> +++ b/include/linux/ksm.h
> @@ -56,8 +56,14 @@ static inline long mm_ksm_zero_pages(struct mm_struct *mm)
>   static inline void ksm_fork(struct mm_struct *mm, struct mm_struct *oldmm)
>   {
>   	/* Adding mm to ksm is best effort on fork. */
> -	if (mm_flags_test(MMF_VM_MERGEABLE, oldmm))
> +	if (mm_flags_test(MMF_VM_MERGEABLE, oldmm)) {
> +		long nr_ksm_zero_pages = atomic_long_read(&mm->ksm_zero_pages);
> +
> +		mm->ksm_merging_pages = 0;
> +		mm->ksm_rmap_items = 0;
> +		atomic_long_add(nr_ksm_zero_pages, &ksm_zero_pages);
>   		__ksm_enter(mm);
> +	}
>   }
>   
>   static inline int ksm_execve(struct mm_struct *mm)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] selftests/mm: Added fork inheritance test for ksm_merging_pages counter
  2025-09-15 15:03 ` [PATCH v2 2/3] selftests/mm: Added fork inheritance test for ksm_merging_pages counter Donet Tom
@ 2025-09-17 13:15   ` David Hildenbrand
  2025-09-17 14:45     ` Donet Tom
  0 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2025-09-17 13:15 UTC (permalink / raw)
  To: Donet Tom, Andrew Morton
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze

On 15.09.25 17:03, Donet Tom wrote:
> Added a new selftest to verify whether the `ksm_merging_pages` counter
> in `mm_struct` is not inherited by a child process after fork. This helps
> ensure correctness of KSM accounting across process creation.
> 
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---
>   .../selftests/mm/ksm_functional_tests.c       | 42 ++++++++++++++++++-
>   1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
> index 712f43c87736..645cefba2126 100644
> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
> @@ -602,6 +602,45 @@ static void test_prot_none(void)
>   	munmap(map, size);
>   }
>   
> +static void test_fork_ksm_merging_page_count(void)
> +{
> +	const unsigned int size = 2 * MiB;
> +	char *map;
> +	pid_t child_pid;
> +	int status;
> +
> +	ksft_print_msg("[RUN] %s\n", __func__);
> +
> +	map = mmap_and_merge_range(0xcf, size, PROT_READ | PROT_WRITE, KSM_MERGE_MADVISE);
> +	if (map == MAP_FAILED)
> +		return;
> +
> +	child_pid = fork();
> +	if (!child_pid) {
> +		init_global_file_handles();
> +		exit(ksm_get_self_merging_pages());
> +	} else if (child_pid < 0) {
> +		ksft_test_result_fail("fork() failed\n");
> +		return;

Probably
	goto out;

in all these fail cases

> +	}
> +
> +	if (waitpid(child_pid, &status, 0) < 0) {
> +		ksft_test_result_fail("waitpid() failed\n");
> +		return;
> +	}
> +
> +	status = WEXITSTATUS(status);
> +	if (status) {
> +		ksft_test_result_fail("ksm_merging_page in child: %d\n", status);
> +		return;
> +	}
> +
> +	ksft_test_result_pass("ksm_merging_pages is not inherited after fork\n");
> +

And have

	out:

here

With that:

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/3] selftests/mm: Added fork test to verify global ksm_zero_pages counter behavior
  2025-09-15 15:03 ` [PATCH v2 3/3] selftests/mm: Added fork test to verify global ksm_zero_pages counter behavior Donet Tom
@ 2025-09-17 13:19   ` David Hildenbrand
  2025-09-17 14:47     ` Donet Tom
  0 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2025-09-17 13:19 UTC (permalink / raw)
  To: Donet Tom, Andrew Morton
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze

On 15.09.25 17:03, Donet Tom wrote:
> Added a selftest to verify the behavior of the global KSM zero-page
> counter during fork. When a process forks, the per-process zero-page
> counter is inherited by the child, and the global counter should be
> updated with this inherited value. This test ensures that the global
> counter is correctly updated after fork.
> 
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---
>   .../selftests/mm/ksm_functional_tests.c       | 74 ++++++++++++++++++-
>   1 file changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
> index 645cefba2126..f23597ac8066 100644
> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
> @@ -602,6 +602,77 @@ static void test_prot_none(void)
>   	munmap(map, size);
>   }
>   
> +long ksm_get_global_ksm_zero_pages(void)
> +{
> +	int global_ksm_zero_pages_fd;
> +	char buf[10];
> +	ssize_t ret;
> +
> +	global_ksm_zero_pages_fd = open("/sys/kernel/mm/ksm/ksm_zero_pages",
> +								O_RDONLY);
> +	if (global_ksm_zero_pages_fd < 0)
> +		return -errno;
> +
> +	ret = pread(global_ksm_zero_pages_fd, buf, sizeof(buf) - 1, 0);
> +	close(global_ksm_zero_pages_fd);
> +	if (ret <= 0)
> +		return -errno;
> +	buf[ret] = 0;
> +
> +	return strtol(buf, NULL, 10);
> +}
> +
> +static void test_fork_global_ksm_zero_pages_count(void)
> +{
> +	const unsigned int size = 2 * MiB;
> +	char *map;
> +	pid_t child_pid;
> +	int status;
> +	long g_zpages_before = 0, g_zpages_after = 0;
> +
> +	ksft_print_msg("[RUN] %s\n", __func__);
> +
> +	/* Unmerge all pages before test */
> +	if (ksm_stop() < 0) {
> +		ksft_test_result_fail("KSM unmerging failed\n");
> +		return;
> +	}
> +	/* Get the global zero page count before test */

That only works when "use_zero_pages" is enabled, no?

> +	g_zpages_before = ksm_get_global_ksm_zero_pages();
> +	/* Let KSM deduplicate zero pages. */
> +	map = mmap_and_merge_range(0x00, size, PROT_READ | PROT_WRITE, KSM_MERGE_MADVISE);
> +	if (map == MAP_FAILED)
> +		return;
> +
> +	child_pid = fork();
> +	if (!child_pid) {
> +		exit(ksm_stop());
> +	} else if (child_pid < 0) {
> +		ksft_test_result_fail("fork() failed\n");
> +		return;

Cleanup missing as for patch #2.

> +	}
> +	if (waitpid(child_pid, &status, 0) < 0) {
> +		ksft_test_result_fail("waitpid() failed\n");
> +		return;
> +	}
> +	status = WEXITSTATUS(status);
> +	if (status < 0) {
> +		ksft_test_result_fail("KSM unmerging failed in child\n");
> +		return;
> +	}
> +
> +	/* Verify global zero-page count remains unchanged */
> +	g_zpages_after = ksm_get_global_ksm_zero_pages();
> +	if (g_zpages_before != g_zpages_after) {
> +		ksft_test_result_fail("Incorrect global ksm zero page count after fork\n");
> +		return;
> +	}
> +
> +	ksft_test_result_pass("Global ksm zero page count is correct after fork\n");
> +	ksm_stop();
>

What stops KSM from merging pages in other processes by accident 
concurrently and giving false failures?

Likely you would want to stop ksm before the fork.

-- 
Cheers

David / dhildenb



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] selftests/mm: Added fork inheritance test for ksm_merging_pages counter
  2025-09-17 13:15   ` David Hildenbrand
@ 2025-09-17 14:45     ` Donet Tom
  0 siblings, 0 replies; 17+ messages in thread
From: Donet Tom @ 2025-09-17 14:45 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze


On 9/17/25 6:45 PM, David Hildenbrand wrote:
> On 15.09.25 17:03, Donet Tom wrote:
>> Added a new selftest to verify whether the `ksm_merging_pages` counter
>> in `mm_struct` is not inherited by a child process after fork. This 
>> helps
>> ensure correctness of KSM accounting across process creation.
>>
>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>> ---
>>   .../selftests/mm/ksm_functional_tests.c       | 42 ++++++++++++++++++-
>>   1 file changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c 
>> b/tools/testing/selftests/mm/ksm_functional_tests.c
>> index 712f43c87736..645cefba2126 100644
>> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
>> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
>> @@ -602,6 +602,45 @@ static void test_prot_none(void)
>>       munmap(map, size);
>>   }
>>   +static void test_fork_ksm_merging_page_count(void)
>> +{
>> +    const unsigned int size = 2 * MiB;
>> +    char *map;
>> +    pid_t child_pid;
>> +    int status;
>> +
>> +    ksft_print_msg("[RUN] %s\n", __func__);
>> +
>> +    map = mmap_and_merge_range(0xcf, size, PROT_READ | PROT_WRITE, 
>> KSM_MERGE_MADVISE);
>> +    if (map == MAP_FAILED)
>> +        return;
>> +
>> +    child_pid = fork();
>> +    if (!child_pid) {
>> +        init_global_file_handles();
>> +        exit(ksm_get_self_merging_pages());
>> +    } else if (child_pid < 0) {
>> +        ksft_test_result_fail("fork() failed\n");
>> +        return;
>
> Probably
>     goto out;
>
> in all these fail cases
>
>> +    }
>> +
>> +    if (waitpid(child_pid, &status, 0) < 0) {
>> +        ksft_test_result_fail("waitpid() failed\n");
>> +        return;
>> +    }
>> +
>> +    status = WEXITSTATUS(status);
>> +    if (status) {
>> +        ksft_test_result_fail("ksm_merging_page in child: %d\n", 
>> status);
>> +        return;
>> +    }
>> +
>> +    ksft_test_result_pass("ksm_merging_pages is not inherited after 
>> fork\n");
>> +
>
> And have
>
>     out:
>
> here


Sure, I will send a v3 with this change.


>
> With that:
>
> Acked-by: David Hildenbrand <david@redhat.com>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/3] selftests/mm: Added fork test to verify global ksm_zero_pages counter behavior
  2025-09-17 13:19   ` David Hildenbrand
@ 2025-09-17 14:47     ` Donet Tom
  0 siblings, 0 replies; 17+ messages in thread
From: Donet Tom @ 2025-09-17 14:47 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton
  Cc: Ritesh Harjani, Xu Xin, Chengming Zhou, Wei Yang,
	Aboorva Devarajan, linux-mm, linux-kernel, Giorgi Tchankvetadze


On 9/17/25 6:49 PM, David Hildenbrand wrote:
> On 15.09.25 17:03, Donet Tom wrote:
>> Added a selftest to verify the behavior of the global KSM zero-page
>> counter during fork. When a process forks, the per-process zero-page
>> counter is inherited by the child, and the global counter should be
>> updated with this inherited value. This test ensures that the global
>> counter is correctly updated after fork.
>>
>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>> ---
>>   .../selftests/mm/ksm_functional_tests.c       | 74 ++++++++++++++++++-
>>   1 file changed, 73 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c 
>> b/tools/testing/selftests/mm/ksm_functional_tests.c
>> index 645cefba2126..f23597ac8066 100644
>> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
>> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
>> @@ -602,6 +602,77 @@ static void test_prot_none(void)
>>       munmap(map, size);
>>   }
>>   +long ksm_get_global_ksm_zero_pages(void)
>> +{
>> +    int global_ksm_zero_pages_fd;
>> +    char buf[10];
>> +    ssize_t ret;
>> +
>> +    global_ksm_zero_pages_fd = 
>> open("/sys/kernel/mm/ksm/ksm_zero_pages",
>> +                                O_RDONLY);
>> +    if (global_ksm_zero_pages_fd < 0)
>> +        return -errno;
>> +
>> +    ret = pread(global_ksm_zero_pages_fd, buf, sizeof(buf) - 1, 0);
>> +    close(global_ksm_zero_pages_fd);
>> +    if (ret <= 0)
>> +        return -errno;
>> +    buf[ret] = 0;
>> +
>> +    return strtol(buf, NULL, 10);
>> +}
>> +
>> +static void test_fork_global_ksm_zero_pages_count(void)
>> +{
>> +    const unsigned int size = 2 * MiB;
>> +    char *map;
>> +    pid_t child_pid;
>> +    int status;
>> +    long g_zpages_before = 0, g_zpages_after = 0;
>> +
>> +    ksft_print_msg("[RUN] %s\n", __func__);
>> +
>> +    /* Unmerge all pages before test */
>> +    if (ksm_stop() < 0) {
>> +        ksft_test_result_fail("KSM unmerging failed\n");
>> +        return;
>> +    }
>> +    /* Get the global zero page count before test */
>
> That only works when "use_zero_pages" is enabled, no?
>
>> +    g_zpages_before = ksm_get_global_ksm_zero_pages();
>> +    /* Let KSM deduplicate zero pages. */
>> +    map = mmap_and_merge_range(0x00, size, PROT_READ | PROT_WRITE, 
>> KSM_MERGE_MADVISE);
>> +    if (map == MAP_FAILED)
>> +        return;
>> +
>> +    child_pid = fork();
>> +    if (!child_pid) {
>> +        exit(ksm_stop());
>> +    } else if (child_pid < 0) {
>> +        ksft_test_result_fail("fork() failed\n");
>> +        return;
>
> Cleanup missing as for patch #2.
>
>> +    }
>> +    if (waitpid(child_pid, &status, 0) < 0) {
>> +        ksft_test_result_fail("waitpid() failed\n");
>> +        return;
>> +    }
>> +    status = WEXITSTATUS(status);
>> +    if (status < 0) {
>> +        ksft_test_result_fail("KSM unmerging failed in child\n");
>> +        return;
>> +    }
>> +
>> +    /* Verify global zero-page count remains unchanged */
>> +    g_zpages_after = ksm_get_global_ksm_zero_pages();
>> +    if (g_zpages_before != g_zpages_after) {
>> +        ksft_test_result_fail("Incorrect global ksm zero page count 
>> after fork\n");
>> +        return;
>> +    }
>> +
>> +    ksft_test_result_pass("Global ksm zero page count is correct 
>> after fork\n");
>> +    ksm_stop();
>>
>
> What stops KSM from merging pages in other processes by accident 
> concurrently and giving false failures?
>
> Likely you would want to stop ksm before the fork.


Yes, you are right. I will fix this and send a v3.





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-09-17 14:47 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15 15:03 [PATCH v2 0/3] mm/ksm: Fix incorrect accounting of KSM counters during fork Donet Tom
2025-09-15 15:03 ` [PATCH v2 1/3] mm/ksm: Fix incorrect KSM counter handling in mm_struct " Donet Tom
2025-09-15 23:42   ` Andrew Morton
2025-09-16  2:14     ` Joe Perches
2025-09-16  2:54       ` Andrew Morton
2025-09-16  4:33     ` Sasha Levin
2025-09-16  4:41       ` Andrew Morton
2025-09-16 12:45         ` Sasha Levin
2025-09-16  5:50       ` Donet Tom
2025-09-17 10:38   ` David Hildenbrand
2025-09-17 12:27   ` Chengming Zhou
2025-09-15 15:03 ` [PATCH v2 2/3] selftests/mm: Added fork inheritance test for ksm_merging_pages counter Donet Tom
2025-09-17 13:15   ` David Hildenbrand
2025-09-17 14:45     ` Donet Tom
2025-09-15 15:03 ` [PATCH v2 3/3] selftests/mm: Added fork test to verify global ksm_zero_pages counter behavior Donet Tom
2025-09-17 13:19   ` David Hildenbrand
2025-09-17 14:47     ` Donet Tom

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).