Re: [PATCH v11 06/10] io_uring: introduce attributes for read/write and PI support

[Pavel Begunkov <asml.silence@gmail.com>]

On 12/5/24 18:04, Keith Busch wrote:
...
>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>> index aac9a4f8fa9a..38f0d6b10eaf 100644
>> --- a/include/uapi/linux/io_uring.h
>> +++ b/include/uapi/linux/io_uring.h
>> @@ -98,6 +98,10 @@ struct io_uring_sqe {
>>   			__u64	addr3;
>>   			__u64	__pad2[1];
>>   		};
>> +		struct {
>> +			__u64	attr_ptr; /* pointer to attribute information */
>> +			__u64	attr_type_mask; /* bit mask of attributes */
>> +		};
> 
> I can't say I'm a fan of how this turned out. I'm merging up the write
> hint stuff, and these new fields occupy where that 16-bit value was
> initially going. Okay, so I guess I need to just add a new attribute
> flag? That might work if I am only appending exactly one extra attribute
> per SQE, but what if I need both PI and Write Hint? Do the attributes
> need to appear in a strict order?

Martin put it well, this version requires attributes to be placed
in their id order, but FWIW without any holes, i.e. the following
is fine:

ATTR_PI = 1
ATTR_WRITE_STREAM = 2

strcut attr_stream attr = { ... };
sqe->attr_mask = ATTR_WRITE_HINT;
sqe->attr_ptr = &attr;

In case of multiple attributes:

struct compound_attr {
	struct attr_pi a;
	struct attr_stream b;
} attr = { ... };

sqe->attr_mask = ATTR_WRITE_HINT | ATTR_PI;
sqe->attr_ptr = &attr;

The other option for the uapi, and Martin mentioned it as well, is
to add a type field to all attributes.

FWIW, I think the space aliased with sqe->splice_fd_in is not
used by read/write, but I haven't looked into streams to get
an opinion on which way is more appropriate.

-- 
Pavel Begunkov