lkml.org 
[lkml]   [2023]   [Nov]   [16]   [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 16.11.23 13:17, xu wrote:
>>>>> @@ -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?
>
> Do you mean like this?

Let me see if the refcounting here is sane:

(a) The caller has a reference on "page" that it will put just after the
cmp_and_merge_page() call.
(b) unstable_tree_search_insert() obtained a reference to the
"tree_page" using get_mergeable_page()->follow_page(). We will put
that reference.

So indeed, if both references are to the same folio, *we* have two
references to the folio and can safely drop one of both.

>
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -2229,23 +2229,21 @@ 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;
> + bool SameCompound;
> + /*
> + * If they belongs to the same compound page, its' reference
> + * get twice, so need to put_page once to avoid that
> + * split_huge_page fails in try_to_merge_two_pages().
> + */
> + if (SameCompound = Is_SameCompound(page, tree_page))
> + put_page(tree_page);
>

bool same_folio = page_folio(page) == page_folio(tree_page);

/*
* If both pages belong to the same folio, we are holding two references
* to the same large folio: splitting the folio in
* try_to_merge_one_page() will fail for that reason. So let's just drop
* one reference early. Note that this is only possible if tree_page is
* not a KSM page yet.
*/
if (same_folio)
put_page(tree_page);

[we could use more folio operations here, but lets KIS]

--
Cheers,

David / dhildenb

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