Re: [PATCHv5 1/1] io_uring: add support for IORING_SETUP_SQE_MIXED

[Keith Busch <kbusch@kernel.org>]

On Tue, Oct 14, 2025 at 08:03:42PM -0600, Keith Busch wrote:
> On Tue, Oct 14, 2025 at 03:33:19PM -0700, Caleb Sander Mateos wrote:
> > On Mon, Oct 13, 2025 at 11:00 AM Keith Busch <kbusch@meta.com> wrote:
> > > +               /*
> > > +                * A 128b op on a non-128b SQ requires mixed SQE support as
> > > +                * well as 2 contiguous entries.
> > > +                */
> > > +               if (!(ctx->flags & IORING_SETUP_SQE_MIXED) || *left < 2 ||
> > > +                   !(ctx->cached_sq_head & (ctx->sq_entries - 1)))
> > > +                       return io_init_fail_req(req, -EINVAL);
> > > +               /*
> > > +                * A 128b operation on a mixed SQ uses two entries, so we have
> > > +                * to increment the head and decrement what's left.
> > > +                */
> > > +               ctx->cached_sq_head++;
> > > +               (*left)--;
> > 
> > Hmm, io_submit_sqes() calls io_get_task_refs() at the start to
> > decrement cached_refs by the number of SQEs (counting 128-byte SQEs
> > twice) but io_put_task() only increments it once for each completed
> > request (counting 128-byte SQEs once). Does that mean there's a
> > refcount leak? Perhaps io_submit_sqes() or this block needs to
> > increment cached_refs to account for each 128-byte SQE?
> 
> It looks like you're right. I think the increment option is the easiest
> way to deal with it, just adding this line into the above:
> 
> + 	current->io_uring->cached_refs++;
> 
> I'm going to take a moment to figure out a good way to test this because
> I don't think I'm hitting any problem with the admittedly simple tests
> I've offered to liburing, so I may be missing something.

So the tests were in fact causing missing putting some usage references,
but I'm not sure how to check for such leakage. Everything ends up
clearing up once the ring closes, and there's no other visibility into
the refcount from user space. I had to add some trace_printks just to
verify it. The increment above gets everything back to normal, at least,
so will send a new version with that in.