Messages in this thread | | | Date | Thu, 16 Nov 2023 18:39:01 +0100 | Subject | Re: [PATCH] ksm: delay the check of splitting compound pages | From | David Hildenbrand <> |
| |
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
| |