Re: [PATCH] io_uring: add io_uring_enter(2) fixed file support

[Jens Axboe <axboe@kernel.dk>]

On 3/3/22 6:38 AM, Jens Axboe wrote:
> On 3/2/22 10:28 PM, Xiaoguang Wang wrote:
>> IORING_REGISTER_FILES is a good feature to reduce fget/fput overhead for
>> each IO we do on file, but still left one, which is io_uring_enter(2).
>> In io_uring_enter(2), it still fget/fput io_ring fd. I have observed
>> this overhead in some our internal oroutine implementations based on
>> io_uring with low submit batch. To totally remove fget/fput overhead in
>> io_uring, we may add a small struct file cache in io_uring_task and add
>> a new IORING_ENTER_FIXED_FILE flag. Currently the capacity of this file
>> cache is 16, wihcih I think it maybe enough, also not that this cache is
>> per-thread.
> 
> Would indeed be nice to get rid of, can be a substantial amount of time
> wasted in fdget/fdput. Does this resolve dependencies correctly if
> someone passes the ring fd? Adding ring registration to test/ring-leak.c
> from the liburing repo would be a useful exercise.

Seems to pass that fine, but I did miss on first read through that you
add that hook to files_cancel() which should break that dependency.

Since I think this is a potentially big win for certain workloads, maybe
we should consider making this easier to use? I don't think we
necessarily need to tie this to the regular file registration. What if
we instead added a SETUP flag for this, and just return the internal
offset for that case? Then we don't need an enter flag, we don't need to
add register/unregister opcodes for it.

This does pose a problem when we fill the array. We can easily go beyond
16 here, that's just an arbitrary limit, but at some point we do have to
handle the case where SETUP_REGISTERED (or whatever we call it) can't
get a slot. I think we just clear the flag and setup the fd normally in
that case. The user doesn't need to know, all the application needs to
are about is that it can use the passed back 'fd' to call the other
io_uring functions.

The only potential oddity here is that the fd passed back is not a
legitimate fd. io_uring does support poll(2) on its file descriptor, so
that could cause some confusion even if I don't think anyone actually
does poll(2) on io_uring.

What do you think?

-- 
Jens Axboe