Re: [PATCH v1 14/16] mm/vmalloc: Batch arch_sync_kernel_mappings() more efficiently
From: Anshuman Khandual <hidden>
Date: 2025-02-10 07:11:27
Also in:
linux-mm, lkml
On 2/5/25 20:39, Ryan Roberts wrote:
When page_shift is greater than PAGE_SIZE, __vmap_pages_range_noflush() will call vmap_range_noflush() for each individual huge page. But vmap_range_noflush() would previously call arch_sync_kernel_mappings() directly so this would end up being called for every huge page. We can do better than this; refactor the call into the outer __vmap_pages_range_noflush() so that it is only called once for the entire batch operation.
This makes sense.
quoted hunk ↗ jump to hunk
This will benefit performance for arm64 which is about to opt-in to using the hook. Signed-off-by: Ryan Roberts <ryan.roberts@arm.com> --- mm/vmalloc.c | 60 ++++++++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 30 deletions(-)diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 68950b1824d0..50fd44439875 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c@@ -285,40 +285,38 @@ static int vmap_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end, static int vmap_range_noflush(unsigned long addr, unsigned long end, phys_addr_t phys_addr, pgprot_t prot, - unsigned int max_page_shift) + unsigned int max_page_shift, pgtbl_mod_mask *mask) { pgd_t *pgd; - unsigned long start; unsigned long next; int err; - pgtbl_mod_mask mask = 0; might_sleep(); BUG_ON(addr >= end); - start = addr; pgd = pgd_offset_k(addr); do { next = pgd_addr_end(addr, end); err = vmap_p4d_range(pgd, addr, next, phys_addr, prot, - max_page_shift, &mask); + max_page_shift, mask); if (err) break; } while (pgd++, phys_addr += (next - addr), addr = next, addr != end); - if (mask & ARCH_PAGE_TABLE_SYNC_MASK) - arch_sync_kernel_mappings(start, end); - return err; }
arch_sync_kernel_mappings() gets dropped here and moved to existing vmap_range_noflush() callers instead.
int vmap_page_range(unsigned long addr, unsigned long end,
phys_addr_t phys_addr, pgprot_t prot)
{
+ pgtbl_mod_mask mask = 0;
int err;
err = vmap_range_noflush(addr, end, phys_addr, pgprot_nx(prot),
- ioremap_max_page_shift);
+ ioremap_max_page_shift, &mask);
+ if (mask & ARCH_PAGE_TABLE_SYNC_MASK)
+ arch_sync_kernel_mappings(addr, end);
+arch_sync_kernel_mappings() gets moved here.
quoted hunk ↗ jump to hunk
flush_cache_vmap(addr, end); if (!err) err = kmsan_ioremap_page_range(addr, end, phys_addr, prot,@@ -587,29 +585,24 @@ static int vmap_pages_p4d_range(pgd_t *pgd, unsigned long addr, } static int vmap_small_pages_range_noflush(unsigned long addr, unsigned long end, - pgprot_t prot, struct page **pages) + pgprot_t prot, struct page **pages, pgtbl_mod_mask *mask) { - unsigned long start = addr; pgd_t *pgd; unsigned long next; int err = 0; int nr = 0; - pgtbl_mod_mask mask = 0; BUG_ON(addr >= end); pgd = pgd_offset_k(addr); do { next = pgd_addr_end(addr, end); if (pgd_bad(*pgd)) - mask |= PGTBL_PGD_MODIFIED; - err = vmap_pages_p4d_range(pgd, addr, next, prot, pages, &nr, &mask); + *mask |= PGTBL_PGD_MODIFIED; + err = vmap_pages_p4d_range(pgd, addr, next, prot, pages, &nr, mask); if (err) break; } while (pgd++, addr = next, addr != end); - if (mask & ARCH_PAGE_TABLE_SYNC_MASK) - arch_sync_kernel_mappings(start, end); - return err; }@@ -626,26 +619,33 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end, pgprot_t prot, struct page **pages, unsigned int page_shift) { unsigned int i, nr = (end - addr) >> PAGE_SHIFT; + unsigned long start = addr; + pgtbl_mod_mask mask = 0; + int err = 0; WARN_ON(page_shift < PAGE_SHIFT); if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC) || - page_shift == PAGE_SHIFT) - return vmap_small_pages_range_noflush(addr, end, prot, pages); - - for (i = 0; i < nr; i += 1U << (page_shift - PAGE_SHIFT)) { - int err; - - err = vmap_range_noflush(addr, addr + (1UL << page_shift), - page_to_phys(pages[i]), prot, - page_shift); - if (err) - return err; + page_shift == PAGE_SHIFT) { + err = vmap_small_pages_range_noflush(addr, end, prot, pages, + &mask);
Unlike earlier don't return here until arch_sync_kernel_mappings() gets covered later.
+ } else {
+ for (i = 0; i < nr; i += 1U << (page_shift - PAGE_SHIFT)) {
+ err = vmap_range_noflush(addr,
+ addr + (1UL << page_shift),
+ page_to_phys(pages[i]), prot,
+ page_shift, &mask);
+ if (err)
+ break;
- addr += 1UL << page_shift;
+ addr += 1UL << page_shift;
+ }
}
- return 0;
+ if (mask & ARCH_PAGE_TABLE_SYNC_MASK)
+ arch_sync_kernel_mappings(start, end);arch_sync_kernel_mappings() gets moved here after getting dropped from both vmap_range_noflush() and vmap_small_pages_range_noflush().
+ + return err; } int vmap_pages_range_noflush(unsigned long addr, unsigned long end,
LGTM, and this can stand on its own as well. Reviewed-by: Anshuman Khandual <redacted>