Re: [PATCH v3 2/5] binder: Make shrinker rely solely on per-VMA lock
Alice Ryhl <[email protected]> Mon, 3 Aug 2026 09:48:19 +0000
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On Sun, Aug 02, 2026 at 02:54:56PM -0700, Suren Baghdasaryan wrote: > From: Dave Hansen <[email protected]> > > tl;dr: lock_vma_under_rcu() is already a trylock. No need to do both > it and mmap_read_trylock(). > > Long Version: > > == Background == > > Historically, binder used an mmap_read_trylock() in its shrinker code. > This ensures that reclaim is not blocked on an mmap_lock. Commit > 95bc2d4a9020 ("binder: use per-vma lock in page reclaiming") added > support for the per-VMA lock, but left mmap_read_trylock() as a > fallback. > > This was presumably because the per-VMA locking can fail for several > reasons and most (all?) lock_vma_under_rcu() callers have a fallback > to mmap_read_trylock(). > > == Problem == > > The fallback is not worth the complexity here. lock_vma_under_rcu() is > essentially already a non-blocking trylock. The main reason it fails > is also the reason mmap_read_trylock() fails: something is holding > mmap_write_lock(). > > The only remedy for a collision with mmap_write_lock() is to wait, > which this code can not do. So the "fallback" after > lock_vma_under_rcu() failure is not really a fallback: it is really > likely to just be retrying in vain. That retry in an of itself isn't > horrible. But it adds complexity. > > == Solution == > > Now that per-VMA locks are universally available, lock_vma_under_rcu() > will not persistently fail. Rely on it alone and simplify the code. > > Full disclosure: I originally tried to do this with > lock_vma_under_rcu_wait(), but it did not fit well with the mmap_lock > trylock semantics. Claude caught this in a review and suggested the > approach in this path. It seemed sane to me. So, Suggesed-by: Claude, > I guess. > > Signed-off-by: Dave Hansen <[email protected]> > Signed-off-by: Suren Baghdasaryan <[email protected]> Reviewed-by: Alice Ryhl <[email protected]>