Re: [PATCH v3 2/5] binder: Make shrinker rely solely on per-VMA lock
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
Date: 2026-08-03 11:34:18
Also in:
linux-mm, lkml
On Mon, Aug 03, 2026 at 12:11:42PM +0100, Lorenzo Stoakes (ARM) wrote:
On Mon, Aug 03, 2026 at 11:50:41AM +0100, Lorenzo Stoakes (ARM) wrote:quoted
On Mon, Aug 03, 2026 at 09:48:19AM +0000, Alice Ryhl wrote:quoted
On Sun, Aug 02, 2026 at 02:54:56PM -0700, Suren Baghdasaryan wrote:quoted
From: Dave Hansen <dave.hansen@linux.intel.com> 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 <dave.hansen@linux.intel.com> Signed-off-by: Suren Baghdasaryan <surenb@google.com>Reviewed-by: Alice Ryhl <aliceryhl@google.com>Is there anything on the rust side that needs to be changed also?Ah ok addressed in 4/5, disregard :)
OK disregard the disregard :)) that addresses something different. So question remains :P -- Cheers, Lorenzo