Re: [RFC] io_uring: remove file batch-get optimisation

[Jens Axboe <axboe@kernel.dk>]

On 8/10/21 11:11 AM, Pavel Begunkov wrote:
> On 8/10/21 6:04 PM, Jens Axboe wrote:
>> On 8/10/21 7:52 AM, Pavel Begunkov wrote:
>>> For requests with non-fixed files, instead of grabbing just one
>>> reference, we get by the number of left requests, so the following
>>> requests using the same file can take it without atomics.
>>>
>>> However, it's not all win. If there is one request in the middle
>>> not using files or having a fixed file, we'll need to put back the left
>>> references. Even worse if an application submits requests dealing with
>>> different files, it will do a put for each new request, so doubling the
>>> number of atomics needed. Also, even if not used, it's still takes some
>>> cycles in the submission path.
>>>
>>> If a file used many times, it rather makes sense to pre-register it, if
>>> not, we may fall in the described pitfall. So, this optimisation is a
>>> matter of use case. Go with the simpliest code-wise way, remove it.
>>
>> I ran this through the peak testing, not using registered files. Doesn't
>> seem to make a real difference here, at least in the quick testing.
>> Which would seem to indicate we could safely kill it. But that's also
>> the best case for non-registered files, would be curious to see if it
>> makes a real difference now for workloads where the file is being
>> shared.
> 
> Do you mean shared between cores so there is contention? Or the worst
> case for non-reg with multiple files as described in the patch? 

The former. The latter is also a concern, but less so to me. That
said, I do think we should just do this. It's less code, and that's
always good. To justify it, numbers would have to back it up.

-- 
Jens Axboe