> +enum io_uring_sqe_ext_cap_bits {
> + EXT_CAP_PI_BIT,
> + /*
> + * not a real extended capability; just to make sure that we don't
> + * overflow
> + */
> + EXT_CAP_LAST_BIT,
> +};
> +
> +/* extended capability flags */
> +#define EXT_CAP_PI (1U << EXT_CAP_PI_BIT)
This is getting into nitpicking, but is the a good reason to have that
enum, which is never used as a type and the values or only defined to
actually define the bit positions below? That's a bit confusing to
me.
Also please document the ABI for EXT_CAP_PI, right now this is again
entirely undocumented.
> +/* Second half of SQE128 for IORING_OP_READ/WRITE */
> +struct io_uring_sqe_ext {
> + __u64 rsvd0[4];
> + /* if sqe->ext_cap is EXT_CAP_PI, last 32 bytes are for PI */
> + union {
> + __u64 rsvd1[4];
> + struct {
> + __u16 flags;
> + __u16 app_tag;
> + __u32 len;
> + __u64 addr;
> + __u64 seed;
> + __u64 rsvd;
> + } rw_pi;
> + };
And this is not what I though we discussed before. By having a
union here you imply some kind of "type" again that is switched
on a value, and not flags indication the presence of potential
multiple optional and combinable features. This is what I would
have expected here based on the previous discussion:
struct io_uring_sqe_ext {
/*
* Reservered for please tell me what and why it is in the beginning
* and not the end:
*/
__u64 rsvd0[4];
/*
* Only valid when EXT_CAP_PI is set:
*/
__u16 pi_flags; /* or make this generic flags, dunno? */
__u16 app_tag;
__u32 pi_len;
__u64 pi_addr;
__u64 pi_seed;
__u64 rsvd1;
};
