2022-10-11 03:43:53

by xu

[permalink] [raw]
Subject: [PATCH v3 0/5] ksm: support tracking KSM-placed zero-pages

From: xu xin <[email protected]>

use_zero_pages is good, not just because of cache colouring as described
in doc, but also because use_zero_pages can accelerate merging empty pages
when there are plenty of empty pages (full of zeros) as the time of
page-by-page comparisons (unstable_tree_search_insert) is saved.

But there is something to improve, that is, when enabling use_zero_pages,
all empty pages will be merged with kernel zero pages instead of with each
other as use_zero_pages is disabled, and then these zero-pages are no longer
managed and monitor by KSM, which leads to two issues at least:

1) MADV_UNMERGEABLE and other ways to trigger unsharing will *not*
unshare the shared zeropage as placed by KSM (which is against the
MADV_UNMERGEABLE documentation at least); see the link:
https://lore.kernel.org/lkml/[email protected]/

2) we cannot know how many pages are zero pages placed by KSM when
enabling use_zero_pages, which leads to KSM not being transparent
with all actual merged pages by KSM.

Zero pages may be the most common merged pages in actual environment(not only VM but
also including other application like containers). Enabling use_zero_pages in the
environment with plenty of empty pages(full of zeros) will be very useful. Users and
app developer can also benefit from knowing the proportion of zero pages in all
merged pages to optimize applications.

With the patch series, we can both unshare zero-pages(KSM-placed) accurately
and count ksm zero pages with enabling use_zero_pages.

---
v2->v3:

1) Add more descriptive information in cover letter.

2) In [patch 2/5], add more commit log for explaining reasons.

3) In [patch 2/5], fix misuse of break_ksm() in unmerge_ksm_pages():
break_ksm(vma, addr, NULL) -> break_ksm(vma, addr, false);

---
v1->v2:

[patch 4/5] fix build warning, mm/ksm.c:550, misleading indentation; statement
'rmap_item->mm->ksm_zero_pages_sharing--;' is not part of the previous 'if'.



*** BLURB HERE ***

xu xin (5):
ksm: abstract the function try_to_get_old_rmap_item
ksm: support unsharing zero pages placed by KSM
ksm: count all zero pages placed by KSM
ksm: count zero pages for each process
ksm: add zero_pages_sharing documentation

Documentation/admin-guide/mm/ksm.rst | 10 +-
fs/proc/base.c | 1 +
include/linux/mm_types.h | 7 +-
mm/ksm.c | 177 +++++++++++++++++++++------
4 files changed, 157 insertions(+), 38 deletions(-)

--
2.25.1


2022-10-11 03:44:02

by xu

[permalink] [raw]
Subject: [PATCH v3 5/5] ksm: add zero_pages_sharing documentation

From: xu xin <[email protected]>

When enabling use_zero_pages, pages_sharing cannot represent how
much memory saved indeed. zero_pages_sharing + pages_sharing does.
add the description of zero_pages_sharing.

Cc: Xiaokai Ran <[email protected]>
Cc: Yang Yang <[email protected]>
Cc: Jiang Xuexin <[email protected]>
Cc: Claudio Imbrenda <[email protected]>
Cc: David Hildenbrand <[email protected]>
Signed-off-by: xu xin <[email protected]>
---
Documentation/admin-guide/mm/ksm.rst | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
index fb6ba2002a4b..484665aa7418 100644
--- a/Documentation/admin-guide/mm/ksm.rst
+++ b/Documentation/admin-guide/mm/ksm.rst
@@ -162,7 +162,7 @@ The effectiveness of KSM and MADV_MERGEABLE is shown in ``/sys/kernel/mm/ksm/``:
pages_shared
how many shared pages are being used
pages_sharing
- how many more sites are sharing them i.e. how much saved
+ how many more sites are sharing them
pages_unshared
how many pages unique but repeatedly checked for merging
pages_volatile
@@ -173,6 +173,14 @@ stable_node_chains
the number of KSM pages that hit the ``max_page_sharing`` limit
stable_node_dups
number of duplicated KSM pages
+zero_pages_sharing
+ how many empty pages are sharing kernel zero page(s) instead of
+ with each other as it would happen normally. Only effective when
+ enabling ``use_zero_pages`` knob.
+
+If ``use_zero_pages`` is 0, only ``pages_sharing`` can represents how
+much saved. Otherwise, ``pages_sharing`` + ``zero_pages_sharing``
+represents how much saved actually.

A high ratio of ``pages_sharing`` to ``pages_shared`` indicates good
sharing, but a high ratio of ``pages_unshared`` to ``pages_sharing``
--
2.25.1

2022-10-17 23:59:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ksm: support tracking KSM-placed zero-pages

On Tue, 11 Oct 2022 02:20:06 +0000 [email protected] wrote:

> From: xu xin <[email protected]>
>
> use_zero_pages is good, not just because of cache colouring as described
> in doc, but also because use_zero_pages can accelerate merging empty pages
> when there are plenty of empty pages (full of zeros) as the time of
> page-by-page comparisons (unstable_tree_search_insert) is saved.
>
> But there is something to improve, that is, when enabling use_zero_pages,
> all empty pages will be merged with kernel zero pages instead of with each
> other as use_zero_pages is disabled, and then these zero-pages are no longer
> managed and monitor by KSM, which leads to two issues at least:

Sorry, but I'm struggling to understand what real value this patchset
offers.

> 1) MADV_UNMERGEABLE and other ways to trigger unsharing will *not*
> unshare the shared zeropage as placed by KSM (which is against the
> MADV_UNMERGEABLE documentation at least); see the link:
> https://lore.kernel.org/lkml/[email protected]/

Is that causing users any real-world problem? If not, just change the
documentation?

> 2) we cannot know how many pages are zero pages placed by KSM when
> enabling use_zero_pages, which leads to KSM not being transparent
> with all actual merged pages by KSM.

Why is this a problem?

A full description of the real-world end-user operational benefits of
these changes would help, please.


2022-10-18 09:25:28

by xu

[permalink] [raw]
Subject: Re:[PATCH v3 0/5] ksm: support tracking KSM-placed zero-pages

>> From: xu xin <[email protected]>
>>
>> use_zero_pages is good, not just because of cache colouring as described
>> in doc, but also because use_zero_pages can accelerate merging empty pages
>> when there are plenty of empty pages (full of zeros) as the time of
>> page-by-page comparisons (unstable_tree_search_insert) is saved.
>>
>> But there is something to improve, that is, when enabling use_zero_pages,
>> all empty pages will be merged with kernel zero pages instead of with each
>> other as use_zero_pages is disabled, and then these zero-pages are no longer
>> managed and monitor by KSM, which leads to two issues at least:
>
>Sorry, but I'm struggling to understand what real value this patchset
>offers.
>
>> 1) MADV_UNMERGEABLE and other ways to trigger unsharing will *not*
>> unshare the shared zeropage as placed by KSM (which is against the
>> MADV_UNMERGEABLE documentation at least); see the link:
>> https://lore.kernel.org/lkml/[email protected]/
>
>Is that causing users any real-world problem? If not, just change the
>documentation?
>
>> 2) we cannot know how many pages are zero pages placed by KSM when
>> enabling use_zero_pages, which leads to KSM not being transparent
>> with all actual merged pages by KSM.
>
>Why is this a problem?
>
>A full description of the real-world end-user operational benefits of
>these changes would help, please.
>

The core idea of this patch set is to enable users to perceive the number of any
pages merged by KSM, regardless of whether use_zero_page switch has been turned
on, so that users can know how much free memory increase is really due to their
madvise(MERGEABLE) actions. The motivation for me to do this is that when I do
an application optimization of KSM on embedded Linux for 5G platform, I find
that ksm_merging_pages of some process becomes very small(but used to be large),
which led me to think that there was any problem with the application KSM-madvise
strategy, but in fact, it was only because use_zero_pages is on.

2022-10-18 23:06:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ksm: support tracking KSM-placed zero-pages

On Tue, 18 Oct 2022 09:00:22 +0000 xu xin <[email protected]> wrote:

> >A full description of the real-world end-user operational benefits of
> >these changes would help, please.
> >
>
> The core idea of this patch set is to enable users to perceive the number of any
> pages merged by KSM, regardless of whether use_zero_page switch has been turned
> on, so that users can know how much free memory increase is really due to their
> madvise(MERGEABLE) actions.

OK, thanks.

> The motivation for me to do this is that when I do
> an application optimization of KSM on embedded Linux for 5G platform, I find
> that ksm_merging_pages of some process becomes very small(but used to be large),
> which led me to think that there was any problem with the application KSM-madvise
> strategy, but in fact, it was only because use_zero_pages is on.

Please expand on the above motivation and experience, and include it in
the [0/n] changelog. But let's leave it a few days to see if there's
additional reviewer input.

2022-10-21 10:37:16

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ksm: support tracking KSM-placed zero-pages

On 19.10.22 00:54, Andrew Morton wrote:
> On Tue, 18 Oct 2022 09:00:22 +0000 xu xin <[email protected]> wrote:
>
>>> A full description of the real-world end-user operational benefits of
>>> these changes would help, please.
>>>
>>
>> The core idea of this patch set is to enable users to perceive the number of any
>> pages merged by KSM, regardless of whether use_zero_page switch has been turned
>> on, so that users can know how much free memory increase is really due to their
>> madvise(MERGEABLE) actions.
>
> OK, thanks.
>
>> The motivation for me to do this is that when I do
>> an application optimization of KSM on embedded Linux for 5G platform, I find
>> that ksm_merging_pages of some process becomes very small(but used to be large),
>> which led me to think that there was any problem with the application KSM-madvise
>> strategy, but in fact, it was only because use_zero_pages is on.
>
> Please expand on the above motivation and experience, and include it in
> the [0/n] changelog. But let's leave it a few days to see if there's
> additional reviewer input.
>

I just posted a selftest:

https://lore.kernel.org/all/[email protected]/T/#u

That could (should) be extended to test if unmerging works as expected.


Having that said, I think we really want a second pair of (KSM-expert)
eyes on these changes before moving forward with them.

--
Thanks,

David / dhildenb

2022-10-24 04:13:42

by xu

[permalink] [raw]
Subject: Re:[PATCH v3 0/5] ksm: support tracking KSM-placed zero-pages

>
>>>> A full description of the real-world end-user operational benefits of
>>>> these changes would help, please.
>>>>
>>>
>>> The core idea of this patch set is to enable users to perceive the number of any
>>> pages merged by KSM, regardless of whether use_zero_page switch has been turned
>>> on, so that users can know how much free memory increase is really due to their
>>> madvise(MERGEABLE) actions.
>>
>> OK, thanks.
>>
>>> The motivation for me to do this is that when I do
>>> an application optimization of KSM on embedded Linux for 5G platform, I find
>>> that ksm_merging_pages of some process becomes very small(but used to be large),
>>> which led me to think that there was any problem with the application KSM-madvise
>>> strategy, but in fact, it was only because use_zero_pages is on.
>>
>> Please expand on the above motivation and experience, and include it in
>> the [0/n] changelog. But let's leave it a few days to see if there's
>> additional reviewer input.
>>
>
>I just posted a selftest:
>
>https://lore.kernel.org/all/[email protected]/T/#u
>
>That could (should) be extended to test if unmerging works as expected.
>

Yes. As you said, these selftests can be extended to test if unsharing KSM-placed
zero pages works as expected, and I'm happy to do the extending after they are merged.

>
>Having that said, I think we really want a second pair of (KSM-expert)
>eyes on these changes before moving forward with them.

OK, don't worry. Let it be reviewed for a more time, so as to absorb more views later.
If necessary, I will resend the patches to adjust to break_ksm()'s changes.