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 <[email protected]>
---
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(-)
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
};
--
2.25.1
On Tue, 27 Jun 2023 21:35:42 +0800 Nanyong Sun <[email protected]> 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.
On 2023/6/28 3:22, Andrew Morton wrote:
> On Tue, 27 Jun 2023 21:35:42 +0800 Nanyong Sun <[email protected]> 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.
> Message-ID: <[email protected]> (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.
The patch is good except some grammar issues.
Reviewed-by: xu xin <[email protected]>
> /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 <[email protected]>
> ---
> 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(-)
>
> 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
is -> are
> + ``/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
> };
> --
> 2.25.1
On 28.06.23 05:12, xu xin wrote:
>> Message-ID: <[email protected]> (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.
--
Cheers,
David / dhildenb