Re: [PATCH] mm: shmem: fix incorrect vm_flags usage when checking allowable orders

[Baolin Wang]

On 8/17/26 5:34 PM, Baolin Wang wrote:
> On 8/17/26 3:40 PM, Lorenzo Stoakes (ARM) wrote:
> > On Mon, Aug 17, 2026 at 03:16:43PM +0800, Baolin Wang wrote:
> > > Lance reported that when MADV_HUGEPAGE is set on a tmpfs file mounted 
> > > with
> > > huge=advise option, khugepaged fails the allowable order check and 
> > > does not
> > > scan the tmpfs file for collapse.
> >
> > Ugh.
> >
> > But really I think this isn't quite accurate - what you mean to say I 
> > think is
> > that when _nothing else_ causes the mm to be considered for khugepaged 
> > collapse,
> > an MADV_HUGEPAGE-advised tmpfs VMA alone does not cause scanning to 
> > commence.
>
> Yes.
>
> > > After commit 6beeab870e70 ("mm: shmem: move 
> > > shmem_huge_global_enabled() into
> > > shmem_allowable_huge_orders()"), the shmem/tmpfs allowable order 
> > > check reads
> > > vma->vm_flags directly.  However, when MADV_HUGEPAGE is handled,
> > > khugepaged_enter_vma() is called before the VMA's vm_flags have been 
> > > updated,
> > > so the check uses stale flags and incorrectly rejects the VMA for 
> > > collapse.
> > > As a result, khugepaged does not collapse the tmpfs file into PMD 
> > > order in time.
> >
> > Could we at least refer to the non-deprecated field in the commit 
> > message?
> > i.e. vma->flags.
> >
> > Probably worth mentioning VMA_HUGEPAGE_BIT also.
>
> Sure.
>
>
> > > Fix this by passing vm_flags as a parameter to 
> > > shmem_allowable_huge_orders()
> > > instead of reading it from the vm_area_struct.
> >
> > Ugh this is so disgusting.
> >
> > I understand this is a fix for a bug to be backported but couldn't we 
> > just
> > achieve the same without having to add a deprecated field to be passed 
> > around?
> >
> > As you say the khugepaged_enter_vma() isn't really so helpful in
> > hugepage_madvise().
> >
> > But you could add this to the bottom of madvise_update_vma():
> >
> >     if (vma_flags_test(&new_vma_flags, VMA_HUGEPAGE_BIT))
> >         khugepaged_enter_vma(vma, new_flags);
> >
> > I don't think this is really egregious on this code path and could go 
> > some way
> > towards us eliminating the silly thing of passing around flags-to-be-set.
>
> This is not the point (maybe I didn't describe it clearly). The point is 
> that the allowable orders check for tmpfs in shmem_huge_global_enabled() 
> (called by shmem_allowable_huge_orders()) uses a stale vma flag.
>
> static unsigned int shmem_huge_global_enabled(struct inode *inode, 
> pgoff_t index,
>              loff_t write_end, bool shmem_huge_force,
>              struct vm_area_struct *vma,
>              vm_flags_t vm_flags)
> {
>      ......
>      switch (SHMEM_SB(inode->i_sb)->huge) {
>
>      ......
>      case SHMEM_HUGE_ADVISE:
>          if (vm_flags & VM_HUGEPAGE)
>              return THP_ORDERS_ALL_FILE_DEFAULT;
>          fallthrough;
>      default:
>          return 0;
>      }
> }
>
> So we should pass the new vma flags for shmem_allowable_huge_orders() to 
> check the allowable orders for tmpfs. Changing madvise_update_vma() 
> doesn't help with the allowable orders check for tmpfs.

Sorry for misreading your code (I need a coffee before reading the email 
:)). Please ignore my reply. After looking at the code again, yes, this 
can work. If nobody rejects, I will follow your suggestion in v2. Thanks.