public inbox for [email protected]
 help / color / mirror / Atom feed
From: Xiaoguang Wang <[email protected]>
To: Jens Axboe <[email protected]>, Hao Xu <[email protected]>,
	[email protected]
Cc: [email protected]
Subject: Re: [RFC] io_uring: let IORING_OP_FILES_UPDATE support to choose fixed file slot
Date: Mon, 30 May 2022 10:47:53 +0800	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

hi,

> On 5/28/22 6:28 AM, Jens Axboe wrote:
>> On 5/28/22 3:45 AM, Xiaoguang Wang wrote:
>>> hi Hao,
>>>
>>>> Hi Xiaoguang,
>>>>
>>>> On 5/26/22 20:38, Xiaoguang Wang wrote:
>>>>> One big issue with file registration feature is that it needs user
>>>>> space apps to maintain free slot info about io_uring's fixed file
>>>>> table, which really is a burden for development. Now since io_uring
>>>>> starts to choose free file slot for user space apps by using
>>>>> IORING_FILE_INDEX_ALLOC flag in accept or open operations, but they
>>>>> need app to uses direct accept or direct open, which as far as I know,
>>>>> some apps are not prepared to use direct accept or open yet.
>>>>>
>>>>> To support apps, who still need real fds, use registration feature
>>>>> easier, let IORING_OP_FILES_UPDATE support to choose fixed file slot,
>>>>> which will return free file slot in cqe->res.
>>>>>
>>>>> TODO list:
>>>>>      Need to prepare liburing corresponding helpers.
>>>>>
>>>>> Signed-off-by: Xiaoguang Wang <[email protected]>
>>>>> ---
>>>>>   fs/io_uring.c                 | 50 ++++++++++++++++++++++++++++++++++---------
>>>>>   include/uapi/linux/io_uring.h |  1 +
>>>>>   2 files changed, 41 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>>>> index 9f1c682d7caf..d77e6bbec81c 100644
>>>>> --- a/fs/io_uring.c
>>>>> +++ b/fs/io_uring.c
>>>>> @@ -680,6 +680,7 @@ struct io_rsrc_update {
>>>>>       u64                arg;
>>>>>       u32                nr_args;
>>>>>       u32                offset;
>>>>> +    u32                flags;
>>>>>   };
>>>>>     struct io_fadvise {
>>>>> @@ -7970,14 +7971,23 @@ static int io_async_cancel(struct io_kiocb *req, unsigned int issue_flags)
>>>>>       return 0;
>>>>>   }
>>>>>   +#define IORING_FILES_UPDATE_INDEX_ALLOC 1
>>>>> +
>>>>>   static int io_rsrc_update_prep(struct io_kiocb *req,
>>>>>                   const struct io_uring_sqe *sqe)
>>>>>   {
>>>>> +    u32 flags = READ_ONCE(sqe->files_update_flags);
>>>>> +
>>>>>       if (unlikely(req->flags & (REQ_F_FIXED_FILE | REQ_F_BUFFER_SELECT)))
>>>>>           return -EINVAL;
>>>>> -    if (sqe->rw_flags || sqe->splice_fd_in)
>>>>> +    if (sqe->splice_fd_in)
>>>>> +        return -EINVAL;
>>>>> +    if (flags & ~IORING_FILES_UPDATE_INDEX_ALLOC)
>>>>> +        return -EINVAL;
>>>>> +    if ((flags & IORING_FILES_UPDATE_INDEX_ALLOC) && READ_ONCE(sqe->len) != 1)
>>>> How about allowing multiple fd update in IORING_FILES_UPDATE_INDEX_ALLOC
>>>> case? For example, using the sqe->addr(the fd array) to store the slots we allocated, and let cqe return the number of slots allocated.
>>> Good idea, I'll try in patch v2, thanks.
>>> Jens, any comments about this patch? At least It's really helpful to our
>>> internal apps based on io_uring :)
>> I like this suggestion too, other thoughts in reply to the original.
> BTW, if you have time, would be great to get this done for 5.19. It
> makes the whole thing more consistent and makes it so that 5.19 has
> (hopefully) all the alloc bits for direct descriptors.
Yeah, I have free time now, will prepare new version today.

Regards,
Xiaoguang Wang

>


  reply	other threads:[~2022-05-30  2:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-26 12:38 [RFC] io_uring: let IORING_OP_FILES_UPDATE support to choose fixed file slot Xiaoguang Wang
2022-05-27  5:54 ` Hao Xu
2022-05-28  9:45   ` Xiaoguang Wang
2022-05-28 12:28     ` Jens Axboe
2022-05-29 16:57       ` Jens Axboe
2022-05-30  2:47         ` Xiaoguang Wang [this message]
2022-05-28 12:27 ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2333ef08-250b-6f8d-10e9-d3c9040bcc47@linux.alibaba.com \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox