Re: [PATCH v2] rust_binder: add TF_DEFER_COMPLETE flag for avoiding userspace roundtrip

On Mon, Aug 10, 2026 at 11:45:32PM +0000, Carlos Llamas wrote:
> On Wed, Jul 22, 2026 at 09:09:22PM +0000, Alice Ryhl wrote:
> > Userspace only actually specifies TF_DEFER_COMPLETE when the Parcel does
> > not contain fds or refcounts on binder objects. This is because
> > otherwise said fd or binder node will not be freed until the binder
> > thread receives another incoming transaction, which could be a long
> > time. In the case of fds, this is especially important because delaying
> > fclose() can result in processes hanging because they read from a pipe
> > that isn't being closed due to fclose() not getting called. Note that
> > even if TF_DEFER_COMPLETE is not specified for this transaction, it can
> > still be useful to defer the BC_REPLY command, as it can still avoid a
> > userspace roundtrip when a new incoming transaction is available right
> > away.
> 
> The processing of a deferred COMPLETE doesn't change right? It doesn't
> matter if the kernel rejects / ignores the new flag, userspace will
> still follow the same path. 100% backward-compatible then.

Yes. In fact, if userspace passes the flag to a kernel without support
for it, the only consequence is worse perf (extra userspace roundtrips).
It will still work correctly.


> > +            // Note that if the thread list is empty, then the call to `pop_work()` has changed
> > +            // `process_work_list` back to `false` even if we set it to `true` above.
> > +            thread_has_deferred_work = inner.process_work_list;
> 
> I might be getting this wrong, but for the new TF_DEFER_COMPLETE case,
> we have !process_work_list and !work_list.is_empty(). Then pop_work()
> does not touch process_work_list because it's already false.
> 
> So we set thread_has_deferred_work to false? Maybe this was meant to be:
> 	thread_has_deferred_work = !inner.work_list.is_empty()

You're right.

> > +            {
> > +                // This performs a deferred push so that `read` can wait for the next incoming
> > +                // transaction without a userspace roundtrip.
> > +                let mut inner = self.inner.lock();
> > +                inner.push_work_deferred(completion);
> > +                // However, if `TF_DEFER_COMPLETE` is not set, then set `process_work_list` to make
> > +                // the push non-deferred. This forces a userspace roundtrip.
> > +                inner.process_work_list |= info.flags & TF_DEFER_COMPLETE == 0;
> 
> If there is already a push_work() and a push_work_deferred() why use
> process_work_list directly? Is it to avoid an if/else?

I guess so ...

Alice