Re: [PATCHv3 1/3] Add support IORING_SETUP_SQE_MIXED

[Caleb Sander Mateos <csander@purestorage.com>]

On Wed, Sep 24, 2025 at 1:30 PM Keith Busch <kbusch@kernel.org> wrote:
>
> On Wed, Sep 24, 2025 at 01:20:44PM -0700, Caleb Sander Mateos wrote:
> > > index 052d6b56..66f1b990 100644
> > > --- a/src/include/liburing.h
> > > +++ b/src/include/liburing.h
> > > @@ -575,6 +575,7 @@ IOURINGINLINE void io_uring_initialize_sqe(struct io_uring_sqe *sqe)
> > >         sqe->buf_index = 0;
> > >         sqe->personality = 0;
> > >         sqe->file_index = 0;
> > > +       sqe->addr2 = 0;
> >
> > Why is this necessary for mixed SQE size support? It looks like this
> > field is already initialized in io_uring_prep_rw() via the unioned off
> > field. Though, to be honest, I can't say I understand why the
> > initialization of the SQE fields is split between
> > io_uring_initialize_sqe() and io_uring_prep_rw().
>
> The nvme passthrough uring_cmd doesn't call io_uring_prep_rw(), so we'd
> just get a stale value in that field if we don't clear it. But you're
> right that many cases would end up setting the field twice when we don't
> need that.

Sure, that's a reasonable concern. Perhaps a helper for initializing a
NVMe passthru operation would make sense, though maybe it's difficult
to do that without requiring the linux/nvme_ioctl.h uapi header. But
regardless, it seems unrelated to the mixed SQE size work.

>
> > > +       IOSQE_SQE_128B_BIT,
> >
> > I thought we decided against using an SQE flag bit for this? Looks
> > like this needs to be re-synced with the kernel uapi header.
>
> We did, and this is a left over artifact that is not supposed to be
> here. :( Nothing is depending on the bit in this series.

Yeah I figured this file just needed to be updated with the current
version of the uapi header defined in your latest kernel patch.

Best,
Caleb