Re: [PATCH v2 07/13] mm: update secretmem to use VMA flags on mmap_prepare

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Wed, Jan 28, 2026 at 04:08:36AM -0800, Chris Mason wrote:
> Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> > This patch updates secretmem to use the new vma_flags_t type which will
> > soon supersede vm_flags_t altogether.
> >
> > In order to make this change we also have to update mlock_future_ok(), we
> > replace the vm_flags_t parameter with a simple boolean is_vma_locked one,
> > which also simplifies the invocation here.
> >
> > This is laying the groundwork for eliminating the vm_flags_t in
> > vm_area_desc and more broadly throughout the kernel.
> >
>
> Hi everyone,
>
> Running mm-new through AI review flagged this one:
>
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -225,12 +225,12 @@ static inline unsigned long round_hint_to_min(unsigned long hint)
> >  	return hint;
> >  }
> >
> > -bool mlock_future_ok(const struct mm_struct *mm, vm_flags_t vm_flags,
> > -			unsigned long bytes)
> > +bool mlock_future_ok(const struct mm_struct *mm, bool is_vma_locked,
> > +		     unsigned long bytes)
> >  {
> >  	unsigned long locked_pages, limit_pages;
> >
> > -	if (!(vm_flags & VM_LOCKED) || capable(CAP_IPC_LOCK))
> > +	if (!is_vma_locked || capable(CAP_IPC_LOCK))
> >  		return true;
>
> [ ... ]
>
> > @@ -416,7 +416,7 @@
> >  		if (!can_do_mlock())
> >  			return -EPERM;
> >
> > -	if (!mlock_future_ok(mm, vm_flags, len))
> > +	if (!mlock_future_ok(mm, vm_flags & VM_LOCKED, len))
> >  		return -EAGAIN;
>
> This looks correct - do_mmap() now extracts the VM_LOCKED bit from
> vm_flags and passes that to mlock_future_ok().
>
> However, it appears check_brk_limits() was not updated when
> mlock_future_ok() changed its signature. The function at mm/mmap.c:111
> still passes the raw def_flags value:
>
>     return mlock_future_ok(current->mm, current->mm->def_flags, len)
>         ? 0 : -EAGAIN;

Ack, the C 'type system' strikes again :) will send a fix-patch.

>
> When def_flags has any bit set, this converts to true, incorrectly
> triggering mlock limit checks even when VM_LOCKED is not set. Should
> this be passing (current->mm->def_flags & VM_LOCKED) instead?
>
> [ ... ]
>
> Additionally, the test stub at tools/testing/vma/vma_internal.h:1627
> still has the old signature (mm, vm_flags_t vm_flags, bytes) while the
> production code now uses (mm, bool is_vma_locked, bytes). This could
> cause compilation issues or mask bugs in the test suite.

Ack, I can fix that later. The VMA test headers have been split and it's too
much merge pain to address at this point given the tests aren't impacted by this
yet. Is on todo!

>
>

Cheers, Lorenzo