Replace goto skip with actual logic for better code readability. No functional change. Reviewed-by: Liam R. Howlett Reviewed-by: Zi Yan Acked-by: David Hildenbrand (Red Hat) Reviewed-by: Lance Yang Signed-off-by: Shivank Garg --- mm/khugepaged.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 16582bdcb6ff..984294a16861 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, break; } if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) { -skip: progress++; continue; } hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE); hend = round_down(vma->vm_end, HPAGE_PMD_SIZE); - if (khugepaged_scan.address > hend) - goto skip; + if (khugepaged_scan.address > hend) { + progress++; + continue; + } if (khugepaged_scan.address < hstart) khugepaged_scan.address = hstart; VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK); -- 2.43.0 The khugepaged_scan_mm_slot() uses a 'progress' counter to limit the amount of work performed and consists of three components: 1. Transitioning to a new mm (+1). 2. Skipping an unsuitable VMA (+1). 3. Scanning a PMD-sized range (+HPAGE_PMD_NR). Consider a 1MB VMA sitting between two 2MB alignment boundaries: vma1 vma2 vma3 +----------+------+----------+ |2M |1M |2M | +----------+------+----------+ ^ ^ start end ^ hstart,hend In this case, for vma2: hstart = round_up(start, HPAGE_PMD_SIZE) -> Next 2MB alignment hend = round_down(end, HPAGE_PMD_SIZE) -> Prev 2MB alignment Currently, since `hend <= hstart`, VMAs that are too small or unaligned to contain a hugepage are skipped without incrementing 'progress'. A process containing a large number of such small VMAs will unfairly consume more CPU cycles before yielding compared to a process with fewer, larger, or aligned VMAs. Fix this by incrementing progress when the `hend <= hstart` condition is met. Additionally, change 'progress' type to `unsigned int` to match both the 'pages' type and the function return value. Suggested-by: Wei Yang Reviewed-by: Wei Yang Reviewed-by: Lance Yang Signed-off-by: Shivank Garg --- Incorporate comment feedback from Lance: https://lore.kernel.org/linux-mm/6b408736-978a-4d40-adfc-97819951c3a6@linux.dev mm/khugepaged.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 984294a16861..93ce39915f4a 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -2403,7 +2403,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, struct mm_slot *slot; struct mm_struct *mm; struct vm_area_struct *vma; - int progress = 0; + unsigned int progress = 0; VM_BUG_ON(!pages); lockdep_assert_held(&khugepaged_mm_lock); @@ -2447,7 +2447,8 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, } hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE); hend = round_down(vma->vm_end, HPAGE_PMD_SIZE); - if (khugepaged_scan.address > hend) { + if (khugepaged_scan.address > hend || hend <= hstart) { + /* VMA already scanned or too small/unaligned for hugepage. */ progress++; continue; } -- 2.43.0 The only external caller of collapse_pte_mapped_thp() is uprobe, which ignores the return value. Change the external API to return void to simplify the interface. Introduce try_collapse_pte_mapped_thp() for internal use that preserves the return value. This prepares for future patch that will convert the return type to use enum scan_result. Suggested-by: David Hildenbrand (Red Hat) Acked-by: Lance Yang Acked-by: David Hildenbrand (Red Hat) Reviewed-by: Zi Yan Signed-off-by: Shivank Garg --- include/linux/khugepaged.h | 9 ++++----- mm/khugepaged.c | 40 ++++++++++++++++++++++---------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/include/linux/khugepaged.h b/include/linux/khugepaged.h index eb1946a70cff..d7a9053ff4fe 100644 --- a/include/linux/khugepaged.h +++ b/include/linux/khugepaged.h @@ -17,8 +17,8 @@ extern void khugepaged_enter_vma(struct vm_area_struct *vma, vm_flags_t vm_flags); extern void khugepaged_min_free_kbytes_update(void); extern bool current_is_khugepaged(void); -extern int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, - bool install_pmd); +void collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, + bool install_pmd); static inline void khugepaged_fork(struct mm_struct *mm, struct mm_struct *oldmm) { @@ -42,10 +42,9 @@ static inline void khugepaged_enter_vma(struct vm_area_struct *vma, vm_flags_t vm_flags) { } -static inline int collapse_pte_mapped_thp(struct mm_struct *mm, - unsigned long addr, bool install_pmd) +static inline void collapse_pte_mapped_thp(struct mm_struct *mm, + unsigned long addr, bool install_pmd) { - return 0; } static inline void khugepaged_min_free_kbytes_update(void) diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 93ce39915f4a..17f3f0043368 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -1477,20 +1477,8 @@ static int set_huge_pmd(struct vm_area_struct *vma, unsigned long addr, return SCAN_SUCCEED; } -/** - * collapse_pte_mapped_thp - Try to collapse a pte-mapped THP for mm at - * address haddr. - * - * @mm: process address space where collapse happens - * @addr: THP collapse address - * @install_pmd: If a huge PMD should be installed - * - * This function checks whether all the PTEs in the PMD are pointing to the - * right THP. If so, retract the page table so the THP can refault in with - * as pmd-mapped. Possibly install a huge PMD mapping the THP. - */ -int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, - bool install_pmd) +static int try_collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, + bool install_pmd) { int nr_mapped_ptes = 0, result = SCAN_FAIL; unsigned int nr_batch_ptes; @@ -1711,6 +1699,24 @@ int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, return result; } +/** + * collapse_pte_mapped_thp - Try to collapse a pte-mapped THP for mm at + * address haddr. + * + * @mm: process address space where collapse happens + * @addr: THP collapse address + * @install_pmd: If a huge PMD should be installed + * + * This function checks whether all the PTEs in the PMD are pointing to the + * right THP. If so, retract the page table so the THP can refault in with + * as pmd-mapped. Possibly install a huge PMD mapping the THP. + */ +void collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, + bool install_pmd) +{ + try_collapse_pte_mapped_thp(mm, addr, install_pmd); +} + /* Can we retract page tables for this file-backed VMA? */ static bool file_backed_vma_is_retractable(struct vm_area_struct *vma) { @@ -2227,7 +2233,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, /* * Remove pte page tables, so we can re-fault the page as huge. - * If MADV_COLLAPSE, adjust result to call collapse_pte_mapped_thp(). + * If MADV_COLLAPSE, adjust result to call try_collapse_pte_mapped_thp(). */ retract_page_tables(mapping, start); if (cc && !cc->is_khugepaged) @@ -2480,7 +2486,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, mmap_read_lock(mm); if (hpage_collapse_test_exit_or_disable(mm)) goto breakouterloop; - *result = collapse_pte_mapped_thp(mm, + *result = try_collapse_pte_mapped_thp(mm, khugepaged_scan.address, false); if (*result == SCAN_PMD_MAPPED) *result = SCAN_SUCCEED; @@ -2845,7 +2851,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start, case SCAN_PTE_MAPPED_HUGEPAGE: BUG_ON(mmap_locked); mmap_read_lock(mm); - result = collapse_pte_mapped_thp(mm, addr, true); + result = try_collapse_pte_mapped_thp(mm, addr, true); mmap_read_unlock(mm); goto handle_result; /* Whitelisted set of results where continuing OK */ -- 2.43.0 Convert result variables and return types from int to enum scan_result throughout khugepaged code. This improves type safety and code clarity by making the intent explicit. No functional change. Reviewed-by: Zi Yan Signed-off-by: Shivank Garg --- mm/khugepaged.c | 99 +++++++++++++++++++++++-------------------------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 17f3f0043368..1667abae6d8d 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -537,17 +537,16 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte, } } -static int __collapse_huge_page_isolate(struct vm_area_struct *vma, - unsigned long start_addr, - pte_t *pte, - struct collapse_control *cc, - struct list_head *compound_pagelist) +static enum scan_result __collapse_huge_page_isolate(struct vm_area_struct *vma, + unsigned long start_addr, pte_t *pte, struct collapse_control *cc, + struct list_head *compound_pagelist) { struct page *page = NULL; struct folio *folio = NULL; unsigned long addr = start_addr; pte_t *_pte; - int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0; + int none_or_zero = 0, shared = 0, referenced = 0; + enum scan_result result = SCAN_FAIL; for (_pte = pte; _pte < pte + HPAGE_PMD_NR; _pte++, addr += PAGE_SIZE) { @@ -780,13 +779,13 @@ static void __collapse_huge_page_copy_failed(pte_t *pte, * @ptl: lock on raw pages' PTEs * @compound_pagelist: list that stores compound pages */ -static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio, +static enum scan_result __collapse_huge_page_copy(pte_t *pte, struct folio *folio, pmd_t *pmd, pmd_t orig_pmd, struct vm_area_struct *vma, unsigned long address, spinlock_t *ptl, struct list_head *compound_pagelist) { unsigned int i; - int result = SCAN_SUCCEED; + enum scan_result result = SCAN_SUCCEED; /* * Copying pages' contents is subject to memory poison at any iteration. @@ -898,10 +897,8 @@ static int hpage_collapse_find_target_node(struct collapse_control *cc) * Returns enum scan_result value. */ -static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address, - bool expect_anon, - struct vm_area_struct **vmap, - struct collapse_control *cc) +static enum scan_result hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address, + bool expect_anon, struct vm_area_struct **vmap, struct collapse_control *cc) { struct vm_area_struct *vma; enum tva_type type = cc->is_khugepaged ? TVA_KHUGEPAGED : @@ -930,7 +927,7 @@ static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address, return SCAN_SUCCEED; } -static inline int check_pmd_state(pmd_t *pmd) +static inline enum scan_result check_pmd_state(pmd_t *pmd) { pmd_t pmde = pmdp_get_lockless(pmd); @@ -953,9 +950,8 @@ static inline int check_pmd_state(pmd_t *pmd) return SCAN_SUCCEED; } -static int find_pmd_or_thp_or_none(struct mm_struct *mm, - unsigned long address, - pmd_t **pmd) +static enum scan_result find_pmd_or_thp_or_none(struct mm_struct *mm, + unsigned long address, pmd_t **pmd) { *pmd = mm_find_pmd(mm, address); if (!*pmd) @@ -964,12 +960,11 @@ static int find_pmd_or_thp_or_none(struct mm_struct *mm, return check_pmd_state(*pmd); } -static int check_pmd_still_valid(struct mm_struct *mm, - unsigned long address, - pmd_t *pmd) +static enum scan_result check_pmd_still_valid(struct mm_struct *mm, + unsigned long address, pmd_t *pmd) { pmd_t *new_pmd; - int result = find_pmd_or_thp_or_none(mm, address, &new_pmd); + enum scan_result result = find_pmd_or_thp_or_none(mm, address, &new_pmd); if (result != SCAN_SUCCEED) return result; @@ -985,15 +980,14 @@ static int check_pmd_still_valid(struct mm_struct *mm, * Called and returns without pte mapped or spinlocks held. * Returns result: if not SCAN_SUCCEED, mmap_lock has been released. */ -static int __collapse_huge_page_swapin(struct mm_struct *mm, - struct vm_area_struct *vma, - unsigned long start_addr, pmd_t *pmd, - int referenced) +static enum scan_result __collapse_huge_page_swapin(struct mm_struct *mm, + struct vm_area_struct *vma, unsigned long start_addr, pmd_t *pmd, + int referenced) { int swapped_in = 0; vm_fault_t ret = 0; unsigned long addr, end = start_addr + (HPAGE_PMD_NR * PAGE_SIZE); - int result; + enum scan_result result; pte_t *pte = NULL; spinlock_t *ptl; @@ -1062,8 +1056,8 @@ static int __collapse_huge_page_swapin(struct mm_struct *mm, return result; } -static int alloc_charge_folio(struct folio **foliop, struct mm_struct *mm, - struct collapse_control *cc) +static enum scan_result alloc_charge_folio(struct folio **foliop, struct mm_struct *mm, + struct collapse_control *cc) { gfp_t gfp = (cc->is_khugepaged ? alloc_hugepage_khugepaged_gfpmask() : GFP_TRANSHUGE); @@ -1090,9 +1084,8 @@ static int alloc_charge_folio(struct folio **foliop, struct mm_struct *mm, return SCAN_SUCCEED; } -static int collapse_huge_page(struct mm_struct *mm, unsigned long address, - int referenced, int unmapped, - struct collapse_control *cc) +static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long address, + int referenced, int unmapped, struct collapse_control *cc) { LIST_HEAD(compound_pagelist); pmd_t *pmd, _pmd; @@ -1100,7 +1093,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address, pgtable_t pgtable; struct folio *folio; spinlock_t *pmd_ptl, *pte_ptl; - int result = SCAN_FAIL; + enum scan_result result = SCAN_FAIL; struct vm_area_struct *vma; struct mmu_notifier_range range; @@ -1246,15 +1239,14 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address, return result; } -static int hpage_collapse_scan_pmd(struct mm_struct *mm, - struct vm_area_struct *vma, - unsigned long start_addr, bool *mmap_locked, - struct collapse_control *cc) +static enum scan_result hpage_collapse_scan_pmd(struct mm_struct *mm, + struct vm_area_struct *vma, unsigned long start_addr, bool *mmap_locked, + struct collapse_control *cc) { pmd_t *pmd; pte_t *pte, *_pte; - int result = SCAN_FAIL, referenced = 0; - int none_or_zero = 0, shared = 0; + int none_or_zero = 0, shared = 0, referenced = 0; + enum scan_result result = SCAN_FAIL; struct page *page = NULL; struct folio *folio = NULL; unsigned long addr; @@ -1441,8 +1433,8 @@ static void collect_mm_slot(struct mm_slot *slot) } /* folio must be locked, and mmap_lock must be held */ -static int set_huge_pmd(struct vm_area_struct *vma, unsigned long addr, - pmd_t *pmdp, struct folio *folio, struct page *page) +static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long addr, + pmd_t *pmdp, struct folio *folio, struct page *page) { struct mm_struct *mm = vma->vm_mm; struct vm_fault vmf = { @@ -1477,10 +1469,11 @@ static int set_huge_pmd(struct vm_area_struct *vma, unsigned long addr, return SCAN_SUCCEED; } -static int try_collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, +static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr, bool install_pmd) { - int nr_mapped_ptes = 0, result = SCAN_FAIL; + enum scan_result result = SCAN_FAIL; + int nr_mapped_ptes = 0; unsigned int nr_batch_ptes; struct mmu_notifier_range range; bool notified = false; @@ -1862,9 +1855,8 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff) * + unlock old pages * + unlock and free huge page; */ -static int collapse_file(struct mm_struct *mm, unsigned long addr, - struct file *file, pgoff_t start, - struct collapse_control *cc) +static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr, + struct file *file, pgoff_t start, struct collapse_control *cc) { struct address_space *mapping = file->f_mapping; struct page *dst; @@ -1872,7 +1864,8 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, pgoff_t index = 0, end = start + HPAGE_PMD_NR; LIST_HEAD(pagelist); XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER); - int nr_none = 0, result = SCAN_SUCCEED; + enum scan_result result = SCAN_SUCCEED; + int nr_none = 0; bool is_shmem = shmem_file(file); VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem); @@ -2293,16 +2286,15 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, return result; } -static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, - struct file *file, pgoff_t start, - struct collapse_control *cc) +static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, + struct file *file, pgoff_t start, struct collapse_control *cc) { struct folio *folio = NULL; struct address_space *mapping = file->f_mapping; XA_STATE(xas, &mapping->i_pages, start); int present, swap; int node = NUMA_NO_NODE; - int result = SCAN_SUCCEED; + enum scan_result result = SCAN_SUCCEED; present = 0; swap = 0; @@ -2400,7 +2392,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr, return result; } -static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result, +static unsigned int khugepaged_scan_mm_slot(unsigned int pages, enum scan_result *result, struct collapse_control *cc) __releases(&khugepaged_mm_lock) __acquires(&khugepaged_mm_lock) @@ -2562,7 +2554,7 @@ static void khugepaged_do_scan(struct collapse_control *cc) unsigned int progress = 0, pass_through_head = 0; unsigned int pages = READ_ONCE(khugepaged_pages_to_scan); bool wait = true; - int result = SCAN_SUCCEED; + enum scan_result result = SCAN_SUCCEED; lru_add_drain_all(); @@ -2775,7 +2767,8 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start, struct collapse_control *cc; struct mm_struct *mm = vma->vm_mm; unsigned long hstart, hend, addr; - int thps = 0, last_fail = SCAN_FAIL; + enum scan_result last_fail = SCAN_FAIL; + int thps = 0; bool mmap_locked = true; BUG_ON(vma->vm_start > start); @@ -2796,7 +2789,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start, hend = end & HPAGE_PMD_MASK; for (addr = hstart; addr < hend; addr += HPAGE_PMD_SIZE) { - int result = SCAN_FAIL; + enum scan_result result = SCAN_FAIL; bool triggered_wb = false; retry: -- 2.43.0 The global variable 'khugepaged_collapse_control' is not used outside of mm/khugepaged.c. Make it static to limit its scope. Reviewed-by: Wei Yang Reviewed-by: Zi Yan Acked-by: David Hildenbrand (Red Hat) Reviewed-by: Anshuman Khandual Signed-off-by: Shivank Garg --- mm/khugepaged.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 1667abae6d8d..fba6aea5bea6 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void) remove_wait_queue(&khugepaged_wait, &wait); } -struct collapse_control khugepaged_collapse_control = { +static struct collapse_control khugepaged_collapse_control = { .is_khugepaged = true, }; -- 2.43.0