| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c6c628d9-5908-47f5-83f6-08d1621489fe@kylinos.cn>
Date: Wed, 18 Jun 2025 15:13:20 +0800
From: Longlong Xia <xialonglong@...inos.cn>
To: David Hildenbrand <david@...hat.com>, akpm@...ux-foundation.org,
xu.xin16@....com.cn
Cc: chengming.zhou@...ux.dev, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Stefan Roesch <shr@...kernel.io>
Subject: Re: [PATCH 1/1] mm/ksm: add ksm_pages_sharing for each process to
calculate profit more accurately
Sorry for the late reply. I was thinking about how to respond to your
question.
在 2025/6/6 18:08, David Hildenbrand 写道:
> On 06.06.25 09:03, Longlong Xia wrote:
>
> CCing Stefan.
>
>> The general_profit_show() function only considers ksm_pages_sharing,
>> while ksm_process_profit() includes both ksm_pages_sharing and
>> ksm_pages_shared for each process. This leads to a mismatch between
>> the total profits from ksm_process_profit() and general_profit_show().
>>
>> Based on my tests, the sum of ksm_process_profit() for all processes
>> can be up to 20% higher than general_profit_show(), depending on
>> the size of page_shared. For individual processes, the ratio of
>> ksm_pages_sharing to ksm_merging_pages is usually not equal to 1.
>>
>> To resolve this, we suggest introducing ksm_pages_sharing for each
>> process to accurately calculate its pages_sharing, ensuring
>> ksm_process_profit() reflects shared memory benefits more accurately.
>>
>> Add a new proc file named as ksm_pages_sharing both under /proc/<pid>/
>
> It's an entry in the file, not a new file.
Thank you for pointing that out. I will fix it。
>
>> and /proc/self/ksm_stat/ to indicate the saved pages of this process.
>> (not including ksm_zero_pages)
>
> Curious, why is updating ksm_process_profit() insufficient and we also
> have to expose ksm_pages_sharing?
>
Since ksm_process_profit() uses ksm_merging_pages(pages_sharing +
pages_shared) to calculate the profit for individual processes,
while general_profit uses pages_sharing for profit calculation, this can
lead to the total profit calculated for each process being greater than
that of general_profit.
Additionally, exposing ksm_pages_sharing under /proc/self/ksm_stat/ may
be sufficient.
>>
>> Suggested-by: Xu Xin <xu.xin16@....com.cn>
>> Signed-off-by: Longlong Xia <xialonglong@...inos.cn>
>> ---
>> Documentation/admin-guide/mm/ksm.rst | 5 +++--
>> Documentation/filesystems/proc.rst | 8 ++++++++
>> fs/proc/base.c | 18 ++++++++++++++++++
>> include/linux/mm_types.h | 5 +++++
>> mm/ksm.c | 12 ++++++++----
>> 5 files changed, 42 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/mm/ksm.rst
>> b/Documentation/admin-guide/mm/ksm.rst
>> index ad8e7a41f3b5..0b33ef98930f 100644
>> --- a/Documentation/admin-guide/mm/ksm.rst
>> +++ b/Documentation/admin-guide/mm/ksm.rst
>> @@ -256,9 +256,10 @@ 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_pages_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``, ksm_merging_pages and ksm_rmap_items are
>> + also shown in
>> ``/proc/<pid>/ksm_stat``. The process profit is also shown in
>> ``/proc/<pid>/ksm_stat`` as ksm_process_profit.
>> diff --git a/Documentation/filesystems/proc.rst
>> b/Documentation/filesystems/proc.rst
>> index 2a17865dfe39..e14ea8389500 100644
>> --- a/Documentation/filesystems/proc.rst
>> +++ b/Documentation/filesystems/proc.rst
>> @@ -2290,6 +2290,7 @@ Example
>> / # cat /proc/self/ksm_stat
>> ksm_rmap_items 0
>> ksm_zero_pages 0
>> + ksm_pages_sharing 0
>> ksm_merging_pages 0
>> ksm_process_profit 0
>> ksm_merge_any: no
>> @@ -2312,6 +2313,13 @@ 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_pages_sharing
>> +^^^^^^^^^^^^^^^^^
>> +
>> +It represents how many pages saved of this process.
>> +(not including ksm_zero_pages). It is the same with what
>> +/proc/<pid>/ksm_pages_sharing shows.
> > +> ksm_merging_pages
>> ^^^^^^^^^^^^^^^^^
>> diff --git a/fs/proc/base.c b/fs/proc/base.c
>> index c667702dc69b..327bf82acf54 100644
>> --- a/fs/proc/base.c
>> +++ b/fs/proc/base.c
>> @@ -3262,6 +3262,21 @@ static int proc_pid_ksm_merging_pages(struct
>> seq_file *m, struct pid_namespace *
>> return 0;
>> }
>> +
>> +static int proc_pid_ksm_pages_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_pages_sharing);
>> + 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)
>> {
>> @@ -3272,6 +3287,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_pages_sharing %lu\n",
>> mm->ksm_pages_sharing);
>> seq_printf(m, "ksm_merging_pages %lu\n",
>> mm->ksm_merging_pages);
>> seq_printf(m, "ksm_process_profit %ld\n",
>> ksm_process_profit(mm));
>> seq_printf(m, "ksm_merge_any: %s\n",
>> @@ -3421,6 +3437,7 @@ static const struct pid_entry tgid_base_stuff[]
>> = {
>> ONE("seccomp_cache", S_IRUSR, proc_pid_seccomp_cache),
>> #endif
>> #ifdef CONFIG_KSM
>> + ONE("ksm_pages_sharing", S_IRUSR, proc_pid_ksm_pages_sharing),
>> ONE("ksm_merging_pages", S_IRUSR, proc_pid_ksm_merging_pages),
>> ONE("ksm_stat", S_IRUSR, proc_pid_ksm_stat),
>> #endif
>> @@ -3758,6 +3775,7 @@ static const struct pid_entry tid_base_stuff[] = {
>> ONE("seccomp_cache", S_IRUSR, proc_pid_seccomp_cache),
>> #endif
>> #ifdef CONFIG_KSM
>> + ONE("ksm_pages_sharing", S_IRUSR, proc_pid_ksm_pages_sharing),
>> ONE("ksm_merging_pages", S_IRUSR, proc_pid_ksm_merging_pages),
>> 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..d260cb09c10a 100644
>> --- a/include/linux/mm_types.h
>> +++ b/include/linux/mm_types.h
>> @@ -1176,6 +1176,11 @@ struct mm_struct {
>> * merging (not including ksm_zero_pages).
>> */
>> unsigned long ksm_merging_pages;
>> + /*
>> + * Represents how many pages saved of this process.
>> + * (not including ksm_zero_pages).
>> + */
>> + unsigned long ksm_pages_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 8583fb91ef13..c2d85ea07b1c 100644
>> --- a/mm/ksm.c
>> +++ b/mm/ksm.c
>> @@ -824,6 +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_pages_sharing--;
>> trace_ksm_remove_rmap_item(stable_node->kpfn,
>> rmap_item, rmap_item->mm);
>> } else {
>> ksm_pages_shared--;
>> @@ -976,8 +977,10 @@ 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--;
>> + rmap_item->mm->ksm_pages_sharing--;
>> + }
>
>
> Hm, I am wondering if that works. Stable nodes are not per MM, so
> can't we create an accounting imbalance for one MM somehow?
>
> (did not look into all the details, just something that came to mind)
>
Indeed, using the method in this patch to calculate ksm_pages_sharing
for each process to determine ksm_pages_shared
can sometimes result in negative values for ksm_pages_shared.
example for calculate mm->ksm_pages_shared：
if (rmap_item->hlist.next) {
ksm_pages_sharing--;
rmap_item->mm->ksm_pages_sharing--;
} else {
ksm_pages_shared--;
rmap_item->mm->ksm_pages_shared--; // can be negative
}
rmap_item->mm->ksm_merging_pages--;
Would it be possible to compare the ratio of each process's rmap_item to
the total rmap_item and the ratio of the process's page_shared to the
total page_shared
to assess this imbalance? For now, I don't have any better ideas.
Thank you for your time.
Best regards,
Longlong Xia
Powered by blists - more mailing lists