| Message ID | 20221009021816.315205-1-xu.xin16@zte.com.cn (mailing list archive) |
|---|---|
| Headers | show |
| Series | ksm: support tracking KSM-placed zero-pages | expand |
On Sun, 9 Oct 2022 02:18:16 +0000 xu.xin.sc@gmail.com wrote: > From: xu xin <xu.xin16@zte.com.cn> > > Before enabling use_zero_pages by setting /sys/kernel/mm/ksm/ > use_zero_pages to 1, pages_sharing of KSM is basically accurate. But > when enabling use_zero_pages, all empty pages that are merged with > kernel zero page are not counted in pages_sharing or pages_shared. > That is because these empty pages are merged with zero-pages then no > longer managed 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/4a3daba6-18f9-d252-697c-197f65578c44@redhat.com/ > > 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. > > With the patch series, we can unshare zero-pages(KSM-placed) accurately > and count ksm zero pages. why are you trying so hard to fix something that is not broken? can't you just avoid using use_zero_pages? why is it so important to know how many zero pages have been merged? and why do you want to unmerge them? the important thing is that the sysadmin knows how much memory would be needed to do the unmerge, and that information is already there. > > --- > 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(-) >
Hello, Thanks for your reply. > >why are you trying so hard to fix something that is not broken? Actually, it breaks the definition of unmerge, though it's usually not a big problem. > >can't you just avoid using use_zero_pages? 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 comparision (unstable_tree_search_insert) is saved. > >why is it so important to know how many zero pages have been merged? >and why do you want to unmerge them? Zero pages may be the most common merged pages in actual environment(not only VM but also including other application like containers). Sometimes customers (app developers) are very interested in how many non-zero-pages are actually merged in their apps. > >the important thing is that the sysadmin knows how much memory would be >needed to do the unmerge, and that information is already there. > I think it's about chicken-and-egg problem. Anyway, thanks for your reply.
On Mon, 10 Oct 2022 12:08:34 +0000 xu xin <xu.xin.sc@gmail.com> wrote: > Hello, Thanks for your reply. > > > > >why are you trying so hard to fix something that is not broken? > > Actually, it breaks the definition of unmerge, though it's usually not a big > problem. > > > >can't you just avoid using use_zero_pages? > > 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 comparision > (unstable_tree_search_insert) is saved. interesting, this is some useful information that you could have written in the cover letter and/or commit messages, to explain why you are proposing these changes :) > > > > >why is it so important to know how many zero pages have been merged? > >and why do you want to unmerge them? > > Zero pages may be the most common merged pages in actual environment(not only VM but also interesting information, which you could also have written in the cover letter and/or commit messages > also including other application like containers). Sometimes customers (app developers) > are very interested in how many non-zero-pages are actually merged in their apps. > > > > >the important thing is that the sysadmin knows how much memory would be > >needed to do the unmerge, and that information is already there. > > > > I think it's about chicken-and-egg problem. > > > Anyway, thanks for your reply. >
> >> Hello, Thanks for your reply. >> >> > >> >why are you trying so hard to fix something that is not broken? >> >> Actually, it breaks the definition of unmerge, though it's usually not a big >> problem. >> > >> >can't you just avoid using use_zero_pages? >> >> 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 comparision >> (unstable_tree_search_insert) is saved. > >interesting, this is some useful information that you could have written >in the cover letter and/or commit messages, to explain why you are >proposing these changes :) Yes. I have done it in v3. Thanks. > >> >> > >> >why is it so important to know how many zero pages have been merged? >> >and why do you want to unmerge them? >> >> Zero pages may be the most common merged pages in actual environment(not only VM but > >also interesting information, which you could also have written in the >cover letter and/or commit messages > >> also including other application like containers). Sometimes customers (app developers) >> are very interested in how many non-zero-pages are actually merged in their apps. >> >> > >> >the important thing is that the sysadmin knows how much memory would be >> >needed to do the unmerge, and that information is already there. >> > >> >> I think it's about chicken-and-egg problem. >> >> >> Anyway, thanks for your reply. >>
From: xu xin <xu.xin16@zte.com.cn> Before enabling use_zero_pages by setting /sys/kernel/mm/ksm/ use_zero_pages to 1, pages_sharing of KSM is basically accurate. But when enabling use_zero_pages, all empty pages that are merged with kernel zero page are not counted in pages_sharing or pages_shared. That is because these empty pages are merged with zero-pages then no longer managed 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/4a3daba6-18f9-d252-697c-197f65578c44@redhat.com/ 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. With the patch series, we can unshare zero-pages(KSM-placed) accurately and count ksm zero pages. --- 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(-)