On 10/24/24 1:53 PM, Jens Axboe wrote:
> On 10/24/24 12:20 PM, Pavel Begunkov wrote:
>> On 10/24/24 18:16, Jens Axboe wrote:
>>> On 10/24/24 10:17 AM, Pavel Begunkov wrote:
>>>> On 10/24/24 16:57, Jens Axboe wrote:
>>>>> 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@xxxxxxxxx>
>>>>>>> ---
>>>>>>> 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.
>>>>
>>>> Right, and I don't think it's unreasonable considering with how
>>>> much memory systems have nowadays, and we think that one large
>>>> registered buffer is a good thing.
>>>
>>> Agree - but at the same time, not a big hardship to chunk up the region
>>> into 8G chunks rather than allow, eg, a 64G region. Would be nice if it
>>> wasn't a requirement, but unsure how to make that work otherwise.
>>>
>>> And not a lot of complaints on having 1G be the size, even from the
>>> varnish side where they register hundreds of gigs of memory.
>>>
>>>>> 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.
>>>>
>>>> That's why I mentioned IO size, you can register a very large buffer
>>>> and do IO with a small subchunk of it, even if that "small" is 4G,
>>>> but it still needs to be addressed. I think we need at least an order
>>>> of magnitude or two more space for it to last for a bit.
>>>>
>>>> Can it steal bits from IOU_BUF_REGBUF_BITS?
>>>
>>> As mentooned, we can definitely move the one bit, which would bring is
>>> to 31/33, and ~2GB IO size (max in linux) and ~8G of offset. That can be
>>> done without having any tradeoffs. Beyond that, and we're starting to
>>> limit the transfer size, eg it's tradeoff of allowing more offset (and
>>> hence bigger registered regions) vs transfer size. You could probably
>>> argue that 1G would be fine, and hence make it 30/34, which would allow
>>> 16GB registered buffers. Just unsure if it's worth it, as neither of
>>> those would allow really huge registered buffers - and does it matter if
>>> your buffer registrations are chunked at 8G vs 16G? Probably not.
>>
>> 6/7 packs offset and the reg buffer index into the same u64,
>> not the len. I'm don't see how it affects the len
>>
>> idx = addr & ((1ULL << IOU_BUF_REGBUF_BITS) - 1);
>> addr >>= IOU_BUF_REGBUF_BITS;
>> *offset = addr & ((1ULL << IOU_BUF_OFFSET_BITS) - 1);
>>
>> So the tradeoff is with the max size of the registered
>> buffer table. I doubt you need 2^32, if each is at least
>> 4KB, it's at least 16TB.
>
> Ah yes, nevermind, the len is of course separate. Yes indeed that falls
> out nicely then, we can just reserve eg 16 bits for registered buffers.
> And that leaves 48 bits for the offset, which would hold more than
> enough. Even at that and 8G max buffer size, that'd be half a TB of
> buffer space. And there's no reason we can't just increase the
> registered buffer size on 64-bit to much beyond that. Maybe play it safe
> and set aside 20 bits for the buffer index, and that leaves 44 bits for
> the registered buffer space? Or 16TB of registered buffer space.
>
> What do you think?
I changed it to 20/44 as that seems to be the best split. It allows much
larger registered buffers, and 1M of them. That should be plenty.
Might even make sense to just up the registered buffer size at this
point too as a separate change, there's no reason for it to be limited
to 1G. We can keep 1G on 32-bit as it could only go to to 4G-1 on that
anyway. But for 64-bit, we can bump it higher.
--
Jens Axboe
