The page can be included into collapse as long as it doesn't have extra
pins (from GUP or otherwise).
Logic to check the refcound is moved to a separate function.
Note that the function is ready to deal with compound pages. It's
preparation for the following patch.
VM_BUG_ON_PAGE() was removed from __collapse_huge_page_copy() as the
invariant it checks is no longer valid: the source can be mapped
multiple times now.
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
mm/khugepaged.c | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index e3e41c2768d8..f9864644c3b7 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -529,6 +529,24 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte)
}
}
+static bool is_refcount_suitable(struct page *page)
+{
+ int expected_refcount, refcount;
+
+ refcount = page_count(page);
+ expected_refcount = total_mapcount(page);
+ if (PageSwapCache(page))
+ expected_refcount += compound_nr(page);
+
+ if (IS_ENABLED(CONFIG_DEBUG_VM) && expected_refcount > refcount) {
+ pr_err("expected_refcount: %d, refcount: %d\n",
+ expected_refcount, refcount);
+ dump_page(page, "Unexpected refcount");
+ }
+
+ return page_count(page) == expected_refcount;
+}
+
static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
unsigned long address,
pte_t *pte)
@@ -581,11 +599,17 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
}
/*
- * cannot use mapcount: can't collapse if there's a gup pin.
- * The page must only be referenced by the scanned process
- * and page swap cache.
+ * Check if the page has any GUP (or other external) pins.
+ *
+ * The page table that maps the page has been already unlinked
+ * from the page table tree and this process cannot get
+ * an additinal pin on the page.
+ *
+ * New pins can come later if the page is shared across fork,
+ * but not for the this process. It is fine. The other process
+ * cannot write to the page, only trigger CoW.
*/
- if (page_count(page) != 1 + PageSwapCache(page)) {
+ if (!is_refcount_suitable(page)) {
unlock_page(page);
result = SCAN_PAGE_COUNT;
goto out;
@@ -672,7 +696,6 @@ static void __collapse_huge_page_copy(pte_t *pte, struct page *page,
} else {
src_page = pte_page(pteval);
copy_user_highpage(page, src_page, address, vma);
- VM_BUG_ON_PAGE(page_mapcount(src_page) != 1, src_page);
release_pte_page(src_page);
/*
* ptl mostly unnecessary, but preempt has to
@@ -1201,12 +1224,8 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
goto out_unmap;
}
- /*
- * cannot use mapcount: can't collapse if there's a gup pin.
- * The page must only be referenced by the scanned process
- * and page swap cache.
- */
- if (page_count(page) != 1 + PageSwapCache(page)) {
+ /* Check if the page has any GUP (or other external) pins */
+ if (!is_refcount_suitable(page)) {
result = SCAN_PAGE_COUNT;
goto out_unmap;
}
--
2.26.0
Quick comments, mostly nits:
[PATCH 5/6] khugepaged: Allow to collapse a page shared across fork
> +static bool is_refcount_suitable(struct page *page)
> +{
> + int expected_refcount, refcount;
> +
> + refcount = page_count(page);
> + expected_refcount = total_mapcount(page);
> + if (PageSwapCache(page))
> + expected_refcount += compound_nr(page);
> +
> + if (IS_ENABLED(CONFIG_DEBUG_VM) && expected_refcount > refcount) {
> + pr_err("expected_refcount: %d, refcount: %d\n",
> + expected_refcount, refcount);
I'd rather see this message reworded slightly as I prefer to know WHY a value
was unexpected directly from the error message:
pr_err("expected_refcount (%d) > refcount (%d)\n",
[PATCHv3, RESEND 7/8] thp: Change CoW semantics for anon-THP
I really like the simplifications here.
[PATCHv3, RESEND 8/8] khugepaged: Introduce 'max_ptes_shared' tunable
As above, I'd like to see a tiny extra bit of specificity in the printf()s:
+static void collapse_max_ptes_shared()
+{
[ ... ]
+ if (check_huge(p))
+ success("OK");
+ else
+ fail("Fail");
+
+ printf("Trigger CoW in %d of %d...",
printf("Trigger CoW on page %d of %d...",
+ hpage_pmd_nr - max_ptes_shared - 1, hpage_pmd_nr);
+ fill_memory(p, 0, (hpage_pmd_nr - max_ptes_shared - 1) * page_size);
+ if (!check_huge(p))
+ success("OK");
+ else
+ fail("Fail");
+
+ if (wait_for_scan("Do not collapse with max_ptes_shared exeeded", p))
+ fail("Timeout");
+ else if (!check_huge(p))
+ success("OK");
+ else
+ fail("Fail");
+
+ printf("Trigger CoW in %d of %d...",
printf("Trigger CoW on page %d of %d...",
+ hpage_pmd_nr - max_ptes_shared, hpage_pmd_nr);
+ fill_memory(p, 0, (hpage_pmd_nr - max_ptes_shared) * page_size);
+ if (!check_huge(p))
+ success("OK");
+ else
+ fail("Fail");
Otherwise, for the khugepaged/THP series:
Reviewed-by: William Kucharski <[email protected]>