Re: [PATCH v7 1/7] arm64/hugetlb: Extend batching of multiple CONT_PTE in a single PTE setup

On Wed, Jul 29, 2026 at 05:47:34AM +0800, Barry Song wrote:
> On Tue, Jul 28, 2026 at 8:03 PM Will Deacon <[email protected]> wrote:
> >
> > On Wed, Jul 15, 2026 at 08:08:07PM +0800, Wen Jiang wrote:
> > > From: "Barry Song (Xiaomi)" <[email protected]>
> > >
> > > For sizes aligned to CONT_PTE_SIZE and smaller than PMD_SIZE,
> > > we can handle CONT_PTE_SIZE groups together.
> > >
> > > These additional sizes are mapping spans used by non-hugetlbfs(vmalloc)
> > > mm code, not new HugeTLB hstate sizes.
> > >
> > > Signed-off-by: Barry Song (Xiaomi) <[email protected]>
> > > Signed-off-by: Wen Jiang <[email protected]>
> > > Tested-by: Xueyuan Chen <[email protected]>
> > > Tested-by: Leo Yan <[email protected]>
> > > Reviewed-by: Dev Jain <[email protected]>
> > > ---
> > >  arch/arm64/mm/hugetlbpage.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> > > index a42c05cf56408..7ce159483a354 100644
> > > --- a/arch/arm64/mm/hugetlbpage.c
> > > +++ b/arch/arm64/mm/hugetlbpage.c
> > > @@ -94,6 +94,11 @@ static int find_num_contig(struct mm_struct *mm, unsigned long addr,
> > >       return CONT_PTES;
> > >  }
> > >
> > > +/*
> > > + * num_contig_ptes(), set_huge_pte_at() and arch_make_huge_pte() can be
> > > + * used by non-hugetlbfs(vmalloc) mm code to set multiple huge mappings
> > > + * at the PTE level.
> > > + */
> > >  static inline int num_contig_ptes(unsigned long size, size_t *pgsize)
> > >  {
> > >       int contig_ptes = 1;
> > > @@ -110,6 +115,12 @@ static inline int num_contig_ptes(unsigned long size, size_t *pgsize)
> > >               contig_ptes = CONT_PTES;
> > >               break;
> > >       default:
> > > +             if (size > 0 && size < PMD_SIZE &&
> > > +                             IS_ALIGNED(size, CONT_PTE_SIZE)) {
> > > +                     *pgsize = PAGE_SIZE;
> > > +                     contig_ptes = size >> PAGE_SHIFT;
> > > +                     break;
> > > +             }
> >
> > Under which circumstances would you get a size of 0 here?
> >
> > Given that you're relying on arch_vmap_pte_range_map_size() to give you
> > a well-formed size, why isn't if sufficient to check only the alignment?
> 
> Thanks very much for your review.
> 
> Are you suggesting the change below? If so, I'm fine with it.
> I guess the current code is just being overly cautious for
> defensive programming.
> 
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 8429d220660b..c1af52ba572d 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -115,8 +115,7 @@ static inline int num_contig_ptes(unsigned long
> size, size_t *pgsize)
>                 contig_ptes = CONT_PTES;
>                 break;
>         default:
> -               if (size > 0 && size < PMD_SIZE &&
> -                               IS_ALIGNED(size, CONT_PTE_SIZE)) {
> +               if (IS_ALIGNED(size, CONT_PTE_SIZE)) {
>                         *pgsize = PAGE_SIZE;
>                         contig_ptes = size >> PAGE_SHIFT;
>                         break;
> 

Yeah, that's what I had in mind.

> > >               WARN_ON(!__hugetlb_valid_size(size));
> >
> > I agree with David that it's messy having hugetlb tangled up in here.
> > It means this validity check is now going to miss some genuinely bogus
> > cases for the hugetlb path (as opposed to the vmalloc path).
> 
> I agree that coupling hugetlb with vmalloc is not ideal. As I
> explained to David, this has been an issue for a couple of
> years and affects multiple architectures since 2021:
> 
> https://lore.kernel.org/all/fb3ccc73377832ac6708181ec419128a2f98ce36.1620795204.git.christophe.leroy@csgroup.eu/
> 
> where `#ifdef CONFIG_HUGETLB_PAGE` is required by `vmalloc`.
> 
> So I'd prefer to address it in a separate follow-up patch
> series.
> 
> For the `WARN_ON(!__hugetlb_valid_size(size))` check, I don't
> see anything broken here. Hugetlb only uses sizes registered
> by `hugetlbpage_init()`, which are validated by
> `arch_hugetlb_valid_size()` implemented in
> `arch/arm64/mm/hugetlbpage.c`:
> 
> static int __init hugetlbpage_init(void)
> {
>         BUILD_BUG_ON(HUGE_MAX_HSTATE < 4);
>         if (pud_sect_supported())
>                 hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> 
>         hugetlb_add_hstate(CONT_PMD_SHIFT - PAGE_SHIFT);
>         hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
>         hugetlb_add_hstate(CONT_PTE_SHIFT - PAGE_SHIFT);
> 
>         return 0;
> }
> arch_initcall(hugetlbpage_init);
> 
> bool __init arch_hugetlb_valid_size(unsigned long size)
> {
>         return __hugetlb_valid_size(size);
> }

I'm just pointing out that the defensive checking in __hugetlb_valid_size(),
which should really only warn if something has gone horribly wrong, will
now not detect bogus sizes if the address is aligned to CONT_PTE_SIZE.

Keeping hugetlb and vmalloc separate would avoid this problem.

Will