Re: [PATCH v3 4/5] binder: Remove mmap_lock fallback

On Mon, Aug 3, 2026 at 4:33 AM Lorenzo Stoakes (ARM) [off-list ref] wrote:

On Sun, Aug 02, 2026 at 02:54:58PM -0700, Suren Baghdasaryan wrote:

quoted

From: Dave Hansen <dave.hansen@linux.intel.com>

Previously, the per-VMA locking could fail in the face of writers
which necessitate a fallback to mmap_lock. The new
vma_start_read_unlocked() will wait for writers instead of failing.

Use the new helper. Wait for writers. Remove the fallback to mmap_lock.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>

LGTM, just a nit below.

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Thanks!

quoted

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Liam R. Howlett <redacted>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: linux-mm@kvack.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Todd Kjos <tkjos@android.com>
Cc: Christian Brauner <christian@brauner.io>
Cc: Carlos Llamas <cmllamas@google.com>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: netdev@vger.kernel.org
---
 drivers/android/binder/page_range.rs | 19 +++----------------
 drivers/android/binder_alloc.c       | 17 +++++------------
 rust/kernel/mm.rs                    | 18 ++++++++++++++++++
 3 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
index e82a5523804f..f7ad88a0d806 100644
--- a/drivers/android/binder/page_range.rs
+++ b/drivers/android/binder/page_range.rs

@@ -439,22 +439,9 @@ unsafe fn use_page_slow(&self, i: usize) -> Result<()> {
         // workqueue.
         let mm = MmWithUser::into_mmput_async(self.mm.mmget_not_zero().ok_or(ESRCH)?);
         {
-            let vma_read;
-            let mmap_read;
-            let vma = if let Some(ret) = mm.lock_vma_under_rcu(vma_addr) {
-                vma_read = ret;
-                check_vma(&vma_read, self)
-            } else {
-                mmap_read = mm.mmap_read_lock();
-                mmap_read
-                    .vma_lookup(vma_addr)
-                    .and_then(|vma| check_vma(vma, self))
-            };
-
-            match vma {
-                Some(vma) => vma.vm_insert_page(user_page_addr, &new_page)?,
-                None => return Err(ESRCH),
-            }
+            let vma_read_guard = mm.vma_start_read_unlocked(vma_addr).ok_or(ESRCH)?;
+            let vma = check_vma(&vma_read_guard, self).ok_or(ESRCH)?;
+            vma.vm_insert_page(user_page_addr, &new_page)?;
         }

         let inner = self.lock.lock();

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 84104ba04e30..519dcded19b2 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c

@@ -259,21 +259,14 @@ static int binder_page_insert(struct binder_alloc *alloc,
      struct vm_area_struct *vma;
      int ret = -ESRCH;

-     /* attempt per-vma lock first */
-     vma = lock_vma_under_rcu(mm, addr);
-     if (vma) {
-             if (binder_alloc_is_mapped(alloc))
-                     ret = vm_insert_page(vma, addr, page);
-             vma_end_read(vma);
+     vma = vma_start_read_unlocked(mm, addr);
+     if (!vma)
              return ret;
-     }

-     /* fall back to mmap_lock */
-     mmap_read_lock(mm);
-     vma = vma_lookup(mm, addr);
-     if (vma && binder_alloc_is_mapped(alloc))
+     if (binder_alloc_is_mapped(alloc))
              ret = vm_insert_page(vma, addr, page);
-     mmap_read_unlock(mm);
+
+     vma_end_read(vma);

Nice cleanup :)

quoted

      return ret;
 }

diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs
index 2633e704c83d..877fad68be9c 100644
--- a/rust/kernel/mm.rs
+++ b/rust/kernel/mm.rs

@@ -190,6 +190,24 @@ pub fn lock_vma_under_rcu(&self, vma_addr: usize) -> Option<VmaReadGuard<'_>> {
         }
     }

+    /// Find the VMA covering 'address' and lock it for reading. Waits for writers to finish if the
+    /// VMA is being modified.

This seems a little inconsistent with the C version's comment, should they not be the same?

Ack. Will change.

quoted

+    #[inline]
+    pub fn vma_start_read_unlocked(&self, vma_addr: usize) -> Option<VmaReadGuard<'_>> {
+        // SAFETY: We may invoke `vma_start_read_unlocked` because we know this `mm` has non-zero
+        // `mm_users`.
+        let vma = unsafe { bindings::vma_start_read_unlocked(self.as_raw(), vma_addr) };
+        if vma.is_null() {
+            return None;
+        }
+        Some(VmaReadGuard {
+            // SAFETY: If `vma_start_read_unlocked` returns a non-null ptr, then it points at a
+            // valid vma. The vma is stable for as long as the vma read lock is held.
+            vma: unsafe { VmaRef::from_raw(vma) },
+            _nts: NotThreadSafe,
+        })
+    }
+
     /// Lock the mmap read lock.
     #[inline]
     pub fn mmap_read_lock(&self) -> MmapReadGuard<'_> {
--
2.55.0.508.g3f0d502094-goog

--
Cheers, Lorenzo