| Message ID | 20220504214437.2850685-8-zokeefe@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: userspace hugepage collapse | expand |
On Wed, 4 May 2022, Zach O'Keefe wrote: > Add enforce_young flag to struct collapse_control that allows context to > ignore requirement that some pages in region being collapsed be young or > referenced. Set this flag in khugepaged collapse context to preserve > existing khugepaged behavior. > > This flag will be used (unset) when introducing madvise collapse > context since here, the user presumably has reason to believe the > collapse will be beneficial and khugepaged heuristics shouldn't tell > the user they are wrong. > > Signed-off-by: Zach O'Keefe <zokeefe@google.com> > --- > mm/khugepaged.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index ca730aec0e3e..b14807b7002e 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -90,6 +90,9 @@ struct collapse_control { > /* Respect khugepaged_max_ptes_[none|swap|shared] */ > bool enforce_pte_scan_limits; > > + /* Require memory to be young */ > + bool enforce_young; > + > /* Num pages scanned per node */ > int node_load[MAX_NUMNODES]; > Small nit: is it possible to unify these into a single bool khugepaged flag? Either way: Acked-by: David Rientjes <rientjes@google.com>
Thanks for the review, David! On Thu, May 12, 2022 at 1:03 PM David Rientjes <rientjes@google.com> wrote: > > On Wed, 4 May 2022, Zach O'Keefe wrote: > > > Add enforce_young flag to struct collapse_control that allows context to > > ignore requirement that some pages in region being collapsed be young or > > referenced. Set this flag in khugepaged collapse context to preserve > > existing khugepaged behavior. > > > > This flag will be used (unset) when introducing madvise collapse > > context since here, the user presumably has reason to believe the > > collapse will be beneficial and khugepaged heuristics shouldn't tell > > the user they are wrong. > > > > Signed-off-by: Zach O'Keefe <zokeefe@google.com> > > --- > > mm/khugepaged.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > index ca730aec0e3e..b14807b7002e 100644 > > --- a/mm/khugepaged.c > > +++ b/mm/khugepaged.c > > @@ -90,6 +90,9 @@ struct collapse_control { > > /* Respect khugepaged_max_ptes_[none|swap|shared] */ > > bool enforce_pte_scan_limits; > > > > + /* Require memory to be young */ > > + bool enforce_young; > > + > > /* Num pages scanned per node */ > > int node_load[MAX_NUMNODES]; > > > > Small nit: is it possible to unify these into a single bool khugepaged > flag? > I think this is fine with a "enforce_page_heuristics" flag or something. Ofc, functionally unifying the flags is a noop, but I was trying to to avoid "if (khugepagd) .. else .." and, where possible, describe the variable properties of the collapse in struct collapse_control itself. If there's no objections, I'll make that change for v6. > Either way: > > Acked-by: David Rientjes <rientjes@google.com>
diff --git a/mm/khugepaged.c b/mm/khugepaged.c index ca730aec0e3e..b14807b7002e 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -90,6 +90,9 @@ struct collapse_control { /* Respect khugepaged_max_ptes_[none|swap|shared] */ bool enforce_pte_scan_limits; + /* Require memory to be young */ + bool enforce_young; + /* Num pages scanned per node */ int node_load[MAX_NUMNODES]; @@ -720,9 +723,10 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma, list_add_tail(&page->lru, compound_pagelist); next: /* There should be enough young pte to collapse the page */ - if (pte_young(pteval) || - page_is_young(page) || PageReferenced(page) || - mmu_notifier_test_young(vma->vm_mm, address)) + if (cc->enforce_young && + (pte_young(pteval) || page_is_young(page) || + PageReferenced(page) || mmu_notifier_test_young(vma->vm_mm, + address))) referenced++; if (pte_write(pteval)) @@ -731,7 +735,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma, if (unlikely(!writable)) { result = SCAN_PAGE_RO; - } else if (unlikely(!referenced)) { + } else if (unlikely(cc->enforce_young && !referenced)) { result = SCAN_LACK_REFERENCED_PAGE; } else { result = SCAN_SUCCEED; @@ -1387,14 +1391,16 @@ static int khugepaged_scan_pmd(struct mm_struct *mm, struct vm_area_struct *vma, result = SCAN_PAGE_COUNT; goto out_unmap; } - if (pte_young(pteval) || - page_is_young(page) || PageReferenced(page) || - mmu_notifier_test_young(vma->vm_mm, address)) + if (cc->enforce_young && + (pte_young(pteval) || page_is_young(page) || + PageReferenced(page) || mmu_notifier_test_young(vma->vm_mm, + address))) referenced++; } if (!writable) { result = SCAN_PAGE_RO; - } else if (!referenced || (unmapped && referenced < HPAGE_PMD_NR/2)) { + } else if (cc->enforce_young && (!referenced || (unmapped && referenced + < HPAGE_PMD_NR / 2))) { result = SCAN_LACK_REFERENCED_PAGE; } else { result = SCAN_SUCCEED; @@ -2343,6 +2349,7 @@ static int khugepaged(void *none) struct mm_slot *mm_slot; struct collapse_control cc = { .enforce_pte_scan_limits = true, + .enforce_young = true, .last_target_node = NUMA_NO_NODE, .alloc_charge_hpage = &alloc_charge_hpage, };
Add enforce_young flag to struct collapse_control that allows context to ignore requirement that some pages in region being collapsed be young or referenced. Set this flag in khugepaged collapse context to preserve existing khugepaged behavior. This flag will be used (unset) when introducing madvise collapse context since here, the user presumably has reason to believe the collapse will be beneficial and khugepaged heuristics shouldn't tell the user they are wrong. Signed-off-by: Zach O'Keefe <zokeefe@google.com> --- mm/khugepaged.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)