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

[Alice Ryhl <aliceryhl@google.com>]

On Sat, Jul 11, 2026 at 06:37:14AM -0700, Boqun Feng wrote:
> On Sat, Jul 11, 2026 at 10:32:39AM +0000, Alice Ryhl wrote:
> > On Fri, Jul 10, 2026 at 05:30:30PM -0700, Boqun Feng wrote:
> > > On Tue, Jul 07, 2026 at 10:43:13AM +0000, Alice Ryhl wrote:
> > > > +        {
> > > > +            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);
> > > > +            };
> > > 
> > > 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).
> > 
> > I'm taking this lock because I want to ensure that losers only loop
> > once. The problem is that just because you lost the race in populate(),
> 
> Then probably you could add some comment explaining this. For example:
> 
>     // Reuse our existing lock to synchronize callers initializing to
>     // make sure in the next iteration `as_ref()` will return `Some`.

I don't mind adding the comment.

> > it's not guaranteed that as_ref() will return Some on the next
> > iteration, since the winner of the race may still be busy executing
> > populate(). Taking the loop avoids this possibility.
> > 
> > With regards to as_ref_or_populate(), I point you to this discussion for
> > reasons why this is hard:
> > https://lore.kernel.org/all/aZLZbN5C3wXgt3kL@google.com/
> > 
> 
> Seems to me you really want OnceLock behavior here, and other code may
> have the same requirement in the future. So maybe we just add OnceLock
> for it? If the space cost is the concern, we can add a 
> SetOnce::populate_with_lock(&self, lock: &Lock<..>) to provide users an
> option. I think it's better than open-code here.
> 
> Thoughts?

Well, I don't think it's a bad idea.

The current code drops the value under the spinlock if populate() loses
the race, and a populate_with_lock() could help avoid that, so that
seems like a reasonable idea. It's not a big problem in this case (it's
just a kfree_rcu() call after all), but could be a problem for other
SetOnce users.

I mean, it'd be ideal if SetOnce could just properly support this, but
like I discussed on the thread, there are a bunch of things to keep in
mind if we want to actually do that. You want preemption disabled while
you memcpy() in the value in populate(). I guess preemption doesn't
matter for as_ref(), though. Not sure how that would interact with
PREEMPT_RT.

Alice