lkml.org 
[lkml]   [2023]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] ksm: delay the check of splitting compound pages
From
On 14.11.23 13:36, yang.yang29@zte.com.cn wrote:
> From: xu xin <xu.xin16@zte.com.cn>
>
> Background
> ==========
> When trying to merge two pages, it may fail because the two pages
> belongs to the same compound page and split_huge_page fails due to
> the incorrect reference to the page. To solve the problem, the commit
> 77da2ba0648a4 ("mm/ksm: fix interaction with THP") tries to split the
> compound page after try_to_merge_two_pages() fails and put_page in
> that case. However it is too early to calculate of the variable 'split' which
> indicates whether the two pages belongs to the same compound page.
>
> What to do
> ==========
> If try_to_merge_two_pages() succeeds, there is no need to check whether
> to splitting compound pages. So we delay the check of splitting compound
> pages until try_to_merge_two_pages() fails, which can improve the
> processing efficiency of cmp_and_merge_page() a little.
>
> Signed-off-by: xu xin <xu.xin16@zte.com.cn>
> Reviewed-by: Yang Yang <yang.yang29@zte.com.cn>
> ---
> mm/ksm.c | 36 ++++++++++++++++++++----------------
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 7efcc68ccc6e..c952fe5d9e43 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -2229,24 +2229,10 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite
> tree_rmap_item =
> unstable_tree_search_insert(rmap_item, page, &tree_page);
> if (tree_rmap_item) {
> - bool split;
> -
> kpage = try_to_merge_two_pages(rmap_item, page,
> tree_rmap_item, tree_page);
> - /*
> - * If both pages we tried to merge belong to the same compound
> - * page, then we actually ended up increasing the reference
> - * count of the same compound page twice, and split_huge_page
> - * failed.
> - * Here we set a flag if that happened, and we use it later to
> - * try split_huge_page again. Since we call put_page right
> - * afterwards, the reference count will be correct and
> - * split_huge_page should succeed.
> - */

I'm curious, why can't we detect that ahead of time and keep only a
single reference? Why do we need the backup code? Anything I am missing?

> - split = PageTransCompound(page)
> - && compound_head(page) == compound_head(tree_page);
> - put_page(tree_page);
> if (kpage) {
> + put_page(tree_page);
> /*
> * The pages were successfully merged: insert new
> * node in the stable tree and add both rmap_items.
> @@ -2271,7 +2257,25 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite
> break_cow(tree_rmap_item);
> break_cow(rmap_item);
> }
> - } else if (split) {
> + } else {
> + bool split;
> + /*
> + * If both pages we tried to merge belong to the same compound
> + * page, then we actually ended up increasing the reference
> + * count of the same compound page twice, and split_huge_page
> + * failed.
> + * Here we set a flag if that happened, and we use it later to
> + * try split_huge_page again. Since we call put_page right
> + * afterwards, the reference count will be correct and
> + * split_huge_page should succeed.
> + */
> +
> + split = PageTransCompound(page)
> + && compound_head(page) == compound_head(tree_page);

Would

split = page_folio(page) == page_folio(tree_page);

do the trick? No need to mess with compound pages.

--
Cheers,

David / dhildenb

\
 
 \ /
  Last update: 2023-11-20 13:59    [W:0.358 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site