diff mbox series

mm/ksm: delete the redundant ksm_merging_pages interafce in proc

Message ID 20230627133542.2446285-1-sunnanyong@huawei.com (mailing list archive)
State New
Headers show
Series mm/ksm: delete the redundant ksm_merging_pages interafce in proc | expand

Commit Message

Nanyong Sun June 27, 2023, 1:35 p.m. UTC
Since the ksm_merging_pages information already included in
/proc/<pid>/ksm_stat, we can delete /proc/<pid>/ksm_merging_pages to
make the directory more clean, and can save a little bit resources.

Signed-off-by: Nanyong Sun <sunnanyong@huawei.com>
---
 Documentation/admin-guide/mm/ksm.rst              |  6 +++---
 .../translations/zh_CN/admin-guide/mm/ksm.rst     |  4 ++--
 fs/proc/base.c                                    | 15 ---------------
 3 files changed, 5 insertions(+), 20 deletions(-)

Comments

Andrew Morton June 27, 2023, 7:22 p.m. UTC | #1
On Tue, 27 Jun 2023 21:35:42 +0800 Nanyong Sun <sunnanyong@huawei.com> wrote:

> Since the ksm_merging_pages information already included in
> /proc/<pid>/ksm_stat, we can delete /proc/<pid>/ksm_merging_pages to
> make the directory more clean, and can save a little bit resources.

Well, this is a non backward compatible change - any userspace
which depends on ksm_merging_pages will break.

Yes, we could remove this but it will be a lengthy process involving
emitting a "please use ksm_stat instead" message for a few
years before removal.
Nanyong Sun June 28, 2023, 2:44 a.m. UTC | #2
On 2023/6/28 3:22, Andrew Morton wrote:

> On Tue, 27 Jun 2023 21:35:42 +0800 Nanyong Sun <sunnanyong@huawei.com> wrote:
>
>> Since the ksm_merging_pages information already included in
>> /proc/<pid>/ksm_stat, we can delete /proc/<pid>/ksm_merging_pages to
>> make the directory more clean, and can save a little bit resources.
> Well, this is a non backward compatible change - any userspace
> which depends on ksm_merging_pages will break.
>
> Yes, we could remove this but it will be a lengthy process involving
> emitting a "please use ksm_stat instead" message for a few
> years before removal.
> .

This interface was just added last year(Apr 28 2022) in commit 7609385337a4

("ksm: count ksm merging pages for each process"), it does not have 
historical baggage.

Additionally, as stated in the commit message, this interface is mainly 
used to query the deduplication effect

of the application for the upper application optimization during the 
development phase,

so deleting it would have a very small impact.
David Hildenbrand June 28, 2023, 9:07 a.m. UTC | #3
On 28.06.23 05:12, xu xin wrote:
>> Message-ID: <20230627133542.2446285-1-sunnanyong@huawei.com> (raw)
>>
>> Since the ksm_merging_pages information already included in
>> /proc/<pid>/ksm_stat, we can delete /proc/<pid>/ksm_merging_pages to
>> make the directory more clean, and can save a little bit resources.
> 
> I think it's ok to remove it because this interface was not proposed for a long time.
> I believe its users are not many yet. The earlier we delete it, the better.
> 

Well ... it's been around since v5.19, which was released 1 year ago.

... not sure if the removal is worth the possible user space break.
diff mbox series

Patch

diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
index 7626392fe82c..e668d4b5e800 100644
--- a/Documentation/admin-guide/mm/ksm.rst
+++ b/Documentation/admin-guide/mm/ksm.rst
@@ -208,9 +208,9 @@  several times, which are unprofitable memory consumed.
 	process_profit =~ ksm_merging_pages * sizeof(page) -
 			  ksm_rmap_items * sizeof(rmap_item).
 
-   where ksm_merging_pages is shown under the directory ``/proc/<pid>/``,
-   and ksm_rmap_items is shown in ``/proc/<pid>/ksm_stat``. The process profit
-   is also shown in ``/proc/<pid>/ksm_stat`` as ksm_process_profit.
+   where ksm_merging_pages and ksm_rmap_items is shown in the file
+   ``/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
diff --git a/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst b/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst
index 0029c4fd2201..1662f271efc8 100644
--- a/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst
+++ b/Documentation/translations/zh_CN/admin-guide/mm/ksm.rst
@@ -167,8 +167,8 @@  KSM可以通过合并相同的页面来节省内存,但也会消耗额外的
        process_profit =~ ksm_merging_pages * sizeof(page) -
                          ksm_rmap_items * sizeof(rmap_item).
 
-   其中ksm_merging_pages显示在 ``/proc/<pid>/`` 目录下,而ksm_rmap_items
-   显示在 ``/proc/<pid>/ksm_stat`` 。
+   其中ksm_merging_pages、ksm_rmap_items显示在 ``/proc/<pid>/ksm_stat`` 文件中,收益
+   值ksm_process_profit也显示在该文件中。
 
 从应用的角度来看, ``ksm_rmap_items`` 和 ``ksm_merging_pages`` 的高比例意
 味着不好的madvise-applied策略,所以开发者或管理员必须重新考虑如何改变madvis策
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 05452c3b9872..173261dbeaea 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3186,19 +3186,6 @@  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,
-				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);
-		mmput(mm);
-	}
-
-	return 0;
-}
 static int proc_pid_ksm_stat(struct seq_file *m, struct pid_namespace *ns,
 				struct pid *pid, struct task_struct *task)
 {
@@ -3348,7 +3335,6 @@  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_stat",  S_IRUSR, proc_pid_ksm_stat),
 #endif
 };
@@ -3686,7 +3672,6 @@  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_stat",  S_IRUSR, proc_pid_ksm_stat),
 #endif
 };