Re: [PATCH 5/7] io_uring: add ability for provided buffer to index registered buffers

[Jens Axboe <axboe@kernel.dk>]

On 10/24/24 9:44 AM, Pavel Begunkov wrote:
> On 10/23/24 17:07, Jens Axboe wrote:
>> This just adds the necessary shifts that define what a provided buffer
>> that is merely an index into a registered buffer looks like. A provided
>> buffer looks like the following:
>>
>> struct io_uring_buf {
>>     __u64    addr;
>>     __u32    len;
>>     __u16    bid;
>>     __u16    resv;
>> };
>>
>> where 'addr' holds a userspace address, 'len' is the length of the
>> buffer, and 'bid' is the buffer ID identifying the buffer. This works
>> fine for a virtual address, but it cannot be used efficiently denote
>> a registered buffer. Registered buffers are pre-mapped into the kernel
>> for more efficient IO, avoiding a get_user_pages() and page(s) inc+dec,
>> and are used for things like O_DIRECT on storage and zero copy send.
>>
>> Particularly for the send case, it'd be useful to support a mix of
>> provided and registered buffers. This enables the use of using a
>> provided ring buffer to serialize sends, and also enables the use of
>> send bundles, where a send can pick multiple buffers and send them all
>> at once.
>>
>> If provided buffers are used as an index into registered buffers, the
>> meaning of buf->addr changes. If registered buffer index 'regbuf_index'
>> is desired, with a length of 'len' and the offset 'regbuf_offset' from
>> the start of the buffer, then the application would fill out the entry
>> as follows:
>>
>> buf->addr = ((__u64) regbuf_offset << IOU_BUF_OFFSET_BITS) | regbuf_index;
>> buf->len = len;
>>
>> and otherwise add it to the buffer ring as usual. The kernel will then
>> first pick a buffer from the desired buffer group ID, and then decode
>> which registered buffer to use for the transfer.
>>
>> This provides a way to use both registered and provided buffers at the
>> same time.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>   include/uapi/linux/io_uring.h | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>> index 86cb385fe0b5..eef88d570cb4 100644
>> --- a/include/uapi/linux/io_uring.h
>> +++ b/include/uapi/linux/io_uring.h
>> @@ -733,6 +733,14 @@ struct io_uring_buf_ring {
>>       };
>>   };
>>   +/*
>> + * When provided buffers are used as indices into registered buffers, the
>> + * lower IOU_BUF_REGBUF_BITS indicate the index into the registered buffers,
>> + * and the upper IOU_BUF_OFFSET_BITS indicate the offset into that buffer.
>> + */
>> +#define IOU_BUF_REGBUF_BITS    (32ULL)
>> +#define IOU_BUF_OFFSET_BITS    (32ULL)
> 
> 32 bit is fine for IO size but not enough to store offsets, it
> can only address under 4GB registered buffers.

I did think about that - at least as it stands, registered buffers are
limited to 1GB in size. That's how it's been since that got added. Now,
for the future, we may obviously lift that limitation, and yeah then
32-bits would not necessarily be enough for the offset.

For linux, the max read/write value has always been INT_MAX & PAGE_MASK,
so we could make do with 31 bits for the size, which would bump the
offset to 33-bits, or 8G. That'd leave enough room for, at least, 8G
buffers, or 8x what we support now. Which is probably fine, you'd just
split your buffer registrations into 8G chunks, if you want to register
more than 8G of memory.

-- 
Jens Axboe