Messages in this thread | | | Subject | Re: [PATCHv3, RESEND 0/8] thp/khugepaged improvements and CoW semantics | From | William Kucharski <> | Date | Tue, 14 Apr 2020 20:47:18 -0600 |
| |
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 <william.kucharski@oracle.com>
| |