Re: [PATCH 0/3] io_uring: fixes for provided buffer ring

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

On 6/13/22 14:16, Dylan Yudaken wrote:
> On Mon, 2022-06-13 at 13:59 +0100, Pavel Begunkov wrote:
>> On 6/13/22 12:08, Hao Xu wrote:
>>> On 6/13/22 18:11, Dylan Yudaken wrote:
>>>> This fixes two problems in the new provided buffer ring feature.
>>>> One
>>>> is a simple arithmetic bug (I think this came out from a
>>>> refactor).
>>>> The other is due to type differences between head & tail, which
>>>> causes
>>>> it to sometimes reuse an old buffer incorrectly.
>>>>
>>>> Patch 1&2 fix bugs
>>>> Patch 3 limits the size of the ring as it's not
>>>> possible to address more entries with 16 bit head/tail
>>>
>>> Reviewed-by: Hao Xu <howeyxu@tencent.com>
>>>
>>>>
>>>> I will send test cases for liburing shortly.
>>>>
>>>> One question might be if we should change the type of
>>>> ring_entries
>>>> to uint16_t in struct io_uring_buf_reg?
>>>
>>> Why not? 5.19 is just rc2 now. So we can assume there is no users
>>> using
>>> it right now I think?
>>
>> It's fine to change, but might be better if we want to extend it
>> in the future. Do other pbuf bits allow more than 2^16 buffers?
>>

might be better to leave it u32 *

> I guess with
> 
> +	if (reg.ring_entries >= 65536)
> +		return -EINVAL;
> 
> it doesn't matter either way. we can always use those bits later if we
> need?

I think so as well.

I was also wondering whether pbufs can potentially allow >16 bits,
but taking a quick look IORING_CQE_BUFFER_SHIFT=16, so we only have
16 bits in cqe::flags for indexes we return.

-- 
Pavel Begunkov