All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock
@ 2026-07-07 10:28 Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Alice Ryhl @ 2026-07-07 10:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Carlos Llamas
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Matthew Maurer, rust-for-linux, linux-kernel

Unfortunately the current use of a mutex for this lock leads to priority
inversion. Traces have been observed where a process is trying to obtain
this mutex for 22ms, but it's unable to do so because the thread holding
the lock is scheduled out. Since this occurred on a UI thread, that is
an extremely long delay.

This patch series fixes that by first making a series of changes that
remove the possibility of sleeping under the lock, and then finally
changing it to a spinlock.

Based on top of:
https://lore.kernel.org/all/20260707-binder-netlink-v7-0-42b40e4b1ac8@google.com/

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v4:
- Fix conflict with commit bc4a98288978 ("rust_binder: clear freeze listener on node removal")
- Rebase on top of v7.2-rc1 and netlink series.
- Link to v3: https://lore.kernel.org/r/20260615-binder-noderefs-spin-v3-0-3235f5a3e0a0@google.com

Changes in v3:
- Clone `node` after loop in freeze listener patch.
- Inline `resize_for_add_freeze_listener` in freeze listener patch.
- Pick up Reviewed-by from Matthew.
- Link to v2: https://lore.kernel.org/r/20260609-binder-noderefs-spin-v2-0-eafde2ff376c@google.com

Changes in v2:
- Make more tweaks to various critical regions and extract them to
  separate commits.
- Link to v1: https://lore.kernel.org/r/20260608-binder-noderefs-spin-v1-1-2584cb4e49ff@google.com

---
Alice Ryhl (6):
      rust_binder: avoid allocating under node_refs for freeze listeners
      rust_binder: avoid dropping NodeRef in update_ref() under lock
      rust_binder: schedule NodeDeath outside of node_refs lock
      rust_binder: keep NodeDeath in NodeRefInfo during process cleanup
      rust_binder: avoid destructors in insert_or_update_handle()
      rust_binder: update Process::node_refs to use SpinLock

 drivers/android/binder/freeze.rs  | 65 ++++++++++++++++++++++++++-------------
 drivers/android/binder/node.rs    | 41 ++++++++++++------------
 drivers/android/binder/process.rs | 57 ++++++++++++++++++++--------------
 3 files changed, 98 insertions(+), 65 deletions(-)
---
base-commit: ffdd0cba394c502b1d7c936586b2a09bb235aa8b
change-id: 20260608-binder-noderefs-spin-3a0ec0589043

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners
  2026-07-07 10:28 [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
@ 2026-07-07 10:28 ` Alice Ryhl
  2026-07-17 13:20   ` Greg Kroah-Hartman
  2026-07-07 10:28 ` [PATCH v4 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock Alice Ryhl
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Alice Ryhl @ 2026-07-07 10:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Carlos Llamas
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Matthew Maurer, rust-for-linux, linux-kernel

The node_refs mutex needs to be changed to a spinlock, so in preparation
for that, update freeze.rs to avoid allocating under the node_refs lock.
This is done by adding a retry loop so that if add_freeze_listener()
requires reallocating the KVVec<_> of freeze listeners, the caller will
allocate a larger vector and retry.

Analogously, the remove_freeze_listener() function is updated to return
the empty KVVec<_> when it is no longer needed, to avoid calling
kvfree() under the node_refs lock.

Reviewed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/freeze.rs | 65 +++++++++++++++++++++++++++-------------
 drivers/android/binder/node.rs   | 41 +++++++++++++------------
 2 files changed, 64 insertions(+), 42 deletions(-)

diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
index e0bc159da63d..f43388ed6ae2 100644
--- a/drivers/android/binder/freeze.rs
+++ b/drivers/android/binder/freeze.rs
@@ -180,36 +180,58 @@ pub(crate) fn request_freeze_notif(
         let msg = FreezeMessage::new(GFP_KERNEL)?;
         let alloc = RBTreeNodeReservation::new(GFP_KERNEL)?;
 
+        let mut afl_vec_alloc = KVVec::new();
+        let mut info;
+        let mut freeze_entry;
         let mut node_refs_guard = self.node_refs.lock();
-        let node_refs = &mut *node_refs_guard;
-        let Some(info) = node_refs.by_handle.get_mut(&handle) else {
-            pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION invalid ref {}\n", handle);
-            return Err(EINVAL);
-        };
-        if info.freeze().is_some() {
-            pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION already set\n");
-            return Err(EINVAL);
-        }
-        let node_ref = info.node_ref();
-        let freeze_entry = node_refs.freeze_listeners.entry(cookie);
-
-        if let rbtree::Entry::Occupied(ref dupe) = freeze_entry {
-            if !dupe.get().allow_duplicate(&node_ref.node) {
-                pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION duplicate cookie\n");
+        loop {
+            let node_refs = &mut *node_refs_guard;
+            info = match node_refs.by_handle.get_mut(&handle) {
+                Some(info) => info,
+                None => {
+                    pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION invalid ref {}\n", handle);
+                    return Err(EINVAL);
+                }
+            };
+            if info.freeze().is_some() {
+                pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION already set\n");
                 return Err(EINVAL);
             }
-        }
+            let node_ref = info.node_ref();
+            freeze_entry = node_refs.freeze_listeners.entry(cookie);
+
+            if let rbtree::Entry::Occupied(ref dupe) = freeze_entry {
+                if !dupe.get().allow_duplicate(&node_ref.node) {
+                    pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION duplicate cookie\n");
+                    return Err(EINVAL);
+                }
+            }
 
-        // All failure paths must come before this call, and all modifications must come after this
-        // call.
-        node_ref.node.add_freeze_listener(self, GFP_KERNEL)?;
+            // Now we add to the node's freeze listener list, with retry and re-allocate if the
+            // vector is full.
+            //
+            // To ensure that the node is added atomically, this is the first time we modify any
+            // state. When this call succeeds, all other modifications must occur without the
+            // possibility for any failure paths.
+            match node_ref
+                .node
+                .add_freeze_listener(self, &mut afl_vec_alloc)?
+            {
+                Ok(()) => break,
+                Err(resize_target) => {
+                    drop(node_refs_guard);
+                    afl_vec_alloc = KVVec::with_capacity(resize_target, GFP_KERNEL)?;
+                    node_refs_guard = self.node_refs.lock();
+                }
+            }
+        }
 
         match freeze_entry {
             rbtree::Entry::Vacant(entry) => {
                 entry.insert(
                     FreezeListener {
                         cookie,
-                        node: node_ref.node.clone(),
+                        node: info.node_ref().node.clone(),
                         last_is_frozen: None,
                         is_pending: false,
                         is_clearing: false,
@@ -280,6 +302,7 @@ pub(crate) fn clear_freeze_notif(self: &Arc<Self>, reader: &mut UserSliceReader)
         let handle = hc.handle;
         let cookie = FreezeCookie(hc.cookie);
 
+        let _to_free_fl;
         let alloc = FreezeMessage::new(GFP_KERNEL)?;
         let mut node_refs_guard = self.node_refs.lock();
         let node_refs = &mut *node_refs_guard;
@@ -300,7 +323,7 @@ pub(crate) fn clear_freeze_notif(self: &Arc<Self>, reader: &mut UserSliceReader)
             return Err(EINVAL);
         };
         listener.is_clearing = true;
-        listener.node.remove_freeze_listener(self);
+        _to_free_fl = listener.node.remove_freeze_listener(self);
         *info.freeze() = None;
         let mut msg = None;
         if !listener.is_pending {
diff --git a/drivers/android/binder/node.rs b/drivers/android/binder/node.rs
index 979366276680..59c5ab747bf4 100644
--- a/drivers/android/binder/node.rs
+++ b/drivers/android/binder/node.rs
@@ -659,29 +659,26 @@ fn do_work_locked(
     pub(crate) fn add_freeze_listener(
         &self,
         process: &Arc<Process>,
-        flags: kernel::alloc::Flags,
-    ) -> Result {
-        let mut vec_alloc = KVVec::<Arc<Process>>::new();
-        loop {
-            let mut guard = self.owner.inner.lock();
-            // Do not check for `guard.dead`. The `dead` flag that matters here is the owner of the
-            // listener, no the target.
-            let inner = self.inner.access_mut(&mut guard);
-            let len = inner.freeze_list.len();
-            if len >= inner.freeze_list.capacity() {
-                if len >= vec_alloc.capacity() {
-                    drop(guard);
-                    vec_alloc = KVVec::with_capacity((1 + len).next_power_of_two(), flags)?;
-                    continue;
-                }
-                mem::swap(&mut inner.freeze_list, &mut vec_alloc);
-                for elem in vec_alloc.drain_all() {
-                    inner.freeze_list.push_within_capacity(elem)?;
-                }
+        // If the vector needs to be resized, it's done via this argument.
+        vec_alloc: &mut KVVec<Arc<Process>>,
+    ) -> Result<Result<(), usize>> {
+        let mut guard = self.owner.inner.lock();
+        // Do not check for `guard.dead`. The `dead` flag that matters here is the owner of the
+        // listener, not the target.
+        let inner = self.inner.access_mut(&mut guard);
+        let len = inner.freeze_list.len();
+        if len == inner.freeze_list.capacity() {
+            if len >= vec_alloc.capacity() {
+                // Request the caller to reallocate.
+                return Ok(Err((1 + len).next_power_of_two()));
+            }
+            mem::swap(&mut inner.freeze_list, vec_alloc);
+            for elem in vec_alloc.drain_all() {
+                inner.freeze_list.push_within_capacity(elem)?;
             }
-            inner.freeze_list.push_within_capacity(process.clone())?;
-            return Ok(());
         }
+        inner.freeze_list.push_within_capacity(process.clone())?;
+        Ok(Ok(()))
     }
 
     pub(crate) fn remove_freeze_listener(&self, p: &Process) -> KVVec<Arc<Process>> {
@@ -697,6 +694,8 @@ pub(crate) fn remove_freeze_listener(&self, p: &Process) -> KVVec<Arc<Process>>
                 p.pid_in_current_ns()
             );
         }
+        // If the vector is empty it needs to be freed. However, we can't free it here because that
+        // might sleep, so return it to the caller.
         if inner.freeze_list.is_empty() {
             return mem::take(&mut inner.freeze_list);
         }

-- 
2.55.0.rc2.803.g1fd1e6609c-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock
  2026-07-07 10:28 [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
@ 2026-07-07 10:28 ` Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 3/6] rust_binder: schedule NodeDeath outside of node_refs lock Alice Ryhl
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Alice Ryhl @ 2026-07-07 10:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Carlos Llamas
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Matthew Maurer, rust-for-linux, linux-kernel

In preparation for changing the node_refs lock to a spinlock, move the
cleanup of NodeRefInfo in update_ref() so that it occurs without the
node_refs lock held. This avoids dropping an Arc<Node> with the lock
held. Furthermore, the NodeDeath field is kept in the NodeRefInfo to be
dropped outside the lock as well.

The removal from the rbtree is updated to use remove_node(), which keeps
the rbtree node allocation until after node_refs is unlocked as well.
This is not strictly necessary as it just moves a kfree() outside the
lock, but there's no reason to invoke the kfree() under the lock if we
can easily avoid it, so avoid it.

Reviewed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/process.rs | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 3b8eeac666bb..da84c0072d46 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -946,15 +946,19 @@ pub(crate) fn update_ref(
 
         // To preserve original binder behaviour, we only fail requests where the manager tries to
         // increment references on itself.
+        let _to_free_by_handle;
+        let _to_free_by_node;
         let _to_free_freeze_listener;
         let _to_free_freeze_listener_cleanup;
         let mut refs = self.node_refs.lock();
         if let Some(info) = refs.by_handle.get_mut(&handle) {
             if info.node_ref().update(inc, strong) {
                 // Clean up death if there is one attached to this node reference.
-                if let Some(death) = info.death().take() {
+                //
+                // We remove the entire `info` below, so no need to remove `death` from `info`.
+                if let Some(death) = info.death().as_ref() {
                     death.set_cleared(true);
-                    self.remove_from_delivered_deaths(&death);
+                    self.remove_from_delivered_deaths(death);
                 }
 
                 // Remove reference from process tables, and from the node's `refs` list.
@@ -971,8 +975,8 @@ pub(crate) fn update_ref(
                     }
                 }
 
-                refs.by_handle.remove(&handle);
-                refs.by_node.remove(&id);
+                _to_free_by_handle = refs.by_handle.remove_node(&handle);
+                _to_free_by_node = refs.by_node.remove_node(&id);
                 refs.handle_is_present.release_id(handle as usize);
 
                 if let Some(shrink) = refs.handle_is_present.shrink_request() {

-- 
2.55.0.rc2.803.g1fd1e6609c-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/6] rust_binder: schedule NodeDeath outside of node_refs lock
  2026-07-07 10:28 [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock Alice Ryhl
@ 2026-07-07 10:28 ` Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup Alice Ryhl
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Alice Ryhl @ 2026-07-07 10:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Carlos Llamas
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Matthew Maurer, rust-for-linux, linux-kernel

There's no reason to hold the node_refs lock while scheduling the
NodeDeath to the thread todo list, so don't. The call to set_cleared()
is kept under the lock so that the state update is kept atomic.

Reviewed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/process.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index da84c0072d46..505e0bc52748 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -1303,7 +1303,10 @@ pub(crate) fn clear_death(&self, reader: &mut UserSliceReader, thread: &Thread)
 
         // Update state and determine if we need to queue a work item. We only need to do it when
         // the node is not dead or if the user already completed the death notification.
-        if death.set_cleared(false) {
+        let should_schedule = death.set_cleared(false);
+        drop(refs);
+
+        if should_schedule {
             if let Some(death) = ListArc::try_from_arc_or_drop(death) {
                 let _ = thread.push_work_if_looper(death);
             }

-- 
2.55.0.rc2.803.g1fd1e6609c-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup
  2026-07-07 10:28 [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
                   ` (2 preceding siblings ...)
  2026-07-07 10:28 ` [PATCH v4 3/6] rust_binder: schedule NodeDeath outside of node_refs lock Alice Ryhl
@ 2026-07-07 10:28 ` Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 5/6] rust_binder: avoid destructors in insert_or_update_handle() Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 6/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
  5 siblings, 0 replies; 10+ messages in thread
From: Alice Ryhl @ 2026-07-07 10:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Carlos Llamas
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Matthew Maurer, rust-for-linux, linux-kernel

By keeping the NodeDeath inside the NodeRefInfo structure during process
cleanup, we avoid running its destructor under the node_refs lock. It is
still dropped shortly thereafter when the entire rbtree holding the
NodeRefInfo objects is dropped, but that occurs outside of the lock.

Reviewed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/process.rs | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 505e0bc52748..410218505417 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -1389,13 +1389,11 @@ fn deferred_release(self: Arc<Self>) {
             // SAFETY: We are removing the `NodeRefInfo` from the right node.
             unsafe { info.node_ref2().node.remove_node_info(info) };
 
-            // Remove all death notifications from the nodes (that belong to a different process).
-            let death = if let Some(existing) = info.death().take() {
-                existing
-            } else {
-                continue;
-            };
-            death.set_cleared(false);
+            // Clear death notifications from the nodes (that belong to a different process).
+            // No need to remove them from `info` as we clear info below.
+            if let Some(death) = info.death().as_ref() {
+                death.set_cleared(false);
+            }
         }
 
         // Clean up freeze listeners.

-- 
2.55.0.rc2.803.g1fd1e6609c-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 5/6] rust_binder: avoid destructors in insert_or_update_handle()
  2026-07-07 10:28 [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
                   ` (3 preceding siblings ...)
  2026-07-07 10:28 ` [PATCH v4 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup Alice Ryhl
@ 2026-07-07 10:28 ` Alice Ryhl
  2026-07-07 10:28 ` [PATCH v4 6/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
  5 siblings, 0 replies; 10+ messages in thread
From: Alice Ryhl @ 2026-07-07 10:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Carlos Llamas
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Matthew Maurer, rust-for-linux, linux-kernel

The insert_or_update_handle() function currently has two places where it
drops objects under the node_refs lock. In preparation for changing
node_refs into a spinlock, update the code to either entirely remove the
codepath or drop the node_refs lock first before running the destructor.

This also has the side-benefit that we avoid traversing the by_node
rbtree twice. Currently it's first traversed to see if the new node is
present, and then traversed again to insert it. By saving the
VacantEntry from the first lookup, we can perform the insertion without
traversing the tree again.

Reviewed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/process.rs | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 410218505417..04d39e5fcb63 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -861,14 +861,17 @@ pub(crate) fn insert_or_update_handle(
         let handle = unused_id.as_u32();
 
         // Do a lookup again as node may have been inserted before the lock was reacquired.
-        if let Some(handle_ref) = refs.by_node.get(&node_ref.node.global_id()) {
-            let handle = *handle_ref;
-            let info = refs.by_handle.get_mut(&handle).unwrap();
-            info.node_ref().absorb(node_ref);
-            return Ok(handle);
-        }
+        let by_node_slot = match refs.by_node.entry(node_ref.node.global_id()) {
+            rbtree::Entry::Vacant(by_node_slot) => by_node_slot,
+            rbtree::Entry::Occupied(handle_ref) => {
+                // The node was inserted by another thread while we didn't hold the lock.
+                let handle = handle_ref.get();
+                let info = refs.by_handle.get_mut(handle).unwrap();
+                info.node_ref().absorb(node_ref);
+                return Ok(*handle);
+            }
+        };
 
-        let gid = node_ref.node.global_id();
         let (info_proc, info_node) = {
             let info_init = NodeRefInfo::new(node_ref, handle, self.into());
             match info.pin_init_with(info_init) {
@@ -884,6 +887,9 @@ pub(crate) fn insert_or_update_handle(
         // first thing in `deferred_release`, process cleanup will not miss the items inserted into
         // `refs` below.
         if self.inner.lock().is_dead {
+            // Explicitly drop the lock so that `info_proc` and `info_node` are dropped outside of
+            // the lock.
+            drop(refs_lock);
             return Err(ESRCH);
         }
 
@@ -891,7 +897,7 @@ pub(crate) fn insert_or_update_handle(
         // `info_node` into the right node's `refs` list.
         unsafe { info_proc.node_ref2().node.insert_node_info(info_node) };
 
-        refs.by_node.insert(reserve1.into_node(gid, handle));
+        by_node_slot.insert(handle, reserve1);
         by_handle_slot.insert(info_proc, reserve2);
         unused_id.acquire();
         Ok(handle)

-- 
2.55.0.rc2.803.g1fd1e6609c-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 6/6] rust_binder: update Process::node_refs to use SpinLock
  2026-07-07 10:28 [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
                   ` (4 preceding siblings ...)
  2026-07-07 10:28 ` [PATCH v4 5/6] rust_binder: avoid destructors in insert_or_update_handle() Alice Ryhl
@ 2026-07-07 10:28 ` Alice Ryhl
  5 siblings, 0 replies; 10+ messages in thread
From: Alice Ryhl @ 2026-07-07 10:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Carlos Llamas
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Matthew Maurer, rust-for-linux, linux-kernel

Unfortunately the current use of a mutex for this lock leads to priority
inversion. Traces have been observed where a process is trying to obtain
this mutex for 22ms, but it's unable to do so because the thread holding
the lock is scheduled out. Since this occurred on a UI thread, that is
an extremely long delay.

Code paths that might sleep under this lock have already been updated in
patches leading up to this one.

Reviewed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/process.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 04d39e5fcb63..0555c4bd503e 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -30,7 +30,7 @@
     sync::{
         aref::ARef,
         lock::{spinlock::SpinLockBackend, Guard},
-        Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
+        Arc, ArcBorrow, CondVar, CondVarTimeoutResult, SpinLock, UniqueArc,
     },
     task::{Pid, Task},
     uaccess::{UserSlice, UserSliceReader},
@@ -455,7 +455,7 @@ pub(crate) struct Process {
     // Node references are in a different lock to avoid recursive acquisition when
     // incrementing/decrementing a node in another process.
     #[pin]
-    node_refs: Mutex<ProcessNodeRefs>,
+    node_refs: SpinLock<ProcessNodeRefs>,
 
     // Work node for deferred work item.
     #[pin]
@@ -510,7 +510,7 @@ fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
                 cred,
                 inner <- kernel::new_spinlock!(ProcessInner::new(), "Process::inner"),
                 pages <- ShrinkablePageRange::new(&super::BINDER_SHRINKER),
-                node_refs <- kernel::new_mutex!(ProcessNodeRefs::new(), "Process::node_refs"),
+                node_refs <- kernel::new_spinlock!(ProcessNodeRefs::new(), "Process::node_refs"),
                 freeze_wait <- kernel::new_condvar!("Process::freeze_wait"),
                 task: current.group_leader().into(),
                 defer_work <- kernel::new_work!("Process::defer_work"),

-- 
2.55.0.rc2.803.g1fd1e6609c-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners
  2026-07-07 10:28 ` [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
@ 2026-07-17 13:20   ` Greg Kroah-Hartman
  2026-07-17 13:48     ` Alice Ryhl
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-17 13:20 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Carlos Llamas, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Danilo Krummrich, Matthew Maurer, rust-for-linux,
	linux-kernel

On Tue, Jul 07, 2026 at 10:28:51AM +0000, Alice Ryhl wrote:
> The node_refs mutex needs to be changed to a spinlock, so in preparation
> for that, update freeze.rs to avoid allocating under the node_refs lock.
> This is done by adding a retry loop so that if add_freeze_listener()
> requires reallocating the KVVec<_> of freeze listeners, the caller will
> allocate a larger vector and retry.
> 
> Analogously, the remove_freeze_listener() function is updated to return
> the empty KVVec<_> when it is no longer needed, to avoid calling
> kvfree() under the node_refs lock.
> 
> Reviewed-by: Matthew Maurer <mmaurer@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  drivers/android/binder/freeze.rs | 65 +++++++++++++++++++++++++++-------------
>  drivers/android/binder/node.rs   | 41 +++++++++++++------------
>  2 files changed, 64 insertions(+), 42 deletions(-)
> 

This doesn't apply against my tree now, given all of the binder patches
now added?  Can you rebase against char-misc-testing and resend?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners
  2026-07-17 13:20   ` Greg Kroah-Hartman
@ 2026-07-17 13:48     ` Alice Ryhl
  2026-07-17 14:35       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Alice Ryhl @ 2026-07-17 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Carlos Llamas, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Danilo Krummrich, Matthew Maurer, rust-for-linux,
	linux-kernel

On Fri, Jul 17, 2026 at 3:20 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jul 07, 2026 at 10:28:51AM +0000, Alice Ryhl wrote:
> > The node_refs mutex needs to be changed to a spinlock, so in preparation
> > for that, update freeze.rs to avoid allocating under the node_refs lock.
> > This is done by adding a retry loop so that if add_freeze_listener()
> > requires reallocating the KVVec<_> of freeze listeners, the caller will
> > allocate a larger vector and retry.
> >
> > Analogously, the remove_freeze_listener() function is updated to return
> > the empty KVVec<_> when it is no longer needed, to avoid calling
> > kvfree() under the node_refs lock.
> >
> > Reviewed-by: Matthew Maurer <mmaurer@google.com>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  drivers/android/binder/freeze.rs | 65 +++++++++++++++++++++++++++-------------
> >  drivers/android/binder/node.rs   | 41 +++++++++++++------------
> >  2 files changed, 64 insertions(+), 42 deletions(-)
> >
>
> This doesn't apply against my tree now, given all of the binder patches
> now added?  Can you rebase against char-misc-testing and resend?

I believe this is already in char-misc-next as commit b9d17aa74ddd
("rust_binder: avoid allocating under node_refs for freeze
listeners").

Alice

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners
  2026-07-17 13:48     ` Alice Ryhl
@ 2026-07-17 14:35       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-17 14:35 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Carlos Llamas, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Danilo Krummrich, Matthew Maurer, rust-for-linux,
	linux-kernel

On Fri, Jul 17, 2026 at 03:48:07PM +0200, Alice Ryhl wrote:
> On Fri, Jul 17, 2026 at 3:20 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jul 07, 2026 at 10:28:51AM +0000, Alice Ryhl wrote:
> > > The node_refs mutex needs to be changed to a spinlock, so in preparation
> > > for that, update freeze.rs to avoid allocating under the node_refs lock.
> > > This is done by adding a retry loop so that if add_freeze_listener()
> > > requires reallocating the KVVec<_> of freeze listeners, the caller will
> > > allocate a larger vector and retry.
> > >
> > > Analogously, the remove_freeze_listener() function is updated to return
> > > the empty KVVec<_> when it is no longer needed, to avoid calling
> > > kvfree() under the node_refs lock.
> > >
> > > Reviewed-by: Matthew Maurer <mmaurer@google.com>
> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > ---
> > >  drivers/android/binder/freeze.rs | 65 +++++++++++++++++++++++++++-------------
> > >  drivers/android/binder/node.rs   | 41 +++++++++++++------------
> > >  2 files changed, 64 insertions(+), 42 deletions(-)
> > >
> >
> > This doesn't apply against my tree now, given all of the binder patches
> > now added?  Can you rebase against char-misc-testing and resend?
> 
> I believe this is already in char-misc-next as commit b9d17aa74ddd
> ("rust_binder: avoid allocating under node_refs for freeze
> listeners").

Ok, sorry about that.

I've now gone through all pending patches that I saw on my side for the
binder code.  There were 2 that needs review from you:
	https://lore.kernel.org/r/20260618121202.6258-1-iganschel@gmail.com
	https://lore.kernel.org/r/20260616170956.2580772-1-georgeandrout13@gmail.com

and then there's the ratelimit patches outstanding too.

If I've missed anything else, please resend.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-07-17 14:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 10:28 [PATCH v4 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
2026-07-07 10:28 ` [PATCH v4 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
2026-07-17 13:20   ` Greg Kroah-Hartman
2026-07-17 13:48     ` Alice Ryhl
2026-07-17 14:35       ` Greg Kroah-Hartman
2026-07-07 10:28 ` [PATCH v4 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock Alice Ryhl
2026-07-07 10:28 ` [PATCH v4 3/6] rust_binder: schedule NodeDeath outside of node_refs lock Alice Ryhl
2026-07-07 10:28 ` [PATCH v4 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup Alice Ryhl
2026-07-07 10:28 ` [PATCH v4 5/6] rust_binder: avoid destructors in insert_or_update_handle() Alice Ryhl
2026-07-07 10:28 ` [PATCH v4 6/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.