[RFC,0/8] cleanup struct io_uring_sqe layout

Message

Stefan Metzmacher Aug. 12, 2022, 8:34 a.m. UTC

Hi Jens,

with all the nested structs, unions and scalar fields in
struct io_uring_sqe, it becomes more and more confusing to
find which fields are used by what opcode for
what and which fields are useable for new opcodes.

I started with some patches to move to a model where we have
a signle top level union with structures for each opcode
(in reality per .prep() function).

At the same time we leave the existing legacy layout as is
and enforce fields are are of the same type and at the same offset.

I only started with the first few files, but in the end
the kernel should no longer use the legacy elements.

The new stuff would look like this:

struct io_uring_sqe {
    union {

        /* This is the legacy structure */
        struct {
                __u8    opcode;         /* type of operation for this sqe */
                __u8    flags;          /* IOSQE_ flags */
                __u16   ioprio;         /* ioprio for the request */
                __s32   fd;             /* file descriptor to do IO on */
                ...
        };

        struct { /* This is the structure showing the generic fields */
                struct io_uring_sqe_hdr {
                        __u8    opcode;         /* type of operation for this sqe */
                        __u8    flags;          /* IOSQE_ flags */
                        __u16   ioprio;         /* ioprio for the request */
                        __s32   fd;             /* file descriptor to do IO on */
                } __attribute__((packed)) hdr;

                __u64   u64_ofs08;
                __u64   u64_ofs16;
                __u32   u32_ofs24;
                __u32   u32_ofs28;

                struct io_uring_sqe_common {
                        __u64   user_data;      /* data to be passed back at completion time */
                        __u16   buf_info;       /* buf_index or buf_group */
                        __u16   personality;    /* the personality to run this request under */
                } __attribute__((packed)) common;

                __u32   u32_ofs44;
                __u64   u64_ofs48;
                __u64   u64_ofs56;
        };

        /* IORING_OP_{READV,WRITEV,READ_FIXED,WRITE_FIXED,READ,WRITE} */
        struct io_uring_sqe_rw {
                struct io_uring_sqe_hdr hdr;

                __u64   offset;
                __u64   buffer_addr;
                __u32   length;
                __u32   flags;

                struct io_uring_sqe_common common;

                __u32   u32_ofs44;
                __u64   u64_ofs48;
                __u64   u64_ofs56;
        } rw;

        ...
};

At least for me it would make things much easier to understand.
Whould that be something useful?
If so we could go that way for all prep() functions and
in the end also convert liburing.

Stefan Metzmacher (8):
  io_uring: move the current struct io_uring_sqe members to legacy sub
    struct
  io_uring: add a generic structure for struct io_uring_sqe
  io_uring: check legacy layout of struct io_uring_sqe with
    BUILD_BUG_SQE_LEGACY*
  io_uring: only make use generic struct io_uring_sqe elements for
    tracing
  io_uring: only access generic struct io_uring_sqe elements in
    io_uring.c
  io_uring: add BUILD_BUG_SQE_HDR_COMMON() macro
  io_uring: introduce struct io_uring_sqe_rw for all io_prep_rw() using
    opcodes
  io_uring: introduce struct io_uring_sqe_{fsync,sfr,fallocate}

 include/trace/events/io_uring.h |  30 ++---
 include/uapi/linux/io_uring.h   | 223 +++++++++++++++++++++++---------
 io_uring/io_uring.c             | 205 ++++++++++++++++++++++-------
 io_uring/rw.c                   |  26 +++-
 io_uring/sync.c                 |  58 ++++++---
 5 files changed, 394 insertions(+), 148 deletions(-)