Re: IORING_SEND_NOTIF_USER_DATA (was Re: IORING_CQE_F_COPIED)

[Stefan Metzmacher <metze@samba.org>]

Am 21.10.22 um 13:26 schrieb Pavel Begunkov:
> On 10/21/22 11:15, Stefan Metzmacher wrote:
>> Hi Pavel, and others...
>>
>>>> As far as I can see io_send_zc_prep has this:
>>>>
>>>>          if (unlikely(READ_ONCE(sqe->__pad2[0]) || READ_ONCE(sqe->addr3)))
>>>>                  return -EINVAL;
>>>>
>>>> both are u64...
>>>
>>> Hah, true, completely forgot about that one
>>
>> BTW: any comment on my "[RFC PATCH 0/8] cleanup struct io_uring_sqe layout"
>> thread, that would make it much easier to figure out which fields are used..., see
>> https://lore.kernel.org/io-uring/cover.1660291547.git.metze@samba.org/#r
> 
> I admit the sqe layout is messy as there is no good separation b/w
> common vs opcode specific fields, but it's not like the new layout
> makes it much simpler.

Really?

> E.g. looking who is using a field will get more complicated.

Why should anyone care what fields other opcodes use
and how they are named.

For legacy reasons we have to live with
struct io_uring_sqe_common common; in the middle.
apart from that each opcode should be free to use
5 u64 fields and 1 u32 field.

But e.g.

+               /* IORING_OP_FALLOCATE */
+               struct io_uring_sqe_fallocate {
+                       struct io_uring_sqe_hdr hdr;
+
+                       __u64   offset;
+                       __u64   length;
+                       __u32   mode;
+                       __u32   u32_ofs28;
+
+                       struct io_uring_sqe_common common;
+
+                       __u32   u32_ofs44;
+                       __u64   u64_ofs48;
+                       __u64   u64_ofs56;
+               } fallocate;

Immediately shows what's used and what not
and it avoids brain dead things like using
sqe->addr instead of sqe->len for the length.

And it makes it trivial to verify that the _prep function
rejects any unused field.

And it would it easier to write per opcode tracing code,
which can be easily analyzed.

> iow, no strong opinion on it.
> 
> btw, will be happy to have the include guard patch from one of
> your branches

This one from the io_uring_livepatch.v6.1 branch?
https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=3c36e05baad737f5cb896fdc9fc53dc1b74d2499

metze