Re: [PATCH v6 2/2] rust_binder: move (e)poll wait queue to Process

[Boqun Feng <boqun@kernel.org>]

On Tue, Jul 07, 2026 at 10:43:13AM +0000, Alice Ryhl wrote:
> 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>
> ---
[...]
> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index 0555c4bd503e..f855d8d9818c 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
[...]
>              }
>          }
>      }
> +
> +    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 {
> @@ -1726,7 +1743,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();

Note sure whether this lock is needed? SetOnce::populate() should be
atomic, i.e. only one populate() would win?

Also seems we should have a SetOnce::as_ref_or_populate(&self, default:
T).

The rest looks good to me. FWIW,

Reviewed-by: Boqun Feng <boqun@kernel.org>

Regards,
Boqun

> +                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;
>          }
[..]