* [PATCH v3 1/2] rust: poll: use kfree_rcu() for PollCondVar
2026-05-08 6:55 [PATCH v3 0/2] Avoid synchronize_rcu() for every thread drop in Rust Binder Alice Ryhl
@ 2026-05-08 6:55 ` Alice Ryhl
2026-05-08 6:55 ` [PATCH v3 2/2] rust_binder: move (e)poll wait queue to Process Alice Ryhl
1 sibling, 0 replies; 3+ messages in thread
From: Alice Ryhl @ 2026-05-08 6:55 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas, Christian Brauner, Boqun Feng
Cc: Paul E. McKenney, Alexander Viro, Jan Kara, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Gary Guo, linux-fsdevel,
rust-for-linux, linux-kernel, Alice Ryhl
Rust Binder currently uses PollCondVar, but it calls synchronize_rcu()
in the destructor, which we would like to avoid. Add a variation of
PollCondVar that kfree_rcu() instead.
One could avoid the `rcu` field and allocate the rcu_head on drop using
a fallback to synchronize_rcu() on ENOMEM. However, I'd prefer to avoid
the potential for synchronize_rcu(), and Binder will only use this for a
small fraction of processes, so even if it changes which kmalloc bucket
it falls into, the extra memory is not a problem.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/sync/poll.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 72 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index 0ec985d560c8..684dfa242b1a 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -5,12 +5,18 @@
//! Utilities for working with `struct poll_table`.
use crate::{
+ alloc::AllocError,
bindings,
fs::File,
prelude::*,
sync::{CondVar, LockClassKey},
+ types::Opaque, //
+};
+use core::{
+ marker::PhantomData,
+ mem::ManuallyDrop,
+ ops::Deref, //
};
-use core::{marker::PhantomData, ops::Deref};
/// Creates a [`PollCondVar`] initialiser with the given name and a newly-created lock class.
#[macro_export]
@@ -66,6 +72,7 @@ pub fn register_wait(&self, file: &File, cv: &PollCondVar) {
///
/// [`CondVar`]: crate::sync::CondVar
#[pin_data(PinnedDrop)]
+#[repr(transparent)]
pub struct PollCondVar {
#[pin]
inner: CondVar,
@@ -104,3 +111,67 @@ fn drop(self: Pin<&mut Self>) {
unsafe { bindings::synchronize_rcu() };
}
}
+
+/// A [`KBox<PollCondVar>`] that uses `kfree_rcu`.
+///
+/// [`KBox<PollCondVar>`]: PollCondVar
+pub struct PollCondVarBox {
+ inner: ManuallyDrop<Pin<KBox<PollCondVarBoxInner>>>,
+}
+
+#[pin_data]
+#[repr(C)]
+struct PollCondVarBoxInner {
+ #[pin]
+ inner: PollCondVar,
+ rcu: Opaque<bindings::callback_head>,
+}
+
+// SAFETY: PollCondVar is Send
+unsafe impl Send for PollCondVarBoxInner {}
+// SAFETY: PollCondVar is Sync
+unsafe impl Sync for PollCondVarBoxInner {}
+
+impl PollCondVarBox {
+ /// Constructs a new boxed [`PollCondVar`].
+ pub fn new(name: &'static CStr, key: Pin<&'static LockClassKey>) -> Result<Self, AllocError> {
+ let b = KBox::pin_init(
+ pin_init!(PollCondVarBoxInner {
+ inner <- PollCondVar::new(name, key),
+ rcu: Opaque::uninit(),
+ }),
+ GFP_KERNEL,
+ )
+ .map_err(|_| AllocError)?;
+
+ Ok(PollCondVarBox {
+ inner: ManuallyDrop::new(b),
+ })
+ }
+}
+
+impl Deref for PollCondVarBox {
+ type Target = PollCondVar;
+ fn deref(&self) -> &PollCondVar {
+ &self.inner.inner
+ }
+}
+
+impl Drop for PollCondVarBox {
+ #[inline]
+ fn drop(&mut self) {
+ // SAFETY: ManuallyDrop::take ok because not already taken.
+ let boxed = unsafe { ManuallyDrop::take(&mut self.inner) };
+
+ // SAFETY: The code below frees the box without calling the actual destructor of the type,
+ // but it's okay because it re-implements the destructor using `kfree_rcu()` in place of
+ // `synchronize_rcu()`.
+ let ptr = KBox::into_raw(unsafe { Pin::into_inner_unchecked(boxed) });
+
+ // SAFETY: The pointer points at a valid `wait_queue_head`.
+ unsafe { bindings::__wake_up_pollfree((*ptr).inner.inner.wait_queue_head.get()) };
+
+ // SAFETY: This was allocated using `KBox::pin_init`, so it can be freed with `kvfree`.
+ unsafe { bindings::kvfree_call_rcu((*ptr).rcu.get(), ptr.cast::<ffi::c_void>()) };
+ }
+}
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v3 2/2] rust_binder: move (e)poll wait queue to Process
2026-05-08 6:55 [PATCH v3 0/2] Avoid synchronize_rcu() for every thread drop in Rust Binder Alice Ryhl
2026-05-08 6:55 ` [PATCH v3 1/2] rust: poll: use kfree_rcu() for PollCondVar Alice Ryhl
@ 2026-05-08 6:55 ` Alice Ryhl
1 sibling, 0 replies; 3+ messages in thread
From: Alice Ryhl @ 2026-05-08 6:55 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas, Christian Brauner, Boqun Feng
Cc: Paul E. McKenney, Alexander Viro, Jan Kara, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Gary Guo, linux-fsdevel,
rust-for-linux, linux-kernel, Alice Ryhl
Most processes do not use Rust Binder with epoll, so avoid paying the
synchronize_rcu() cost in drop for those that don't need it. For those
that do, we also manage to replace synchronize_rcu() with kfree_rcu(),
though we introduce an extra allocation.
In case the last ref to an Arc<Thread> is dropped outside of
deferred_release(), this also ensures that synchronize_rcu() is not
called in destructor of Arc<Thread> in other places. Theoretically that
could lead to jank by making other syscalls slow, which would be
problematic.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
drivers/android/binder/process.rs | 51 +++++++++++++++++------
drivers/android/binder/thread.rs | 78 +++++++++++++++++------------------
drivers/android/binder/transaction.rs | 4 ++
3 files changed, 82 insertions(+), 51 deletions(-)
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 820cbd541435..29fdfc964685 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -30,6 +30,7 @@
sync::{
aref::ARef,
lock::{spinlock::SpinLockBackend, Guard},
+ poll::PollCondVarBox,
Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
},
task::Task,
@@ -135,6 +136,7 @@ pub(crate) struct ProcessInner {
pub(crate) binderfs_file: Option<BinderfsProcFile>,
/// Check for oneway spam
oneway_spam_detection_enabled: bool,
+ poll: Option<PollCondVarBox>,
}
impl ProcessInner {
@@ -158,6 +160,7 @@ fn new() -> Self {
async_recv: false,
binderfs_file: None,
oneway_spam_detection_enabled: false,
+ poll: None,
}
}
@@ -174,19 +177,23 @@ pub(crate) fn push_work(
&mut self,
work: DLArc<dyn DeliverToRead>,
) -> Result<(), (BinderError, DLArc<dyn DeliverToRead>)> {
+ let sync = work.should_sync_wakeup();
+
// Try to find a ready thread to which to push the work.
if let Some(thread) = self.ready_threads.pop_front() {
// Push to thread while holding state lock. This prevents the thread from giving up
// (for example, because of a signal) when we're about to deliver work.
- match thread.push_work(work) {
+ match thread.push_work_inner(work, sync) {
PushWorkRes::Ok => Ok(()),
+ PushWorkRes::OkNotifyPoll => {
+ self.notify_poll(sync);
+ Ok(())
+ }
PushWorkRes::FailedDead(work) => Err((BinderError::new_dead(), work)),
}
} else if self.is_dead {
Err((BinderError::new_dead(), work))
} else {
- let sync = work.should_sync_wakeup();
-
// Didn't find a thread waiting for proc work; this can happen
// in two scenarios:
// 1. All threads are busy handling transactions
@@ -194,22 +201,26 @@ pub(crate) fn push_work(
// the kernel driver soon and pick up this work.
// 2. Threads are using the (e)poll interface, in which case
// they may be blocked on the waitqueue without having been
- // added to waiting_threads. For this case, we just iterate
- // over all threads not handling transaction work, and
- // wake them all up. We wake all because we don't know whether
- // a thread that called into (e)poll is handling non-binder
- // work currently.
+ // added to waiting_threads. For this case, we wake it up
+ // directly.
self.work.push_back(work);
// Wake up polling threads, if any.
- for thread in self.threads.values() {
- thread.notify_if_poll_ready(sync);
- }
+ self.notify_poll(sync);
Ok(())
}
}
+ pub(crate) fn notify_poll(&self, sync: bool) {
+ if let Some(poll) = &self.poll {
+ if sync {
+ poll.notify_sync();
+ }
+ poll.notify_all();
+ }
+ }
+
pub(crate) fn remove_node(&mut self, ptr: u64) {
self.nodes.remove(&ptr);
}
@@ -1322,11 +1333,15 @@ fn deferred_flush(&self) {
fn deferred_release(self: Arc<Self>) {
let is_manager = {
+ // These variables contain values to be dropped after releasing spinlock.
+ let _poll;
+
let mut inner = self.inner.lock();
inner.is_dead = true;
inner.is_frozen = IsFrozen::No;
inner.sync_recv = false;
inner.async_recv = false;
+ _poll = inner.poll.take();
inner.is_manager
};
@@ -1696,7 +1711,19 @@ pub(crate) fn poll(
table: PollTable<'_>,
) -> Result<u32> {
let thread = this.get_current_thread()?;
- let (from_proc, mut mask) = thread.poll(file, table);
+ {
+ let mut inner = this.inner.lock();
+ let poll = if let Some(poll) = &inner.poll {
+ &**poll
+ } else {
+ drop(inner);
+ let poll = PollCondVarBox::new(c"Process::poll", kernel::static_lock_class!())?;
+ inner = this.inner.lock();
+ &**inner.poll.get_or_insert(poll)
+ };
+ table.register_wait(file, poll);
+ }
+ let (from_proc, mut mask) = thread.poll()?;
if mask == 0 && from_proc && !this.inner.lock().work.is_empty() {
mask |= bindings::POLLIN;
}
diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index 97d5f31e8fe3..425c50298706 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -9,15 +9,14 @@
use kernel::{
bindings,
- fs::{File, LocalFile},
+ fs::LocalFile,
list::{AtomicTracker, List, ListArc, ListLinks, TryNewListArc},
prelude::*,
security,
seq_file::SeqFile,
seq_print,
sync::atomic::{ordering::Relaxed, Atomic},
- sync::poll::{PollCondVar, PollTable},
- sync::{aref::ARef, Arc, SpinLock},
+ sync::{aref::ARef, Arc, CondVar, SpinLock},
task::Task,
uaccess::{UserPtr, UserSlice, UserSliceReader},
uapi,
@@ -225,8 +224,10 @@ fn claim_next(&mut self, size: usize) -> Result<usize> {
}
}
+#[must_use]
pub(crate) enum PushWorkRes {
Ok,
+ OkNotifyPoll,
FailedDead(DLArc<dyn DeliverToRead>),
}
@@ -234,6 +235,7 @@ impl PushWorkRes {
fn is_ok(&self) -> bool {
match self {
PushWorkRes::Ok => true,
+ PushWorkRes::OkNotifyPoll => true,
PushWorkRes::FailedDead(_) => false,
}
}
@@ -310,27 +312,32 @@ fn pop_work(&mut self) -> Option<DLArc<dyn DeliverToRead>> {
fn push_work(&mut self, work: DLArc<dyn DeliverToRead>) -> PushWorkRes {
if self.is_dead {
- PushWorkRes::FailedDead(work)
+ return PushWorkRes::FailedDead(work);
+ }
+ self.work_list.push_back(work);
+ self.process_work_list = true;
+ if self.looper_flags & LOOPER_POLL != 0 {
+ PushWorkRes::OkNotifyPoll
} else {
- self.work_list.push_back(work);
- self.process_work_list = true;
PushWorkRes::Ok
}
}
- fn push_reply_work(&mut self, code: u32) {
+ fn push_reply_work(&mut self, code: u32) -> PushWorkRes {
if let Ok(work) = ListArc::try_from_arc(self.reply_work.clone()) {
work.set_error_code(code);
- self.push_work(work);
+ self.push_work(work)
} else {
pr_warn!("Thread reply work is already in use.");
+ PushWorkRes::Ok
}
}
fn push_return_work(&mut self, reply: u32) {
if let Ok(work) = ListArc::try_from_arc(self.return_work.clone()) {
work.set_error_code(reply);
- self.push_work(work);
+ // Not notifying: Reply to current thread.
+ let _ = self.push_work(work);
} else {
pr_warn!("Thread return work is already in use.");
}
@@ -422,7 +429,7 @@ pub(crate) struct Thread {
#[pin]
inner: SpinLock<InnerThread>,
#[pin]
- work_condvar: PollCondVar,
+ work_condvar: CondVar,
/// Used to insert this thread into the process' `ready_threads` list.
///
/// INVARIANT: May never be used for any other list than the `self.process.ready_threads`.
@@ -453,7 +460,7 @@ pub(crate) fn new(id: i32, process: Arc<Process>) -> Result<Arc<Self>> {
process,
task: ARef::from(&**kernel::current!()),
inner <- kernel::new_spinlock!(inner, "Thread::inner"),
- work_condvar <- kernel::new_poll_condvar!("Thread::work_condvar"),
+ work_condvar <- kernel::new_condvar!("Thread::work_condvar"),
links <- ListLinks::new(),
links_track <- AtomicTracker::new(),
}),
@@ -617,7 +624,14 @@ fn get_work(self: &Arc<Self>, wait: bool) -> Result<Option<DLArc<dyn DeliverToRe
/// Returns whether the item was successfully pushed. This can only fail if the thread is dead.
pub(crate) fn push_work(&self, work: DLArc<dyn DeliverToRead>) -> PushWorkRes {
let sync = work.should_sync_wakeup();
+ self.push_work_inner(work, sync)
+ }
+ pub(crate) fn push_work_inner(
+ &self,
+ work: DLArc<dyn DeliverToRead>,
+ sync: bool,
+ ) -> PushWorkRes {
let res = self.inner.lock().push_work(work);
if res.is_ok() {
@@ -636,7 +650,8 @@ pub(crate) fn push_work(&self, work: DLArc<dyn DeliverToRead>) -> PushWorkRes {
pub(crate) fn push_work_if_looper(&self, work: DLArc<dyn DeliverToRead>) -> BinderResult {
let mut inner = self.inner.lock();
if inner.is_looper() && !inner.is_dead {
- inner.push_work(work);
+ // Not notifying: Reply to current thread.
+ let _ = inner.push_work(work);
Ok(())
} else {
drop(inner);
@@ -1142,7 +1157,7 @@ fn deliver_single_reply(
transaction.set_outstanding(&mut self.process.inner.lock());
}
- {
+ let ret = {
let mut inner = self.inner.lock();
if !inner.pop_transaction_replied(transaction) {
return false;
@@ -1153,15 +1168,16 @@ fn deliver_single_reply(
}
match reply {
- Ok(work) => {
- inner.push_work(work);
- }
+ Ok(work) => inner.push_work(work),
Err(code) => inner.push_reply_work(code),
}
- }
+ };
// Notify the thread now that we've released the inner lock.
self.work_condvar.notify_sync();
+ if matches!(ret, PushWorkRes::OkNotifyPoll) {
+ self.process.inner.lock().notify_poll(true);
+ }
false
}
@@ -1319,7 +1335,8 @@ fn reply_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResult {
let process = orig.from.process.clone();
let allow_fds = orig.flags & TF_ACCEPT_FDS != 0;
let reply = Transaction::new_reply(self, process, info, allow_fds)?;
- self.inner.lock().push_work(completion);
+ // Not notifying: Reply to current thread.
+ let _ = self.inner.lock().push_work(completion);
orig.from.deliver_reply(Ok(reply), &orig);
Ok(())
})()
@@ -1351,7 +1368,8 @@ fn oneway_transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> Bin
};
let list_completion = DTRWrap::arc_try_new(DeliverCode::new(code))?;
let completion = list_completion.clone_arc();
- self.inner.lock().push_work(list_completion);
+ // Not notifying: Reply to current thread.
+ let _ = self.inner.lock().push_work(list_completion);
match transaction.submit(info) {
Ok(()) => Ok(()),
Err(err) => {
@@ -1551,10 +1569,9 @@ pub(crate) fn write_read(self: &Arc<Self>, data: UserSlice, wait: bool) -> Resul
ret
}
- pub(crate) fn poll(&self, file: &File, table: PollTable<'_>) -> (bool, u32) {
- table.register_wait(file, &self.work_condvar);
+ pub(crate) fn poll(&self) -> Result<(bool, u32)> {
let mut inner = self.inner.lock();
- (inner.should_use_process_work_queue(), inner.poll())
+ Ok((inner.should_use_process_work_queue(), inner.poll()))
}
/// Make the call to `get_work` or `get_work_local` return immediately, if any.
@@ -1571,26 +1588,9 @@ pub(crate) fn exit_looper(&self) {
}
}
- pub(crate) fn notify_if_poll_ready(&self, sync: bool) {
- // Determine if we need to notify. This requires the lock.
- let inner = self.inner.lock();
- let notify = inner.looper_flags & LOOPER_POLL != 0 && inner.should_use_process_work_queue();
- drop(inner);
-
- // Now that the lock is no longer held, notify the waiters if we have to.
- if notify {
- if sync {
- self.work_condvar.notify_sync();
- } else {
- self.work_condvar.notify_one();
- }
- }
- }
-
pub(crate) fn release(self: &Arc<Self>) {
self.inner.lock().is_dead = true;
- //self.work_condvar.clear();
self.unwind_transaction_stack();
// Cancel all pending work items.
diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
index 47d5e4d88b07..3c9620da3d25 100644
--- a/drivers/android/binder/transaction.rs
+++ b/drivers/android/binder/transaction.rs
@@ -322,6 +322,10 @@ pub(crate) fn submit(self: DLArc<Self>, info: &mut TransactionInfo) -> BinderRes
crate::trace::trace_transaction(false, &self, Some(&thread.task));
match thread.push_work(self) {
PushWorkRes::Ok => Ok(()),
+ PushWorkRes::OkNotifyPoll => {
+ process_inner.notify_poll(true);
+ Ok(())
+ }
PushWorkRes::FailedDead(me) => Err((BinderError::new_dead(), me)),
}
} else {
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread