* [PATCH v2 1/6] rust_binder: avoid allocating under node_refs for freeze listeners
2026-06-09 9:33 [PATCH v2 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
@ 2026-06-09 9:33 ` Alice Ryhl
2026-06-11 22:18 ` Matthew Maurer
2026-06-09 9:33 ` [PATCH v2 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock Alice Ryhl
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Alice Ryhl @ 2026-06-09 9:33 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, 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.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
drivers/android/binder/freeze.rs | 67 +++++++++++++++++++++++++++-------------
drivers/android/binder/node.rs | 55 +++++++++++++++++++--------------
2 files changed, 77 insertions(+), 45 deletions(-)
diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
index 53b60035639a..20041689e98d 100644
--- a/drivers/android/binder/freeze.rs
+++ b/drivers/android/binder/freeze.rs
@@ -173,36 +173,60 @@ 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 node;
+ 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();
+ node = node_ref.node.clone();
+ 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);
+ Node::resize_for_add_freeze_listener(&mut afl_vec_alloc, resize_target)?;
+ node_refs_guard = self.node_refs.lock();
+ }
+ }
+ }
match freeze_entry {
rbtree::Entry::Vacant(entry) => {
entry.insert(
FreezeListener {
cookie,
- node: node_ref.node.clone(),
+ node,
last_is_frozen: None,
is_pending: false,
is_clearing: false,
@@ -273,6 +297,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;
@@ -293,7 +318,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 69f757ff7461..fb27674a8c94 100644
--- a/drivers/android/binder/node.rs
+++ b/drivers/android/binder/node.rs
@@ -657,33 +657,37 @@ 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));
+ }
+ 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 resize_for_add_freeze_listener(
+ vec_alloc: &mut KVVec<Arc<Process>>,
+ target_size: usize,
+ ) -> Result {
+ *vec_alloc = KVVec::with_capacity(target_size.next_power_of_two(), GFP_KERNEL)?;
+ Ok(())
}
- pub(crate) fn remove_freeze_listener(&self, p: &Arc<Process>) {
- let _unused_capacity;
+ pub(crate) fn remove_freeze_listener(&self, p: &Arc<Process>) -> KVVec<Arc<Process>> {
let mut guard = self.owner.inner.lock();
let inner = self.inner.access_mut(&mut guard);
let len = inner.freeze_list.len();
@@ -694,9 +698,12 @@ pub(crate) fn remove_freeze_listener(&self, p: &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() {
- _unused_capacity = mem::take(&mut inner.freeze_list);
+ return mem::take(&mut inner.freeze_list);
}
+ KVVec::new()
}
pub(crate) fn freeze_list<'a>(&'a self, guard: &'a ProcessInner) -> &'a [Arc<Process>] {
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/6] rust_binder: avoid allocating under node_refs for freeze listeners
2026-06-09 9:33 ` [PATCH v2 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
@ 2026-06-11 22:18 ` Matthew Maurer
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Maurer @ 2026-06-11 22:18 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
On Tue, Jun 9, 2026 at 2:46 AM Alice Ryhl <aliceryhl@google.com> 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.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Nit: Consider moving the clone to the success path at the end, since
the cloned node is not used until then? It's probably cheap enough
that it doesn't matter though.
>
> + node = node_ref.node.clone();
This is wrapping a single line, and is only called once. Why add a
separate function for it?
> + pub(crate) fn resize_for_add_freeze_listener(
> + vec_alloc: &mut KVVec<Arc<Process>>,
> + target_size: usize,
> + ) -> Result {
> + *vec_alloc = KVVec::with_capacity(target_size.next_power_of_two(), GFP_KERNEL)?;
> + Ok(())
> }
Otherwise looks fine.
Reviewed-By: Matthew Maurer <mmaurer@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock
2026-06-09 9:33 [PATCH v2 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
2026-06-09 9:33 ` [PATCH v2 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
@ 2026-06-09 9:33 ` Alice Ryhl
2026-06-11 22:21 ` Matthew Maurer
2026-06-09 9:33 ` [PATCH v2 3/6] rust_binder: schedule NodeDeath outside of node_refs lock Alice Ryhl
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Alice Ryhl @ 2026-06-09 9:33 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, 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.
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 96b8440ceac6..f4f4d45e1ab7 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -942,13 +942,17 @@ 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 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.
@@ -957,8 +961,8 @@ pub(crate) fn update_ref(
unsafe { info.node_ref2().node.remove_node_info(info) };
let id = info.node_ref().node.global_id();
- 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.54.0.1064.gd145956f57-goog
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock
2026-06-09 9:33 ` [PATCH v2 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock Alice Ryhl
@ 2026-06-11 22:21 ` Matthew Maurer
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Maurer @ 2026-06-11 22:21 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
On Tue, Jun 9, 2026 at 2:44 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> 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.
>
> 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 96b8440ceac6..f4f4d45e1ab7 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -942,13 +942,17 @@ 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 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.
> @@ -957,8 +961,8 @@ pub(crate) fn update_ref(
> unsafe { info.node_ref2().node.remove_node_info(info) };
>
> let id = info.node_ref().node.global_id();
> - 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.54.0.1064.gd145956f57-goog
>
>
Reviewed-by: Matthew Maurer <mmaurer@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/6] rust_binder: schedule NodeDeath outside of node_refs lock
2026-06-09 9:33 [PATCH v2 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
2026-06-09 9:33 ` [PATCH v2 1/6] rust_binder: avoid allocating under node_refs for freeze listeners Alice Ryhl
2026-06-09 9:33 ` [PATCH v2 2/6] rust_binder: avoid dropping NodeRef in update_ref() under lock Alice Ryhl
@ 2026-06-09 9:33 ` Alice Ryhl
2026-06-11 22:22 ` Matthew Maurer
2026-06-09 9:33 ` [PATCH v2 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup Alice Ryhl
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Alice Ryhl @ 2026-06-09 9:33 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, 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.
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 f4f4d45e1ab7..a8836cb8b8cc 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -1289,7 +1289,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.54.0.1064.gd145956f57-goog
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/6] rust_binder: schedule NodeDeath outside of node_refs lock
2026-06-09 9:33 ` [PATCH v2 3/6] rust_binder: schedule NodeDeath outside of node_refs lock Alice Ryhl
@ 2026-06-11 22:22 ` Matthew Maurer
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Maurer @ 2026-06-11 22:22 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
On Tue, Jun 9, 2026 at 2:34 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> 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.
>
> 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 f4f4d45e1ab7..a8836cb8b8cc 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -1289,7 +1289,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.54.0.1064.gd145956f57-goog
>
>
Reviewed-by: Matthew Maurer <mmaurer@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup
2026-06-09 9:33 [PATCH v2 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
` (2 preceding siblings ...)
2026-06-09 9:33 ` [PATCH v2 3/6] rust_binder: schedule NodeDeath outside of node_refs lock Alice Ryhl
@ 2026-06-09 9:33 ` Alice Ryhl
2026-06-11 23:16 ` Matthew Maurer
2026-06-09 9:33 ` [PATCH v2 5/6] rust_binder: avoid destructors in insert_or_update_handle() Alice Ryhl
2026-06-09 9:33 ` [PATCH v2 6/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
5 siblings, 1 reply; 13+ messages in thread
From: Alice Ryhl @ 2026-06-09 9:33 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, 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.
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 a8836cb8b8cc..c8e9fd0d0472 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -1375,13 +1375,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.54.0.1064.gd145956f57-goog
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup
2026-06-09 9:33 ` [PATCH v2 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup Alice Ryhl
@ 2026-06-11 23:16 ` Matthew Maurer
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Maurer @ 2026-06-11 23:16 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
On Tue, Jun 9, 2026 at 2:45 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> 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.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> drivers/android/binder/process.rs | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
Reviewed-by: Matthew Maurer <mmaurer@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] rust_binder: avoid destructors in insert_or_update_handle()
2026-06-09 9:33 [PATCH v2 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
` (3 preceding siblings ...)
2026-06-09 9:33 ` [PATCH v2 4/6] rust_binder: keep NodeDeath in NodeRefInfo during process cleanup Alice Ryhl
@ 2026-06-09 9:33 ` Alice Ryhl
2026-06-11 23:36 ` Matthew Maurer
2026-06-09 9:33 ` [PATCH v2 6/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
5 siblings, 1 reply; 13+ messages in thread
From: Alice Ryhl @ 2026-06-09 9:33 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, 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.
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 c8e9fd0d0472..268d78f8cfb3 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.54.0.1064.gd145956f57-goog
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/6] rust_binder: avoid destructors in insert_or_update_handle()
2026-06-09 9:33 ` [PATCH v2 5/6] rust_binder: avoid destructors in insert_or_update_handle() Alice Ryhl
@ 2026-06-11 23:36 ` Matthew Maurer
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Maurer @ 2026-06-11 23:36 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
On Tue, Jun 9, 2026 at 2:34 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> 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.
>
> 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 c8e9fd0d0472..268d78f8cfb3 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.54.0.1064.gd145956f57-goog
>
>
Reviewed-by: Matthew Maurer <mmaurer@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] rust_binder: update Process::node_refs to use SpinLock
2026-06-09 9:33 [PATCH v2 0/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
` (4 preceding siblings ...)
2026-06-09 9:33 ` [PATCH v2 5/6] rust_binder: avoid destructors in insert_or_update_handle() Alice Ryhl
@ 2026-06-09 9:33 ` Alice Ryhl
2026-06-11 23:37 ` Matthew Maurer
5 siblings, 1 reply; 13+ messages in thread
From: Alice Ryhl @ 2026-06-09 9:33 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, 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.
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 268d78f8cfb3..82c34a93660e 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::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.54.0.1064.gd145956f57-goog
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 6/6] rust_binder: update Process::node_refs to use SpinLock
2026-06-09 9:33 ` [PATCH v2 6/6] rust_binder: update Process::node_refs to use SpinLock Alice Ryhl
@ 2026-06-11 23:37 ` Matthew Maurer
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Maurer @ 2026-06-11 23:37 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
On Tue, Jun 9, 2026 at 2:45 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> 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.
>
> 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 268d78f8cfb3..82c34a93660e 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::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.54.0.1064.gd145956f57-goog
>
>
Reviewed-by: Matthew Maurer <mmaurer@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread