[PATCH 0/2] mm/ksm: calculate ksm_process_profit more accurately

[PATCH 0/2] mm/ksm: calculate ksm_process_profit more accurately

[Longlong Xia]

During the KSM testing, it was observed that the sum of KSM profits 
for each process does not equal the general_profit.

The reason is general_profit_show() only considers ksm_pages_sharing,
whereas ksm_process_profit() accounts for both ksm_pages_sharing
and ksm_pages_shared for each process. By concentrating solely on 
ksm_pages_sharing, we can achieve a more accurate representation 
of actual profits.

Thanks for review and comments!

Longlong Xia (2):
  mm/ksm: calculate ksm_process_profit more accurately
  mm/ksm: Rename ksm_merging_pages to ksm_process_sharing

 Documentation/admin-guide/mm/ksm.rst          |  6 ++---
 Documentation/filesystems/proc.rst            |  6 ++---
 .../translations/zh_CN/admin-guide/mm/ksm.rst |  8 +++----
 .../translations/zh_TW/admin-guide/mm/ksm.rst |  8 +++----
 fs/proc/base.c                                | 10 ++++-----
 include/linux/mm_types.h                      |  2 +-
 mm/ksm.c                                      | 22 ++++++++-----------
 .../selftests/mm/ksm_functional_tests.c       | 10 ++++-----
 8 files changed, 34 insertions(+), 38 deletions(-)

-- 
2.43.0

[PATCH 1/2] mm/ksm: calculate ksm_process_profit more accurately

[Longlong Xia]

The general_profit_show() only considers ksm_pages_sharing,
whereas ksm_process_profit() accounts for both ksm_pages_sharing
and ksm_pages_shared for each process. This discrepancy leads to
the sum of ksm_process_profit() across all processes not being equal
to general_profit_show().

Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
---
 mm/ksm.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 8583fb91ef13..fa4e1618b671 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -824,12 +824,10 @@ static void remove_node_from_stable_tree(struct ksm_stable_node *stable_node)
 	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
 		if (rmap_item->hlist.next) {
 			ksm_pages_sharing--;
+			rmap_item->mm->ksm_merging_pages--;
 			trace_ksm_remove_rmap_item(stable_node->kpfn, rmap_item, rmap_item->mm);
-		} else {
+		} else
 			ksm_pages_shared--;
-		}
-
-		rmap_item->mm->ksm_merging_pages--;
 
 		VM_BUG_ON(stable_node->rmap_hlist_len <= 0);
 		stable_node->rmap_hlist_len--;
@@ -976,13 +974,12 @@ static void remove_rmap_item_from_tree(struct ksm_rmap_item *rmap_item)
 		folio_unlock(folio);
 		folio_put(folio);
 
-		if (!hlist_empty(&stable_node->hlist))
+		if (!hlist_empty(&stable_node->hlist)) {
 			ksm_pages_sharing--;
-		else
+			rmap_item->mm->ksm_merging_pages--;
+		} else
 			ksm_pages_shared--;
 
-		rmap_item->mm->ksm_merging_pages--;
-
 		VM_BUG_ON(stable_node->rmap_hlist_len <= 0);
 		stable_node->rmap_hlist_len--;
 
@@ -2202,12 +2199,11 @@ static void stable_tree_append(struct ksm_rmap_item *rmap_item,
 	rmap_item->address |= STABLE_FLAG;
 	hlist_add_head(&rmap_item->hlist, &stable_node->hlist);
 
-	if (rmap_item->hlist.next)
+	if (rmap_item->hlist.next) {
 		ksm_pages_sharing++;
-	else
+		rmap_item->mm->ksm_merging_pages++;
+	} else
 		ksm_pages_shared++;
-
-	rmap_item->mm->ksm_merging_pages++;
 }
 
 /*
-- 
2.43.0

Re: [PATCH 1/2] mm/ksm: calculate ksm_process_profit more accurately

[xu.xin16]

> The general_profit_show() only considers ksm_pages_sharing,
> whereas ksm_process_profit() accounts for both ksm_pages_sharing
> and ksm_pages_shared for each process. This discrepancy leads to
> the sum of ksm_process_profit() across all processes not being equal
> to general_profit_show().
> 
> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> ---
>  mm/ksm.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)

Thanks for you report, but we'd better not change the definition of ksm_merging_pages which means
all pages involved in ksm merging (pages_sharing + pages_shared), and Do not rename the existing
of interface, which breaks user-space tools.

If we reallt want a more pricise profit of a process , I suggest:
 -  1) Add a variable 'ksm_pages_sharing'  in mm_struct for processes.
 -  2) Refine the implementation of ksm_process_profit().

Thanks~

> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 8583fb91ef13..fa4e1618b671 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -824,12 +824,10 @@ static void remove_node_from_stable_tree(struct ksm_stable_node *stable_node)
>  	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
>  		if (rmap_item->hlist.next) {
>  			ksm_pages_sharing--;
> +			rmap_item->mm->ksm_merging_pages--;
>  			trace_ksm_remove_rmap_item(stable_node->kpfn, rmap_item, rmap_item->mm);
> -		} else {
> +		} else
>  			ksm_pages_shared--;
> -		}
> -
> -		rmap_item->mm->ksm_merging_pages--;

Re: [PATCH 1/2] mm/ksm: calculate ksm_process_profit more accurately

[David Hildenbrand]

On 04.06.25 05:17, Longlong Xia wrote:
> The general_profit_show() only considers ksm_pages_sharing,
> whereas ksm_process_profit() accounts for both ksm_pages_sharing
> and ksm_pages_shared for each process. This discrepancy leads to
> the sum of ksm_process_profit() across all processes not being equal
> to general_profit_show().
> 
> Fixes: 7609385337a4 ("ksm: count ksm merging pages for each process")
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> ---


What happened to the original idea of [1]

[1] 
https://lkml.kernel.org/r/20250606070314.3028593-1-xialonglong@kylinos.cn

-- 
Cheers,

David / dhildenb

[PATCH 2/2] mm/ksm: Rename ksm_merging_pages to ksm_process_sharing

[Longlong Xia]

Renamed ksm_merging_pages to ksm_process_sharing to better
reflect its role in representing the number of sharing pages
for each process.

Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
---
 Documentation/admin-guide/mm/ksm.rst                   |  6 +++---
 Documentation/filesystems/proc.rst                     |  6 +++---
 .../translations/zh_CN/admin-guide/mm/ksm.rst          |  8 ++++----
 .../translations/zh_TW/admin-guide/mm/ksm.rst          |  8 ++++----
 fs/proc/base.c                                         | 10 +++++-----
 include/linux/mm_types.h                               |  2 +-
 mm/ksm.c                                               |  8 ++++----
 tools/testing/selftests/mm/ksm_functional_tests.c      | 10 +++++-----
 8 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
index ad8e7a41f3b5..02b165dfba79 100644
--- a/Documentation/admin-guide/mm/ksm.rst
+++ b/Documentation/admin-guide/mm/ksm.rst
@@ -256,18 +256,18 @@ several times, which are unprofitable memory consumed.
 	process_profit =~ ksm_saved_pages * sizeof(page) -
 			  ksm_rmap_items * sizeof(rmap_item).
 
-   where ksm_saved_pages equals to the sum of ``ksm_merging_pages`` and
+   where ksm_saved_pages equals to the sum of ``ksm_process_sharing`` and
    ``ksm_zero_pages``, both of which are shown under the directory
    ``/proc/<pid>/ksm_stat``, and ksm_rmap_items is also shown in
    ``/proc/<pid>/ksm_stat``. The process profit is also shown in
    ``/proc/<pid>/ksm_stat`` as ksm_process_profit.
 
 From the perspective of application, a high ratio of ``ksm_rmap_items`` to
-``ksm_merging_pages`` means a bad madvise-applied policy, so developers or
+``ksm_process_sharing`` means a bad madvise-applied policy, so developers or
 administrators have to rethink how to change madvise policy. Giving an example
 for reference, a page's size is usually 4K, and the rmap_item's size is
 separately 32B on 32-bit CPU architecture and 64B on 64-bit CPU architecture.
-so if the ``ksm_rmap_items/ksm_merging_pages`` ratio exceeds 64 on 64-bit CPU
+so if the ``ksm_rmap_items/ksm_process_sharing`` ratio exceeds 64 on 64-bit CPU
 or exceeds 128 on 32-bit CPU, then the app's madvise policy should be dropped,
 because the ksm profit is approximately zero or negative.
 
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 2a17865dfe39..08840f027b60 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2290,7 +2290,7 @@ Example
     / # cat /proc/self/ksm_stat
     ksm_rmap_items 0
     ksm_zero_pages 0
-    ksm_merging_pages 0
+    ksm_process_sharing 0
     ksm_process_profit 0
     ksm_merge_any: no
     ksm_mergeable: no
@@ -2312,12 +2312,12 @@ ksm_zero_pages
 When /sys/kernel/mm/ksm/use_zero_pages is enabled, it represent how many
 empty pages are merged with kernel zero pages by KSM.
 
-ksm_merging_pages
+ksm_process_sharing
 ^^^^^^^^^^^^^^^^^
 
 It represents how many pages of this process are involved in KSM merging
 (not including ksm_zero_pages). It is the same with what
-/proc/<pid>/ksm_merging_pages shows.
+/proc/<pid>/ksm_process_sharing shows.
 
 ksm_process_profit
 ^^^^^^^^^^^^^^^^^^
diff --git a/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst b/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst
index 0029c4fd2201..8fb9e1452954 100644
--- a/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst
+++ b/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst
@@ -164,16 +164,16 @@ KSM可以通过合并相同的页面来节省内存，但也会消耗额外的
 
 2) 单一进程中KSM的收益也可以通过以下近似的计算得到::
 
-       process_profit =~ ksm_merging_pages * sizeof(page) -
+       process_profit =~ ksm_process_sharing * sizeof(page) -
                          ksm_rmap_items * sizeof(rmap_item).
 
-   其中ksm_merging_pages显示在 ``/proc/<pid>/`` 目录下，而ksm_rmap_items
+   其中ksm_process_sharing显示在 ``/proc/<pid>/`` 目录下，而ksm_rmap_items
    显示在 ``/proc/<pid>/ksm_stat`` 。
 
-从应用的角度来看， ``ksm_rmap_items`` 和 ``ksm_merging_pages`` 的高比例意
+从应用的角度来看， ``ksm_rmap_items`` 和 ``ksm_process_sharing`` 的高比例意
 味着不好的madvise-applied策略，所以开发者或管理员必须重新考虑如何改变madvis策
 略。举个例子供参考，一个页面的大小通常是4K，而rmap_item的大小在32位CPU架构上分
-别是32B，在64位CPU架构上是64B。所以如果 ``ksm_rmap_items/ksm_merging_pages``
+别是32B，在64位CPU架构上是64B。所以如果 ``ksm_rmap_items/ksm_process_sharing``
 的比例在64位CPU上超过64，或者在32位CPU上超过128，那么应用程序的madvise策略应
 该被放弃，因为ksm收益大约为零或负值。
 
diff --git a/Documentation/translations/zh_TW/admin-guide/mm/ksm.rst b/Documentation/translations/zh_TW/admin-guide/mm/ksm.rst
index 1b4944b3cf61..3bcefda97827 100644
--- a/Documentation/translations/zh_TW/admin-guide/mm/ksm.rst
+++ b/Documentation/translations/zh_TW/admin-guide/mm/ksm.rst
@@ -164,16 +164,16 @@ KSM可以通過合併相同的頁面來節省內存，但也會消耗額外的
 
 2) 單一進程中KSM的收益也可以通過以下近似的計算得到::
 
-       process_profit =~ ksm_merging_pages * sizeof(page) -
+       process_profit =~ ksm_process_sharing * sizeof(page) -
                          ksm_rmap_items * sizeof(rmap_item).
 
-   其中ksm_merging_pages顯示在 ``/proc/<pid>/`` 目錄下，而ksm_rmap_items
+   其中ksm_process_sharing顯示在 ``/proc/<pid>/`` 目錄下，而ksm_rmap_items
    顯示在 ``/proc/<pid>/ksm_stat`` 。
 
-從應用的角度來看， ``ksm_rmap_items`` 和 ``ksm_merging_pages`` 的高比例意
+從應用的角度來看， ``ksm_rmap_items`` 和 ``ksm_process_sharing`` 的高比例意
 味着不好的madvise-applied策略，所以開發者或管理員必須重新考慮如何改變madvis策
 略。舉個例子供參考，一個頁面的大小通常是4K，而rmap_item的大小在32位CPU架構上分
-別是32B，在64位CPU架構上是64B。所以如果 ``ksm_rmap_items/ksm_merging_pages``
+別是32B，在64位CPU架構上是64B。所以如果 ``ksm_rmap_items/ksm_process_sharing``
 的比例在64位CPU上超過64，或者在32位CPU上超過128，那麼應用程序的madvise策略應
 該被放棄，因爲ksm收益大約爲零或負值。
 
diff --git a/fs/proc/base.c b/fs/proc/base.c
index c667702dc69b..91582e829818 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3249,14 +3249,14 @@ static int proc_pid_patch_state(struct seq_file *m, struct pid_namespace *ns,
 #endif /* CONFIG_LIVEPATCH */
 
 #ifdef CONFIG_KSM
-static int proc_pid_ksm_merging_pages(struct seq_file *m, struct pid_namespace *ns,
+static int proc_pid_ksm_process_sharing(struct seq_file *m, struct pid_namespace *ns,
 				struct pid *pid, struct task_struct *task)
 {
 	struct mm_struct *mm;
 
 	mm = get_task_mm(task);
 	if (mm) {
-		seq_printf(m, "%lu\n", mm->ksm_merging_pages);
+		seq_printf(m, "%lu\n", mm->ksm_process_sharing);
 		mmput(mm);
 	}
 
@@ -3272,7 +3272,7 @@ static int proc_pid_ksm_stat(struct seq_file *m, struct pid_namespace *ns,
 	if (mm) {
 		seq_printf(m, "ksm_rmap_items %lu\n", mm->ksm_rmap_items);
 		seq_printf(m, "ksm_zero_pages %ld\n", mm_ksm_zero_pages(mm));
-		seq_printf(m, "ksm_merging_pages %lu\n", mm->ksm_merging_pages);
+		seq_printf(m, "ksm_process_sharing %lu\n", mm->ksm_process_sharing);
 		seq_printf(m, "ksm_process_profit %ld\n", ksm_process_profit(mm));
 		seq_printf(m, "ksm_merge_any: %s\n",
 				test_bit(MMF_VM_MERGE_ANY, &mm->flags) ? "yes" : "no");
@@ -3421,7 +3421,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 	ONE("seccomp_cache", S_IRUSR, proc_pid_seccomp_cache),
 #endif
 #ifdef CONFIG_KSM
-	ONE("ksm_merging_pages",  S_IRUSR, proc_pid_ksm_merging_pages),
+	ONE("ksm_process_sharing",  S_IRUSR, proc_pid_ksm_process_sharing),
 	ONE("ksm_stat",  S_IRUSR, proc_pid_ksm_stat),
 #endif
 };
@@ -3758,7 +3758,7 @@ static const struct pid_entry tid_base_stuff[] = {
 	ONE("seccomp_cache", S_IRUSR, proc_pid_seccomp_cache),
 #endif
 #ifdef CONFIG_KSM
-	ONE("ksm_merging_pages",  S_IRUSR, proc_pid_ksm_merging_pages),
+	ONE("ksm_process_sharing",  S_IRUSR, proc_pid_ksm_process_sharing),
 	ONE("ksm_stat",  S_IRUSR, proc_pid_ksm_stat),
 #endif
 };
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index d6b91e8a66d6..c3383701c2a7 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1175,7 +1175,7 @@ struct mm_struct {
 		 * Represent how many pages of this process are involved in KSM
 		 * merging (not including ksm_zero_pages).
 		 */
-		unsigned long ksm_merging_pages;
+		unsigned long ksm_process_sharing;
 		/*
 		 * Represent how many pages are checked for ksm merging
 		 * including merged and not merged.
diff --git a/mm/ksm.c b/mm/ksm.c
index fa4e1618b671..fbcecd0fc1fd 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -824,7 +824,7 @@ static void remove_node_from_stable_tree(struct ksm_stable_node *stable_node)
 	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
 		if (rmap_item->hlist.next) {
 			ksm_pages_sharing--;
-			rmap_item->mm->ksm_merging_pages--;
+			rmap_item->mm->ksm_process_sharing--;
 			trace_ksm_remove_rmap_item(stable_node->kpfn, rmap_item, rmap_item->mm);
 		} else
 			ksm_pages_shared--;
@@ -976,7 +976,7 @@ static void remove_rmap_item_from_tree(struct ksm_rmap_item *rmap_item)
 
 		if (!hlist_empty(&stable_node->hlist)) {
 			ksm_pages_sharing--;
-			rmap_item->mm->ksm_merging_pages--;
+			rmap_item->mm->ksm_process_sharing--;
 		} else
 			ksm_pages_shared--;
 
@@ -2201,7 +2201,7 @@ static void stable_tree_append(struct ksm_rmap_item *rmap_item,
 
 	if (rmap_item->hlist.next) {
 		ksm_pages_sharing++;
-		rmap_item->mm->ksm_merging_pages++;
+		rmap_item->mm->ksm_process_sharing++;
 	} else
 		ksm_pages_shared++;
 }
@@ -3286,7 +3286,7 @@ bool ksm_process_mergeable(struct mm_struct *mm)
 
 long ksm_process_profit(struct mm_struct *mm)
 {
-	return (long)(mm->ksm_merging_pages + mm_ksm_zero_pages(mm)) * PAGE_SIZE -
+	return (long)(mm->ksm_process_sharing + mm_ksm_zero_pages(mm)) * PAGE_SIZE -
 		mm->ksm_rmap_items * sizeof(struct ksm_rmap_item);
 }
 #endif /* CONFIG_PROC_FS */
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index b61803e36d1c..48037e6cc55d 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -41,7 +41,7 @@ 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 proc_self_ksm_process_sharing_fd;
 static int ksm_use_zero_pages_fd;
 static int pagemap_fd;
 static size_t pagesize;
@@ -105,10 +105,10 @@ 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;
+	if (proc_self_ksm_process_sharing_fd < 0)
+		return proc_self_ksm_process_sharing_fd;
 
-	ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0);
+	ret = pread(proc_self_ksm_process_sharing_fd, buf, sizeof(buf) - 1, 0);
 	if (ret <= 0)
 		return -errno;
 	buf[ret] = 0;
@@ -671,7 +671,7 @@ static void init_global_file_handles(void)
 	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",
+	proc_self_ksm_process_sharing_fd = open("/proc/self/ksm_process_sharing",
 						O_RDONLY);
 	ksm_use_zero_pages_fd = open("/sys/kernel/mm/ksm/use_zero_pages", O_RDWR);
 }
-- 
2.43.0

Re: [PATCH 2/2] mm/ksm: Rename ksm_merging_pages to ksm_process_sharing

[David Hildenbrand]

On 04.06.25 05:17, Longlong Xia wrote:
> Renamed ksm_merging_pages to ksm_process_sharing to better
> reflect its role in representing the number of sharing pages
> for each process.
> 

Isn't this possibly breaking user space?


-- 
Cheers,

David / dhildenb

Re: [PATCH 0/2] mm/ksm: calculate ksm_process_profit more accurately

[David Hildenbrand]

On 04.06.25 05:17, Longlong Xia wrote:

./scripts/get_maintainer.pl is helpful to identify all people worth 
CCing (such as, all maintainers).

> During the KSM testing, it was observed that the sum of KSM profits
> for each process does not equal the general_profit.
> 
> The reason is general_profit_show() only considers ksm_pages_sharing,
> whereas ksm_process_profit() accounts for both ksm_pages_sharing
> and ksm_pages_shared for each process. By concentrating solely on
> ksm_pages_sharing, we can achieve a more accurate representation
> of actual profits.
> 
> Thanks for review and comments!
> 
> Longlong Xia (2):
>    mm/ksm: calculate ksm_process_profit more accurately
>    mm/ksm: Rename ksm_merging_pages to ksm_process_sharing
> 
>   Documentation/admin-guide/mm/ksm.rst          |  6 ++---
>   Documentation/filesystems/proc.rst            |  6 ++---
>   .../translations/zh_CN/admin-guide/mm/ksm.rst |  8 +++----
>   .../translations/zh_TW/admin-guide/mm/ksm.rst |  8 +++----
>   fs/proc/base.c                                | 10 ++++-----
>   include/linux/mm_types.h                      |  2 +-
>   mm/ksm.c                                      | 22 ++++++++-----------
>   .../selftests/mm/ksm_functional_tests.c       | 10 ++++-----
>   8 files changed, 34 insertions(+), 38 deletions(-)
> 


-- 
Cheers,

David / dhildenb