rust_binder: move (e)poll wait queue to Process

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>
Reviewed-by: Boqun Feng <boqun@kernel.org>
Link: https://patch.msgid.link/20260707-upgrade-poll-v6-2-4b8fae7bf1d9@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Alice Ryhl
2026-07-07 10:43:13 +00:00
committed by Greg Kroah-Hartman
parent e5e86df8b6
commit dbb17c9ea7
4 changed files with 95 additions and 60 deletions

View File

@@ -538,7 +538,7 @@ pub(crate) fn submit_oneway(
inner.oneway_todo.push_back(transaction);
} else {
inner.has_oneway_transaction = true;
guard.push_work(transaction)?;
guard.push_work(&self.owner, transaction)?;
}
Ok(())
}
@@ -570,7 +570,7 @@ pub(crate) fn pending_oneway_finished(&self) {
let transaction = inner.oneway_todo.pop_front();
inner.has_oneway_transaction = transaction.is_some();
if let Some(transaction) = transaction {
match guard.push_work(transaction) {
match guard.push_work(&self.owner, transaction) {
Ok(()) => {}
Err((_err, work)) => {
// Process is dead.

View File

@@ -30,7 +30,8 @@
sync::{
aref::ARef,
lock::{spinlock::SpinLockBackend, Guard},
Arc, ArcBorrow, CondVar, CondVarTimeoutResult, SpinLock, UniqueArc,
poll::PollCondVarBox,
Arc, ArcBorrow, CondVar, CondVarTimeoutResult, SetOnce, SpinLock, UniqueArc,
},
task::{Pid, Task},
uaccess::{UserSlice, UserSliceReader},
@@ -172,21 +173,26 @@ fn new() -> Self {
/// taken while holding the inner process lock.
pub(crate) fn push_work(
&mut self,
proc: &Process,
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 => {
proc.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,17 +200,12 @@ 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);
}
proc.notify_poll(sync);
Ok(())
}
@@ -227,11 +228,11 @@ pub(crate) fn update_node_refcount(
// If we decided that we need to push work, push either to the process or to a thread if
// one is specified.
if let Some(node) = push {
if let Some(pnode) = push {
if let Some(thread) = othread {
thread.push_work_deferred(node);
thread.push_work_deferred(pnode);
} else {
let _ = self.push_work(node);
let _ = self.push_work(&node.owner, pnode);
// Nothing to do: `push_work` may fail if the process is dead, but that's ok as in
// that case, it doesn't care about the notification.
}
@@ -457,6 +458,12 @@ pub(crate) struct Process {
#[pin]
node_refs: SpinLock<ProcessNodeRefs>,
// Synchronizes `register_wait` calls to the `PollCondVarBox`.
//
// The `PollCondVarBox` is not stored here because synchronization is
// done for `register_wait` only. Wakeups do not take this lock.
poll: SetOnce<PollCondVarBox>,
// Work node for deferred work item.
#[pin]
defer_work: Work<Process>,
@@ -516,6 +523,7 @@ fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
defer_work <- kernel::new_work!("Process::defer_work"),
links <- ListLinks::new(),
stats: BinderStats::new(),
poll: SetOnce::new(),
}),
GFP_KERNEL,
)?;
@@ -715,7 +723,7 @@ fn get_current_thread(self: ArcBorrow<'_, Self>) -> Result<Arc<Thread>> {
pub(crate) fn push_work(&self, work: DLArc<dyn DeliverToRead>) -> BinderResult {
// If push_work fails, drop the work item outside the lock.
let res = self.inner.lock().push_work(work);
let res = self.inner.lock().push_work(self, work);
match res {
Ok(()) => Ok(()),
Err((err, work)) => {
@@ -1024,7 +1032,7 @@ pub(crate) fn inc_ref_done(&self, reader: &mut UserSliceReader, strong: bool) ->
if let Ok(Some(node)) = inner.get_existing_node(ptr, cookie) {
if let Some(node) = node.inc_ref_done_locked(strong, &mut inner) {
// This only fails if the process is dead.
let _ = inner.push_work(node);
let _ = inner.push_work(self, node);
}
}
Ok(())
@@ -1573,6 +1581,15 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
}
}
}
pub(crate) fn notify_poll(&self, sync: bool) {
if let Some(poll) = self.poll.as_ref() {
if sync {
poll.notify_sync();
}
poll.notify_all();
}
}
}
fn get_frozen_status(data: UserSlice) -> Result {
@@ -1766,7 +1783,21 @@ pub(crate) fn poll(
table: PollTable<'_>,
) -> Result<u32> {
let thread = this.get_current_thread()?;
let (from_proc, mut mask) = thread.poll(file, table);
{
let poll = loop {
if let Some(poll) = this.poll.as_ref() {
break poll;
}
let poll = PollCondVarBox::new(c"Process::poll", kernel::static_lock_class!())?;
// Reuse our existing lock to synchronize callers initializing.
let _guard = this.node_refs.lock();
this.poll.populate(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;
}

View File

@@ -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(),
}),
@@ -624,7 +631,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() {
@@ -643,7 +657,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);
@@ -1160,7 +1175,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;
@@ -1177,15 +1192,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.notify_poll(true);
}
false
}
@@ -1382,7 +1398,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, None);
Ok(())
})()
@@ -1421,7 +1438,8 @@ fn oneway_transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> Bin
let list_completion =
DTRWrap::arc_try_new(DeliverCode::new(code, self.process.task.pid()))?;
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) => {
@@ -1623,10 +1641,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.
@@ -1643,26 +1660,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.

View File

@@ -371,11 +371,15 @@ 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.notify_poll(true);
Ok(())
}
PushWorkRes::FailedDead(me) => Err((BinderError::new_dead(), me)),
}
} else {
crate::trace::trace_transaction(false, &self, None);
process_inner.push_work(self)
process_inner.push_work(&process, self)
};
drop(process_inner);