Re: [PATCH] mm/ksm: prepare to remove the redundant ksm_merging_pages in procfs

From: David Hildenbrand
Date: Thu Jul 06 2023 - 08:02:42 EST


On 06.07.23 13:59, Nanyong Sun wrote:
On 2023/7/6 16:55, David Hildenbrand wrote:

On 06.07.23 11:49, Nanyong Sun wrote:
Since the ksm_merging_pages information already included in
/proc/<pid>/ksm_stat, we could remove /proc/<pid>/ksm_merging_pages
to make the directory more clean, and can save a little bit resources.

To delete this interface more smoothly and avoid userspace break,
retain this interface temporarily and modify its function to hint
users to use ksm_stat instead.

Signed-off-by: Nanyong Sun <sunnanyong@xxxxxxxxxx>
---
  fs/proc/base.c | 9 +--------
  1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index eb2e498e3b8d..d080c58cbe6c 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3189,14 +3189,7 @@ static int proc_pid_patch_state(struct
seq_file *m, struct pid_namespace *ns,
  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);
-    }
-
+    seq_puts(m, "please use /proc/<pid>/ksm_stat instead\n");
      return 0;
  }
  static int proc_pid_ksm_stat(struct seq_file *m, struct
pid_namespace *ns,


Why do we care so much about removing 15 simple LOC? That change here
will already mess with user space.

Sorry, but IMHO it's all not worth the churn.

We do not pay attention to these 15 LOC. We pay more attention to the
redundant interface under procfs.

This interface is used by developers and has a short history. Therefore,
changing it now has no big impact.


Again, already 1 year old. And returning garbage instead of ripping it out might be even nastier.

--
Cheers,

David / dhildenb