Re: [PATCHv11 03/10] io_uring: add write stream attribute

[Keith Busch <kbusch@kernel.org>]

On Fri, Dec 06, 2024 at 06:14:29PM +0530, Kanchan Joshi wrote:
> On 12/6/2024 7:23 AM, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > Adds a new attribute type to specify a write stream per-IO.
> > 
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
> >   include/uapi/linux/io_uring.h |  9 ++++++++-
> >   io_uring/rw.c                 | 28 +++++++++++++++++++++++++++-
> >   2 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> > index 5fa38467d6070..263cd57aae72d 100644
> > --- a/include/uapi/linux/io_uring.h
> > +++ b/include/uapi/linux/io_uring.h
> > @@ -123,7 +123,14 @@ struct io_uring_attr_pi {
> >   	__u64	rsvd;
> >   };
> >   
> > -#define IORING_RW_ATTR_FLAGS_SUPPORTED (IORING_RW_ATTR_FLAG_PI)
> > +#define IORING_RW_ATTR_FLAG_WRITE_STREAM (1U << 1)
> > +struct io_uring_write_stream {
> > +	__u16	write_stream;
> > +	__u8	rsvd[6];
> > +};
> 
> So this needs 8 bytes. Maybe passing just 'u16 write_stream' is better? 
> Or do you expect future additions here (to keep rsvd).

I don't have any plans to use it. It's just padded for alignment. I am
not sure what future attributes might be proposed, but I don't want to
force them be align to a 2-byte boundary.
 
> Optimization is possible (now or in future) if it's 4 bytes or smaller, 
> as that can be placed in SQE along with a new RW attribute flag that 
> says it's placed inline. Like this -

Oh, that's definitely preferred IMO, because it is that much easier to
reach the capability. Previous versions of this proposal had the field
in the next union, so I for some reason this union you're showing here
was unavailable for new fields, but it looks like it's unused for
read/write. So, yeah, let's put it in the sqe if there's no conflict
here.

> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -92,6 +92,10 @@ struct io_uring_sqe {
>                          __u16   addr_len;
>                          __u16   __pad3[1];
>                  };
> +               struct {
> +                       __u16   write_hint;
> +                       __u16   __rsvd[1];
> +               };
>          };
>          union {
>                  struct {