Re: [PATCH v6 2/2] rust_binder: move (e)poll wait queue to Process
From: Boqun Feng
Date: Sat Jul 11 2026 - 09:37:22 EST
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`.
> 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@xxxxxxxxxx/
>
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?
Regards,
Boqun
> > The rest looks good to me. FWIW,
> >
> > Reviewed-by: Boqun Feng <boqun@xxxxxxxxxx>
>
> Thanks!
>
> Alice