Re: [PATCH] io_uring: add support for IORING_REGISTER_FILES_UPDATE

[Jeff Moyer <jmoyer@redhat.com>]

Jens Axboe <axboe@kernel.dk> writes:

> On 10/4/19 10:03 AM, Jeff Moyer wrote:
>> Jens Axboe <axboe@kernel.dk> writes:
>> 
>>> On 10/4/19 9:34 AM, Jeff Moyer wrote:
>>>> If I'm reading this (and the code) right, that means you can't add files
>>>> to a set.  Wouldn't that be a useful thing to do, instead of just
>>>> replacing existing ones?
>>>
>>> You can add files to a set, you just can't grow a set beyond the size
>>> you originally registered. I actually forgot to post the pre-patch for
>>> this, which is:
>>>
>>> http://git.kernel.dk/cgit/linux-block/commit/?h=for-5.5/io_uring&id=fb3e60f87aa43f4f047f01243d6be54dadd9d67a
>>>
>>> This allows registering -1 as the fd, so you could register 10 files,
>>> but an array of size 500 (for example), and the last 490 fds are just
>>> -1. Then you can use the IORING_REGISTER_FILES_UPDATE to replace those
>>> empty fds with real files later on.
>> 
>> That makes more sense, but still requires a priori knowledge of how many
>> files you'll need to work with, otherwise you're back to
>> unregister/register dance.  Is it really that hard to grow on demand?
>
> It's not, it's just more efficient to add a file through replace, than it
> is to alloc a new array, copy things over, free it. That also impacts the
> application side of things, since that has to maintain an array of
> descriptors so that it knows what fd maps to what index.

Yeah, that's a good point about the application side.

> If you expose growing, you also have some kind of obligation to make it
> efficient, and I just don't think that's possible. But there's nothing
> preventing this API from supporting it, you'd just do an update with
> offset == current_array_size, and then nr_args as what to grow with.
> Currently that would -EINVAL, but it could be added as a feature.

OK, I'm fine with the API as-is.  If someone asks, growing can be added
later.

Cheers,
Jeff