| Message ID | 20250211003028.213461-2-npache@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | khugepaged: mTHP support | expand |
On 11/02/2025 00:30, Nico Pache wrote: > The khugepaged daemon and madvise_collapse have two different > implementations that do almost the same thing. > > Create khugepaged_collapse_single_pmd to increase code > reuse and create an entry point for future khugepaged changes. > > Refactor madvise_collapse and khugepaged_scan_mm_slot to use > the new khugepaged_collapse_single_pmd function. > > Signed-off-by: Nico Pache <npache@redhat.com> > --- > mm/khugepaged.c | 96 +++++++++++++++++++++++++------------------------ > 1 file changed, 50 insertions(+), 46 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 5f0be134141e..46faee67378b 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -2365,6 +2365,52 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, > } > #endif > > +/* > + * Try to collapse a single PMD starting at a PMD aligned addr, and return > + * the results. > + */ > +static int khugepaged_collapse_single_pmd(unsigned long addr, struct mm_struct *mm, > + struct vm_area_struct *vma, bool *mmap_locked, > + struct collapse_control *cc) > +{ > + int result = SCAN_FAIL; > + unsigned long tva_flags = cc->is_khugepaged ? TVA_ENFORCE_SYSFS : 0; > + > + if (!*mmap_locked) { > + mmap_read_lock(mm); > + *mmap_locked = true; > + } > + > + if (thp_vma_allowable_order(vma, vma->vm_flags, > + tva_flags, PMD_ORDER)) { > + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > + struct file *file = get_file(vma->vm_file); > + pgoff_t pgoff = linear_page_index(vma, addr); > + > + mmap_read_unlock(mm); > + *mmap_locked = false; > + result = hpage_collapse_scan_file(mm, addr, file, pgoff, > + cc); > + fput(file); > + if (result == SCAN_PTE_MAPPED_HUGEPAGE) { > + mmap_read_lock(mm); > + if (hpage_collapse_test_exit_or_disable(mm)) > + goto end; > + result = collapse_pte_mapped_thp(mm, addr, > + !cc->is_khugepaged); > + mmap_read_unlock(mm); > + } > + } else { > + result = hpage_collapse_scan_pmd(mm, vma, addr, > + mmap_locked, cc); > + } > + if (result == SCAN_SUCCEED || result == SCAN_PMD_MAPPED) > + ++khugepaged_pages_collapsed; > + } > +end: > + return result; > +} > + > static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > struct collapse_control *cc) > __releases(&khugepaged_mm_lock) > @@ -2439,33 +2485,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > VM_BUG_ON(khugepaged_scan.address < hstart || > khugepaged_scan.address + HPAGE_PMD_SIZE > > hend); > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > - struct file *file = get_file(vma->vm_file); > - pgoff_t pgoff = linear_page_index(vma, > - khugepaged_scan.address); > > - mmap_read_unlock(mm); > - mmap_locked = false; > - *result = hpage_collapse_scan_file(mm, > - khugepaged_scan.address, file, pgoff, cc); > - fput(file); > - if (*result == SCAN_PTE_MAPPED_HUGEPAGE) { > - mmap_read_lock(mm); > - if (hpage_collapse_test_exit_or_disable(mm)) > - goto breakouterloop; > - *result = collapse_pte_mapped_thp(mm, > - khugepaged_scan.address, false); > - if (*result == SCAN_PMD_MAPPED) > - *result = SCAN_SUCCEED; > - mmap_read_unlock(mm); > - } > - } else { > - *result = hpage_collapse_scan_pmd(mm, vma, > - khugepaged_scan.address, &mmap_locked, cc); > - } > - > - if (*result == SCAN_SUCCEED) > - ++khugepaged_pages_collapsed; > + *result = khugepaged_collapse_single_pmd(khugepaged_scan.address, > + mm, vma, &mmap_locked, cc); > > /* move to next address */ > khugepaged_scan.address += HPAGE_PMD_SIZE; > @@ -2785,36 +2807,18 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev, > mmap_assert_locked(mm); > memset(cc->node_load, 0, sizeof(cc->node_load)); > nodes_clear(cc->alloc_nmask); > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > - struct file *file = get_file(vma->vm_file); > - pgoff_t pgoff = linear_page_index(vma, addr); > > - mmap_read_unlock(mm); > - mmap_locked = false; > - result = hpage_collapse_scan_file(mm, addr, file, pgoff, > - cc); > - fput(file); > - } else { > - result = hpage_collapse_scan_pmd(mm, vma, addr, > - &mmap_locked, cc); > - } > + result = khugepaged_collapse_single_pmd(addr, mm, vma, &mmap_locked, cc); > + you will be incrementing khugepaged_pages_collapsed at madvise_collapse by calling khugepaged_collapse_single_pmd which is not correct. > if (!mmap_locked) > *prev = NULL; /* Tell caller we dropped mmap_lock */ > > -handle_result: > switch (result) { > case SCAN_SUCCEED: > case SCAN_PMD_MAPPED: > ++thps; > break; > case SCAN_PTE_MAPPED_HUGEPAGE: > - BUG_ON(mmap_locked); > - BUG_ON(*prev); > - mmap_read_lock(mm); > - result = collapse_pte_mapped_thp(mm, addr, true); > - mmap_read_unlock(mm); > - goto handle_result; > - /* Whitelisted set of results where continuing OK */ > case SCAN_PMD_NULL: > case SCAN_PTE_NON_PRESENT: > case SCAN_PTE_UFFD_WP:
On Mon, Feb 17, 2025 at 10:11 AM Usama Arif <usamaarif642@gmail.com> wrote: > > > > On 11/02/2025 00:30, Nico Pache wrote: > > The khugepaged daemon and madvise_collapse have two different > > implementations that do almost the same thing. > > > > Create khugepaged_collapse_single_pmd to increase code > > reuse and create an entry point for future khugepaged changes. > > > > Refactor madvise_collapse and khugepaged_scan_mm_slot to use > > the new khugepaged_collapse_single_pmd function. > > > > Signed-off-by: Nico Pache <npache@redhat.com> > > --- > > mm/khugepaged.c | 96 +++++++++++++++++++++++++------------------------ > > 1 file changed, 50 insertions(+), 46 deletions(-) > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > index 5f0be134141e..46faee67378b 100644 > > --- a/mm/khugepaged.c > > +++ b/mm/khugepaged.c > > @@ -2365,6 +2365,52 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, > > } > > #endif > > > > +/* > > + * Try to collapse a single PMD starting at a PMD aligned addr, and return > > + * the results. > > + */ > > +static int khugepaged_collapse_single_pmd(unsigned long addr, struct mm_struct *mm, > > + struct vm_area_struct *vma, bool *mmap_locked, > > + struct collapse_control *cc) > > +{ > > + int result = SCAN_FAIL; > > + unsigned long tva_flags = cc->is_khugepaged ? TVA_ENFORCE_SYSFS : 0; > > + > > + if (!*mmap_locked) { > > + mmap_read_lock(mm); > > + *mmap_locked = true; > > + } > > + > > + if (thp_vma_allowable_order(vma, vma->vm_flags, > > + tva_flags, PMD_ORDER)) { > > + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > > + struct file *file = get_file(vma->vm_file); > > + pgoff_t pgoff = linear_page_index(vma, addr); > > + > > + mmap_read_unlock(mm); > > + *mmap_locked = false; > > + result = hpage_collapse_scan_file(mm, addr, file, pgoff, > > + cc); > > + fput(file); > > + if (result == SCAN_PTE_MAPPED_HUGEPAGE) { > > + mmap_read_lock(mm); > > + if (hpage_collapse_test_exit_or_disable(mm)) > > + goto end; > > + result = collapse_pte_mapped_thp(mm, addr, > > + !cc->is_khugepaged); > > + mmap_read_unlock(mm); > > + } > > + } else { > > + result = hpage_collapse_scan_pmd(mm, vma, addr, > > + mmap_locked, cc); > > + } > > + if (result == SCAN_SUCCEED || result == SCAN_PMD_MAPPED) > > + ++khugepaged_pages_collapsed; > > + } > > +end: > > + return result; > > +} > > + > > static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > > struct collapse_control *cc) > > __releases(&khugepaged_mm_lock) > > @@ -2439,33 +2485,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > > VM_BUG_ON(khugepaged_scan.address < hstart || > > khugepaged_scan.address + HPAGE_PMD_SIZE > > > hend); > > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > > - struct file *file = get_file(vma->vm_file); > > - pgoff_t pgoff = linear_page_index(vma, > > - khugepaged_scan.address); > > > > - mmap_read_unlock(mm); > > - mmap_locked = false; > > - *result = hpage_collapse_scan_file(mm, > > - khugepaged_scan.address, file, pgoff, cc); > > - fput(file); > > - if (*result == SCAN_PTE_MAPPED_HUGEPAGE) { > > - mmap_read_lock(mm); > > - if (hpage_collapse_test_exit_or_disable(mm)) > > - goto breakouterloop; > > - *result = collapse_pte_mapped_thp(mm, > > - khugepaged_scan.address, false); > > - if (*result == SCAN_PMD_MAPPED) > > - *result = SCAN_SUCCEED; > > - mmap_read_unlock(mm); > > - } > > - } else { > > - *result = hpage_collapse_scan_pmd(mm, vma, > > - khugepaged_scan.address, &mmap_locked, cc); > > - } > > - > > - if (*result == SCAN_SUCCEED) > > - ++khugepaged_pages_collapsed; > > + *result = khugepaged_collapse_single_pmd(khugepaged_scan.address, > > + mm, vma, &mmap_locked, cc); > > > > /* move to next address */ > > khugepaged_scan.address += HPAGE_PMD_SIZE; > > @@ -2785,36 +2807,18 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev, > > mmap_assert_locked(mm); > > memset(cc->node_load, 0, sizeof(cc->node_load)); > > nodes_clear(cc->alloc_nmask); > > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > > - struct file *file = get_file(vma->vm_file); > > - pgoff_t pgoff = linear_page_index(vma, addr); > > > > - mmap_read_unlock(mm); > > - mmap_locked = false; > > - result = hpage_collapse_scan_file(mm, addr, file, pgoff, > > - cc); > > - fput(file); > > - } else { > > - result = hpage_collapse_scan_pmd(mm, vma, addr, > > - &mmap_locked, cc); > > - } > > + result = khugepaged_collapse_single_pmd(addr, mm, vma, &mmap_locked, cc); > > + > > you will be incrementing khugepaged_pages_collapsed at madvise_collapse by calling > khugepaged_collapse_single_pmd which is not correct. Ah good catch, Ill add a conditional around the incrementation. Thanks! > > > if (!mmap_locked) > > *prev = NULL; /* Tell caller we dropped mmap_lock */ > > > > -handle_result: > > switch (result) { > > case SCAN_SUCCEED: > > case SCAN_PMD_MAPPED: > > ++thps; > > break; > > case SCAN_PTE_MAPPED_HUGEPAGE: > > - BUG_ON(mmap_locked); > > - BUG_ON(*prev); > > - mmap_read_lock(mm); > > - result = collapse_pte_mapped_thp(mm, addr, true); > > - mmap_read_unlock(mm); > > - goto handle_result; > > - /* Whitelisted set of results where continuing OK */ > > case SCAN_PMD_NULL: > > case SCAN_PTE_NON_PRESENT: > > case SCAN_PTE_UFFD_WP: >
On 11/02/2025 00:30, Nico Pache wrote: > The khugepaged daemon and madvise_collapse have two different > implementations that do almost the same thing. > > Create khugepaged_collapse_single_pmd to increase code > reuse and create an entry point for future khugepaged changes. > > Refactor madvise_collapse and khugepaged_scan_mm_slot to use > the new khugepaged_collapse_single_pmd function. > > Signed-off-by: Nico Pache <npache@redhat.com> > --- > mm/khugepaged.c | 96 +++++++++++++++++++++++++------------------------ > 1 file changed, 50 insertions(+), 46 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 5f0be134141e..46faee67378b 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -2365,6 +2365,52 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, > } > #endif > > +/* > + * Try to collapse a single PMD starting at a PMD aligned addr, and return > + * the results. > + */ > +static int khugepaged_collapse_single_pmd(unsigned long addr, struct mm_struct *mm, > + struct vm_area_struct *vma, bool *mmap_locked, > + struct collapse_control *cc) nit: given the vma links to the mm is it really neccessary to pass both? Why not just pass vma? > +{ > + int result = SCAN_FAIL; > + unsigned long tva_flags = cc->is_khugepaged ? TVA_ENFORCE_SYSFS : 0; > + > + if (!*mmap_locked) { > + mmap_read_lock(mm); > + *mmap_locked = true; > + } AFAICT, the read lock is always held when khugepaged_collapse_single_pmd() is called. Perhaps VM_WARN_ON(!*mmap_locked) would be more appropriate? > + > + if (thp_vma_allowable_order(vma, vma->vm_flags, > + tva_flags, PMD_ORDER)) { > + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { I guess it was like this before, but what's the relevance of CONFIG_SHMEM? Surely this should work for any file if CONFIG_READ_ONLY_THP_FOR_FS is enabled? > + struct file *file = get_file(vma->vm_file); > + pgoff_t pgoff = linear_page_index(vma, addr); > + > + mmap_read_unlock(mm); > + *mmap_locked = false; > + result = hpage_collapse_scan_file(mm, addr, file, pgoff, > + cc); > + fput(file); > + if (result == SCAN_PTE_MAPPED_HUGEPAGE) { > + mmap_read_lock(mm); > + if (hpage_collapse_test_exit_or_disable(mm)) > + goto end; > + result = collapse_pte_mapped_thp(mm, addr, > + !cc->is_khugepaged); > + mmap_read_unlock(mm); > + } > + } else { > + result = hpage_collapse_scan_pmd(mm, vma, addr, > + mmap_locked, cc); > + } > + if (result == SCAN_SUCCEED || result == SCAN_PMD_MAPPED) > + ++khugepaged_pages_collapsed; Looks like this counter was previously only incremented for the scan path, not for the madvise_collapse path. Not sure if that's a problem? Thanks, Ryan > + } > +end: > + return result; > +} > + > static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > struct collapse_control *cc) > __releases(&khugepaged_mm_lock) > @@ -2439,33 +2485,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > VM_BUG_ON(khugepaged_scan.address < hstart || > khugepaged_scan.address + HPAGE_PMD_SIZE > > hend); > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > - struct file *file = get_file(vma->vm_file); > - pgoff_t pgoff = linear_page_index(vma, > - khugepaged_scan.address); > > - mmap_read_unlock(mm); > - mmap_locked = false; > - *result = hpage_collapse_scan_file(mm, > - khugepaged_scan.address, file, pgoff, cc); > - fput(file); > - if (*result == SCAN_PTE_MAPPED_HUGEPAGE) { > - mmap_read_lock(mm); > - if (hpage_collapse_test_exit_or_disable(mm)) > - goto breakouterloop; > - *result = collapse_pte_mapped_thp(mm, > - khugepaged_scan.address, false); > - if (*result == SCAN_PMD_MAPPED) > - *result = SCAN_SUCCEED; > - mmap_read_unlock(mm); > - } > - } else { > - *result = hpage_collapse_scan_pmd(mm, vma, > - khugepaged_scan.address, &mmap_locked, cc); > - } > - > - if (*result == SCAN_SUCCEED) > - ++khugepaged_pages_collapsed; > + *result = khugepaged_collapse_single_pmd(khugepaged_scan.address, > + mm, vma, &mmap_locked, cc); > > /* move to next address */ > khugepaged_scan.address += HPAGE_PMD_SIZE; > @@ -2785,36 +2807,18 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev, > mmap_assert_locked(mm); > memset(cc->node_load, 0, sizeof(cc->node_load)); > nodes_clear(cc->alloc_nmask); > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > - struct file *file = get_file(vma->vm_file); > - pgoff_t pgoff = linear_page_index(vma, addr); > > - mmap_read_unlock(mm); > - mmap_locked = false; > - result = hpage_collapse_scan_file(mm, addr, file, pgoff, > - cc); > - fput(file); > - } else { > - result = hpage_collapse_scan_pmd(mm, vma, addr, > - &mmap_locked, cc); > - } > + result = khugepaged_collapse_single_pmd(addr, mm, vma, &mmap_locked, cc); > + > if (!mmap_locked) > *prev = NULL; /* Tell caller we dropped mmap_lock */ > > -handle_result: > switch (result) { > case SCAN_SUCCEED: > case SCAN_PMD_MAPPED: > ++thps; > break; > case SCAN_PTE_MAPPED_HUGEPAGE: > - BUG_ON(mmap_locked); > - BUG_ON(*prev); > - mmap_read_lock(mm); > - result = collapse_pte_mapped_thp(mm, addr, true); > - mmap_read_unlock(mm); > - goto handle_result; > - /* Whitelisted set of results where continuing OK */ > case SCAN_PMD_NULL: > case SCAN_PTE_NON_PRESENT: > case SCAN_PTE_UFFD_WP:
On Tue, Feb 18, 2025 at 9:26 AM Ryan Roberts <ryan.roberts@arm.com> wrote: > > On 11/02/2025 00:30, Nico Pache wrote: > > The khugepaged daemon and madvise_collapse have two different > > implementations that do almost the same thing. > > > > Create khugepaged_collapse_single_pmd to increase code > > reuse and create an entry point for future khugepaged changes. > > > > Refactor madvise_collapse and khugepaged_scan_mm_slot to use > > the new khugepaged_collapse_single_pmd function. > > > > Signed-off-by: Nico Pache <npache@redhat.com> > > --- > > mm/khugepaged.c | 96 +++++++++++++++++++++++++------------------------ > > 1 file changed, 50 insertions(+), 46 deletions(-) > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > index 5f0be134141e..46faee67378b 100644 > > --- a/mm/khugepaged.c > > +++ b/mm/khugepaged.c > > @@ -2365,6 +2365,52 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, > > } > > #endif > > > > +/* > > + * Try to collapse a single PMD starting at a PMD aligned addr, and return > > + * the results. > > + */ > > +static int khugepaged_collapse_single_pmd(unsigned long addr, struct mm_struct *mm, > > + struct vm_area_struct *vma, bool *mmap_locked, > > + struct collapse_control *cc) > > nit: given the vma links to the mm is it really neccessary to pass both? Why not > just pass vma? Ah good point! > > > +{ > > + int result = SCAN_FAIL; > > + unsigned long tva_flags = cc->is_khugepaged ? TVA_ENFORCE_SYSFS : 0; > > + > > + if (!*mmap_locked) { > > + mmap_read_lock(mm); > > + *mmap_locked = true; > > + } > > AFAICT, the read lock is always held when khugepaged_collapse_single_pmd() is > called. Perhaps VM_WARN_ON(!*mmap_locked) would be more appropriate? Hmm, I'm actually not sure why that's there. Its probably left over from a previous change I was trying a while back. > > > + > > + if (thp_vma_allowable_order(vma, vma->vm_flags, > > + tva_flags, PMD_ORDER)) { > > + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > > I guess it was like this before, but what's the relevance of CONFIG_SHMEM? > Surely this should work for any file if CONFIG_READ_ONLY_THP_FOR_FS is enabled? Yeah I think David brought up a similar point during one of our meetings. I'm just combining the collapse users, so I'll leave it for now. > > > + struct file *file = get_file(vma->vm_file); > > + pgoff_t pgoff = linear_page_index(vma, addr); > > + > > + mmap_read_unlock(mm); > > + *mmap_locked = false; > > + result = hpage_collapse_scan_file(mm, addr, file, pgoff, > > + cc); > > + fput(file); > > + if (result == SCAN_PTE_MAPPED_HUGEPAGE) { > > + mmap_read_lock(mm); > > + if (hpage_collapse_test_exit_or_disable(mm)) > > + goto end; > > + result = collapse_pte_mapped_thp(mm, addr, > > + !cc->is_khugepaged); > > + mmap_read_unlock(mm); > > + } > > + } else { > > + result = hpage_collapse_scan_pmd(mm, vma, addr, > > + mmap_locked, cc); > > + } > > + if (result == SCAN_SUCCEED || result == SCAN_PMD_MAPPED) > > + ++khugepaged_pages_collapsed; > > Looks like this counter was previously only incremented for the scan path, not > for the madvise_collapse path. Not sure if that's a problem? Yep! Usama noted that too, already fixed in the next version :) Thanks! -- Nico > > Thanks, > Ryan > > > + } > > +end: > > + return result; > > +} > > + > > static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > > struct collapse_control *cc) > > __releases(&khugepaged_mm_lock) > > @@ -2439,33 +2485,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, > > VM_BUG_ON(khugepaged_scan.address < hstart || > > khugepaged_scan.address + HPAGE_PMD_SIZE > > > hend); > > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > > - struct file *file = get_file(vma->vm_file); > > - pgoff_t pgoff = linear_page_index(vma, > > - khugepaged_scan.address); > > > > - mmap_read_unlock(mm); > > - mmap_locked = false; > > - *result = hpage_collapse_scan_file(mm, > > - khugepaged_scan.address, file, pgoff, cc); > > - fput(file); > > - if (*result == SCAN_PTE_MAPPED_HUGEPAGE) { > > - mmap_read_lock(mm); > > - if (hpage_collapse_test_exit_or_disable(mm)) > > - goto breakouterloop; > > - *result = collapse_pte_mapped_thp(mm, > > - khugepaged_scan.address, false); > > - if (*result == SCAN_PMD_MAPPED) > > - *result = SCAN_SUCCEED; > > - mmap_read_unlock(mm); > > - } > > - } else { > > - *result = hpage_collapse_scan_pmd(mm, vma, > > - khugepaged_scan.address, &mmap_locked, cc); > > - } > > - > > - if (*result == SCAN_SUCCEED) > > - ++khugepaged_pages_collapsed; > > + *result = khugepaged_collapse_single_pmd(khugepaged_scan.address, > > + mm, vma, &mmap_locked, cc); > > > > /* move to next address */ > > khugepaged_scan.address += HPAGE_PMD_SIZE; > > @@ -2785,36 +2807,18 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev, > > mmap_assert_locked(mm); > > memset(cc->node_load, 0, sizeof(cc->node_load)); > > nodes_clear(cc->alloc_nmask); > > - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { > > - struct file *file = get_file(vma->vm_file); > > - pgoff_t pgoff = linear_page_index(vma, addr); > > > > - mmap_read_unlock(mm); > > - mmap_locked = false; > > - result = hpage_collapse_scan_file(mm, addr, file, pgoff, > > - cc); > > - fput(file); > > - } else { > > - result = hpage_collapse_scan_pmd(mm, vma, addr, > > - &mmap_locked, cc); > > - } > > + result = khugepaged_collapse_single_pmd(addr, mm, vma, &mmap_locked, cc); > > + > > if (!mmap_locked) > > *prev = NULL; /* Tell caller we dropped mmap_lock */ > > > > -handle_result: > > switch (result) { > > case SCAN_SUCCEED: > > case SCAN_PMD_MAPPED: > > ++thps; > > break; > > case SCAN_PTE_MAPPED_HUGEPAGE: > > - BUG_ON(mmap_locked); > > - BUG_ON(*prev); > > - mmap_read_lock(mm); > > - result = collapse_pte_mapped_thp(mm, addr, true); > > - mmap_read_unlock(mm); > > - goto handle_result; > > - /* Whitelisted set of results where continuing OK */ > > case SCAN_PMD_NULL: > > case SCAN_PTE_NON_PRESENT: > > case SCAN_PTE_UFFD_WP: >
diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 5f0be134141e..46faee67378b 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -2365,6 +2365,52 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, } #endif +/* + * Try to collapse a single PMD starting at a PMD aligned addr, and return + * the results. + */ +static int khugepaged_collapse_single_pmd(unsigned long addr, struct mm_struct *mm, + struct vm_area_struct *vma, bool *mmap_locked, + struct collapse_control *cc) +{ + int result = SCAN_FAIL; + unsigned long tva_flags = cc->is_khugepaged ? TVA_ENFORCE_SYSFS : 0; + + if (!*mmap_locked) { + mmap_read_lock(mm); + *mmap_locked = true; + } + + if (thp_vma_allowable_order(vma, vma->vm_flags, + tva_flags, PMD_ORDER)) { + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { + struct file *file = get_file(vma->vm_file); + pgoff_t pgoff = linear_page_index(vma, addr); + + mmap_read_unlock(mm); + *mmap_locked = false; + result = hpage_collapse_scan_file(mm, addr, file, pgoff, + cc); + fput(file); + if (result == SCAN_PTE_MAPPED_HUGEPAGE) { + mmap_read_lock(mm); + if (hpage_collapse_test_exit_or_disable(mm)) + goto end; + result = collapse_pte_mapped_thp(mm, addr, + !cc->is_khugepaged); + mmap_read_unlock(mm); + } + } else { + result = hpage_collapse_scan_pmd(mm, vma, addr, + mmap_locked, cc); + } + if (result == SCAN_SUCCEED || result == SCAN_PMD_MAPPED) + ++khugepaged_pages_collapsed; + } +end: + return result; +} + static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, struct collapse_control *cc) __releases(&khugepaged_mm_lock) @@ -2439,33 +2485,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, VM_BUG_ON(khugepaged_scan.address < hstart || khugepaged_scan.address + HPAGE_PMD_SIZE > hend); - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { - struct file *file = get_file(vma->vm_file); - pgoff_t pgoff = linear_page_index(vma, - khugepaged_scan.address); - mmap_read_unlock(mm); - mmap_locked = false; - *result = hpage_collapse_scan_file(mm, - khugepaged_scan.address, file, pgoff, cc); - fput(file); - if (*result == SCAN_PTE_MAPPED_HUGEPAGE) { - mmap_read_lock(mm); - if (hpage_collapse_test_exit_or_disable(mm)) - goto breakouterloop; - *result = collapse_pte_mapped_thp(mm, - khugepaged_scan.address, false); - if (*result == SCAN_PMD_MAPPED) - *result = SCAN_SUCCEED; - mmap_read_unlock(mm); - } - } else { - *result = hpage_collapse_scan_pmd(mm, vma, - khugepaged_scan.address, &mmap_locked, cc); - } - - if (*result == SCAN_SUCCEED) - ++khugepaged_pages_collapsed; + *result = khugepaged_collapse_single_pmd(khugepaged_scan.address, + mm, vma, &mmap_locked, cc); /* move to next address */ khugepaged_scan.address += HPAGE_PMD_SIZE; @@ -2785,36 +2807,18 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev, mmap_assert_locked(mm); memset(cc->node_load, 0, sizeof(cc->node_load)); nodes_clear(cc->alloc_nmask); - if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) { - struct file *file = get_file(vma->vm_file); - pgoff_t pgoff = linear_page_index(vma, addr); - mmap_read_unlock(mm); - mmap_locked = false; - result = hpage_collapse_scan_file(mm, addr, file, pgoff, - cc); - fput(file); - } else { - result = hpage_collapse_scan_pmd(mm, vma, addr, - &mmap_locked, cc); - } + result = khugepaged_collapse_single_pmd(addr, mm, vma, &mmap_locked, cc); + if (!mmap_locked) *prev = NULL; /* Tell caller we dropped mmap_lock */ -handle_result: switch (result) { case SCAN_SUCCEED: case SCAN_PMD_MAPPED: ++thps; break; case SCAN_PTE_MAPPED_HUGEPAGE: - BUG_ON(mmap_locked); - BUG_ON(*prev); - mmap_read_lock(mm); - result = collapse_pte_mapped_thp(mm, addr, true); - mmap_read_unlock(mm); - goto handle_result; - /* Whitelisted set of results where continuing OK */ case SCAN_PMD_NULL: case SCAN_PTE_NON_PRESENT: case SCAN_PTE_UFFD_WP:
The khugepaged daemon and madvise_collapse have two different implementations that do almost the same thing. Create khugepaged_collapse_single_pmd to increase code reuse and create an entry point for future khugepaged changes. Refactor madvise_collapse and khugepaged_scan_mm_slot to use the new khugepaged_collapse_single_pmd function. Signed-off-by: Nico Pache <npache@redhat.com> --- mm/khugepaged.c | 96 +++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 46 deletions(-)