Re: [PATCH for-next] io_uring: let to set a range for file slot allocation

[Hao Xu <hao.xu@linux.dev>]

On 6/29/22 20:18, Pavel Begunkov wrote:
> On 6/27/22 08:47, Hao Xu wrote:
>> On 6/25/22 18:55, Pavel Begunkov wrote:
>>>  From recently io_uring provides an option to allocate a file index for
>>> operation registering fixed files. However, it's utterly unusable with
>>> mixed approaches when for a part of files the userspace knows better
>>> where to place it, as it may race and users don't have any sane way to
>>> pick a slot and hoping it will not be taken.
>>
>> Exactly, with high frequency of index allocation from like multishot
>> accept, it's easy that user-pick requests fails.
>> By the way, just curious, I can't recall a reason that users pick a slot
>> rather than letting kernel do the decision, is there any? So I guess
> 
> Can't say for the initial design, but I prefer to give away control
> over such stuff to the userspace 1) to not over pollute the kernel
> (not relevant anymore), 2) because it has more knowledge and can
> use it more efficiently. E.g. to have O(1) memory and search time
> by using in-place index based free slot list, when indexes can be
> contants, and so on.
> 

Yea, I knew. Leave it alone to userspace makes biggest flexibility.

> 
>> users may use all the indexes as 'file slot allocation' range. Correct
>> me if I miss something.
> 
> Yeah, can be enough, and that's what the range is set to by default.
> 
>>> Let the userspace to register a range of fixed file slots in which the
>>> auto-allocation happens. The use case is splittting the fixed table in
>>> two parts, where on of them is used for auto-allocation and another for
>>> slot-specified operations.
>>>
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>> ---
> [...]
>>> @@ -24,11 +24,10 @@ static int io_file_bitmap_get(struct io_ring_ctx 
>>> *ctx)
>>>           if (ret != nr)
>>>               return ret;
>>> -        if (!table->alloc_hint)
>>> +        if (table->alloc_hint == ctx->file_alloc_start)
>>>               break;
>>> -
>>>           nr = table->alloc_hint;
>>> -        table->alloc_hint = 0;
>>> +        table->alloc_hint = ctx->file_alloc_start;
>>
>> should we use io_reset_alloc_hint() ?
> 
> We could but I'd rather prefer not. It's used just to anything valid
> within the range, while in io_file_bitmap_get() it's specifically to
> wrap around.
>