Re: [PATCH v3 24/40] mm/mlock: eliminate weird VMA_IO_BIT abuse and simplify

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Wed, Sep 23, 2026 at 04:06:14PM -0400, Zi Yan wrote:
> On 17 Sep 2026, at 12:22, Lorenzo Stoakes (ARM) wrote:
>
> > When performing mlock() or munlock() otherwise normal VMAs have VMA_IO_BIT
> > solely to fix a race with migration which might otherwise double-count
> > mlock VMAs.
> >
> > This is unnecessary - at the point of applying folio mlock state, whether
> > setting or clearing PG_mlocked, we know whether or not we are locking.
> >
> > Solve this in two ways - thread a boolean through the page table walk
> > indicating whether a lock or unlock is being performed, and run a locking
> > walk with VMA_LOCKONFAULT_BIT set and VMA_LOCKED_BIT cleared.
> >
> > This state never occurs otherwise, as VMA_LOCKONFAULT_BIT always implies
> > VMA_LOCKED_BIT. These are also always cleared together.
> >
> > Then, update folio_add_lru_vma() and mlock_folio() to check only for
> > VMA_LOCKED_BIT, and update try_to_unmap_one() to check for VMA_LOCKED_MASK
> > instead.
> >
> > Also remove the useless invocation of allow_mlock_munlock() which simply
> > returns true if unlocking and instead rename it to allow_mlock() and only
> > call it when locking.
> >
> > Finally, with the other mlock abuse of VMA_IO_BIT addressed, update
> > mlock_vma_folio() and folio_add_lru_vma() to simply test for
> > VMA_LOCKED_BIT. munlock_vma_folio() tests VMA_LOCKED_MASK instead, as an
> > unmap racing with the locking walk must still munlock folios the walk has
> > already counted.
> >
> > While here, also replace some deprecated VMA flag predicates.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  mm/folio.c    |  2 +-
> >  mm/internal.h | 10 +++++++---
> >  mm/mlock.c    | 51 +++++++++++++++++++--------------------------------
> >  mm/rmap.c     |  4 +++-
> >  4 files changed, 30 insertions(+), 37 deletions(-)
> >
> > diff --git a/mm/folio.c b/mm/folio.c
> > index 47a437e0f7fd..35e242b48870 100644
> > --- a/mm/folio.c
> > +++ b/mm/folio.c
> > @@ -505,7 +505,7 @@ void folio_add_lru_vma(struct folio *folio, struct vm_area_struct *vma)
> >  {
> >  	VM_BUG_ON_FOLIO(folio_test_lru(folio), folio);
> >
> > -	if (unlikely((vma->vm_flags & (VM_LOCKED | VM_SPECIAL)) == VM_LOCKED))
> > +	if (vma_test(vma, VMA_LOCKED_BIT))
>
> I think it is worth documenting VMA_LOCKONFAULT_BIT alone means mlock in
> progress, like you did in munlock_vma_folio(). Just to keep the protocol
> explicit for all the readers.

Well I'm not sure it's necessary here honestly, because this never checked
VMA_LOCKED_MASK anyway, and VMA_LOCKONFAULT_BIT never made a difference.

So the meaning of VMA_LOCKED_BIT here is strictly 'is it locked' and it's
correctly handled.

And I fear that it becomes whack-a-mole - the neat thing about this change is
that you no longer have to special case the stupid VM_SPECIAL thing, and can in
fact do the 'normal' thing of _just checking_ VMA_LOCKED_BIT :)

So I think it's better not to.

>
> >  		mlock_new_folio(folio);
> >  	else
> >  		folio_add_lru(folio);
> > diff --git a/mm/internal.h b/mm/internal.h
> > index b2c6c9435021..84aa3e6c8bac 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -971,8 +971,7 @@ void mlock_folio(struct folio *folio);
> >  static inline void mlock_vma_folio(struct folio *folio,
> >  				struct vm_area_struct *vma)
> >  {
> > -	/* The VM_IO check prevents migration from double-counting during mlock. */
> > -	if (unlikely((vma->vm_flags & (VM_LOCKED|VM_SPECIAL)) == VM_LOCKED))
> > +	if (vma_test(vma, VMA_LOCKED_BIT))
>
> Ditto.

Similar reasoning to above.

>
> >  		mlock_folio(folio);
> >  }
> >
> > @@ -989,7 +988,12 @@ static inline void munlock_vma_folio(struct folio *folio,
> >  	 * always munlock the folio and page reclaim will correct it
> >  	 * if it's wrong.
> >  	 */
> > -	if (unlikely(vma->vm_flags & VM_LOCKED))
> > +	/*
> > +	 * VMA_LOCKONFAULT_BIT alone marks an mlock walk in progress, see
> > +	 * mlock_vma_pages_range(). An unmap racing with the walk must still
> > +	 * munlock folios the walk has already counted.
> > +	 */

Here it's worth mentioning, as it's specifically relying on the new
behaviour. Although it's neatly using the VMA_LOCKED_MASK to handle both the
locked case and the 'being locked' case :)

> > +	if (unlikely(vma_test_any_mask(vma, VMA_LOCKED_MASK)))
> >  		munlock_folio(folio);
> >  }
> >
>
> Why I am commenting in the middle of the series? Because I am taking
> a quiz given by LLM based on this series to get myself enough background
> knowledge to review this series. This mlock part came up at part E
> and I only have part F left before I can do the full review. :)

Thanks! :) I really appreciate you taking the time to look at this! Sorry it's
so large.

I held this series back from last cycle to help with review load, then spent
some time fixing various AI-discovered things, and all the patches are necessary
(well for the most part) to get where the series needs to go.

I think the change is worth it though!

>
> Best Regards,
> Yan, Zi

--
Cheers, Lorenzo