Re: [PATCH RESEND v3 10/10] mm/vma: add and use vma_assert_stabilised()

[Vlastimil Babka <vbabka@suse.cz>]

On 1/22/26 14:02, Lorenzo Stoakes wrote:
> Sometimes we wish to assert that a VMA is stable, that is - the VMA cannot
> be changed underneath us. This will be the case if EITHER the VMA lock or
> the mmap lock is held.
> 
> In order to do so, we introduce a new assert vma_assert_stablised() - this
> will make a lockdep assert if lockdep is enabled AND the VMA is
> read-locked.
> 
> Currently lockdep tracking for VMA write locks is not implemented, so it
> suffices to check in this case that we have either an mmap read or write
> semaphore held.
> 
> Note that because the VMA lock uses the non-standard vmlock_dep_map naming
> convention, we cannot use lockdep_assert_is_write_held() so have to open
> code this ourselves via lockdep-asserting that
> lock_is_held_type(&vma->vmlock_dep_map, 0).
> 
> We have to be careful here - for instance when merging a VMA, we use the
> mmap write lock to stabilise the examination of adjacent VMAs which might
> be simultaneously VMA read-locked whilst being faulted in.
> 
> If we were to assert VMA read lock using lockdep we would encounter an
> incorrect lockdep assert.
> 
> Also, we have to be careful about asserting mmap locks are held - if we try
> to address the above issue by first checking whether mmap lock is held and
> if so asserting it via lockdep, we may find that we were raced by another
> thread acquiring an mmap read lock simultaneously that either we don't
> own (and thus can be released any time - so we are not stable) or was
> indeed released since we last checked.
> 
> So to deal with these complexities we end up with either a precise (if
> lockdep is enabled) or imprecise (if not) approach - in the first instance
> we assert the lock is held using lockdep and thus whether we own it.
> 
> If we do own it, then the check is complete, otherwise we must check for
> the VMA read lock being held (VMA write lock implies mmap write lock so the
> mmap lock suffices for this).
> 
> If lockdep is not enabled we simply check if the mmap lock is held and risk
> a false negative (i.e. not asserting when we should do).
> 
> There are a couple places in the kernel where we already do this
> stabliisation check - the anon_vma_name() helper in mm/madvise.c and
> vma_flag_set_atomic() in include/linux/mm.h, which we update to use
> vma_assert_stabilised().
> 
> This change abstracts these into vma_assert_stabilised(), uses lockdep if
> possible, and avoids a duplicate check of whether the mmap lock is held.
> 
> This is also self-documenting and lays the foundations for further VMA
> stability checks in the code.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

LGTM, thanks!

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>