Re: [PATCH v3 24/40] mm/mlock: eliminate weird VMA_IO_BIT abuse and simplify
From: Zi Yan <ziy@nvidia.com>
Date: 2026-09-24 15:50:34
Also in:
bpf, dri-devel, fuse-devel, kvm, kvm-riscv, kvmarm, linux-arch, linux-doc, linux-fsdevel, linux-mm, linux-perf-users, linux-rdma, linux-riscv, linux-s390, linux-scsi, linux-sound, linux-trace-kernel, linux-usb, linuxppc-dev, lkml, selinux, sparclinux
On 24 Sep 2026, at 6:21, Lorenzo Stoakes (ARM) wrote:
On Wed, Sep 23, 2026 at 04:06:14PM -0400, Zi Yan wrote:quoted
On 17 Sep 2026, at 12:22, Lorenzo Stoakes (ARM) wrote:quoted
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.
Your reasoning makes sense to me.
quoted
quoted
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.quoted
quoted
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 :)quoted
quoted
+ 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.
Sure. It is great learning material for me. Thank you for the patches.
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!
Of course, great to see hacky code being removed by this series. For this patch, feel free to add Reviewed-by: Zi Yan <ziy@nvidia.com> Best Regards, Yan, Zi