Re: [PATCH v4 1/5] mm: Make per-VMA locks available universally
Suren Baghdasaryan <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <CAJuCfpHngcCttUaUtAJVxF5J4-WGGscx+mEu7LPqd4K61kLOdw@mail.gmail.com> |
On Mon, Aug 10, 2026 at 2:43 AM Lorenzo Stoakes (ARM) <[email protected]> wrote: > > On Mon, Aug 10, 2026 at 09:52:11AM +0100, Lorenzo Stoakes (ARM) wrote: > > 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. > > Actually scratch that I'm wrong, firstly Suren's series explicitly does some > nommu-specific logic in the bpf code and secondly BPF already has nommu-specific > stuff in it. > > It seems nommu bpf is (kinda) a supported path. Ok, I'll change it the way Matthew suggested then. > > > > > > > > > > * Returns NULL on failure, in which case no lock is held. > > > > */ > > -- > Cheers, Lorenzo