Re: [PATCH v4 1/5] mm: Make per-VMA locks available universally

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

On Sat, Aug 08, 2026 at 02:12:50AM +0100, Matthew Wilcox wrote:
> On Thu, Aug 06, 2026 at 01:05:44PM -0700, Suren Baghdasaryan wrote:
> > +++ b/kernel/bpf/stackmap.c
> > @@ -272,13 +272,8 @@ struct stack_map_vma_lock {
> >  /*
> >   * Acquire a stable read-side reference on the VMA covering @ip.
> >   *
> > - * With CONFIG_PER_VMA_LOCK=y this returns a VMA with its per-VMA read
> > - * lock held and mmap_lock dropped, so the caller may sleep.
> > - *
> > - * With CONFIG_PER_VMA_LOCK=n it returns a VMA with mmap_lock still
> > - * held; the caller must snapshot any fields it needs and pin vm_file
> > - * with get_file() before stack_map_unlock_vma() drops mmap_lock, as
> > - * the VMA may be split, merged, or freed after that.
> > + * This returns a VMA with its per-VMA read lock held and mmap_lock
> > + * dropped, so the caller may sleep.
>
> I don't know if BPF is compatible with !MMU or not, but the comment
> is inconsistent with the code.  How about:

<requisite nommu rant>

I do think there are components that simply don't think to depend on CONFIG_MMU
even though they do.

In fact more than think - have run into exactly that before.

It's another thing that speaks to nommu being a legacy barnacle that bashes us
on the head fairly regularly for little to no gain (and nobody is testing it for
tip kernel AFAICT).

</requisite nommu rant>

>
>  * On NOMMU configurations, returns with the mmap_lock held.  If the MMU
>  * is enabled, the per-VMA lock will be held instead.  The lock
>  * should be released with stack_map_unlock_vma() which will release the
>  * appropriate lock.  Once the lock is released, the VMA may be freed.

I mean I suppose it's accurate but I don't love the idea of essentially implying
nommu+bpf is a thing and also treating it as so important that it must be called
out here.

I'd rather it be inaccurate for nommu as are most comments in mm and mm-adjacent
components, it's kinda implied in general. Those who care can look at the code.

>
> >   * Returns NULL on failure, in which case no lock is held.
> >   */
> > @@ -288,7 +283,6 @@ stack_map_lock_vma(struct stack_map_vma_lock *lock, unsigned long ip)
> >  	struct mm_struct *mm = lock->mm;
> >  	struct vm_area_struct *vma;
> >
> > -	/* noop under !CONFIG_PER_VMA_LOCK */
> >  	vma = lock_vma_under_rcu(mm, ip);
> >  	if (vma) {
> >  		lock->vma = vma;
> > @@ -308,21 +302,20 @@ stack_map_lock_vma(struct stack_map_vma_lock *lock, unsigned long ip)
> >  		return NULL;
> >  	}
> >
> > -#ifdef CONFIG_PER_VMA_LOCK
> > +#ifdef CONFIG_MMU
> >  	if (!vma_start_read_locked(vma)) {
> >  		mmap_read_unlock(mm);
> >  		return NULL;
> >  	}
> >  	mmap_read_unlock(mm);
> >  #endif
> > -
> >  	lock->vma = vma;
> >  	return vma;
> >  }

--
Cheers, Lorenzo