Re: [PATCH 6/6] io_uring: enable multishot mode for accept

[Pavel Begunkov <asml.silence@gmail.com>]

On 9/5/21 7:25 AM, Hao Xu wrote:
> 在 2021/9/5 上午6:43, Pavel Begunkov 写道:
>> On 9/3/21 12:00 PM, Hao Xu wrote:
>>> Update io_accept_prep() to enable multishot mode for accept operation.
>>>
>>> Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
>>> ---
>>>   fs/io_uring.c | 14 ++++++++++++--
>>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>> index eb81d37dce78..34612646ae3c 100644
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -4861,6 +4861,7 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
>>>   static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>   {
>>>       struct io_accept *accept = &req->accept;
>>> +    bool is_multishot;
>>>         if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>>           return -EINVAL;
>>> @@ -4872,14 +4873,23 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>       accept->flags = READ_ONCE(sqe->accept_flags);
>>>       accept->nofile = rlimit(RLIMIT_NOFILE);
>>>   +    is_multishot = accept->flags & IORING_ACCEPT_MULTISHOT;
>>> +    if (is_multishot && (req->flags & REQ_F_FORCE_ASYNC))
>>> +        return -EINVAL;
>>
>> Why REQ_F_FORCE_ASYNC is not allowed? It doesn't sound like there
>> should be any problem, would just eventually go looping
>> poll_wait + tw
> Hmm..The arm_poll facility is only on the io_submit_sqes() path. If
> FORCE_ASYNC is set, the req goes to io_wq_submit_work-->io_issue_sqe.
> Moreover, I guess theoretically poll based retry and async iowq are
> two ways for users to handle their sqes, it may not be sane to go to
> poll based retry path if a user already forcely choose iowq as their
> prefer way.

The flag is just a hint, there are already plenty of ways to trick
requests into tw. Forbidding it would be inconsistent.


>>> +
>>>       accept->file_slot = READ_ONCE(sqe->file_index);
>>>       if (accept->file_slot && ((req->open.how.flags & O_CLOEXEC) ||
>>> -                  (accept->flags & SOCK_CLOEXEC)))
>>> +                  (accept->flags & SOCK_CLOEXEC) || is_multishot))
>>>           return -EINVAL;
>>> -    if (accept->flags & ~(SOCK_CLOEXEC | SOCK_NONBLOCK))
>>> +    if (accept->flags & ~(SOCK_CLOEXEC | SOCK_NONBLOCK | IORING_ACCEPT_MULTISHOT))
>>>           return -EINVAL;
>>>       if (SOCK_NONBLOCK != O_NONBLOCK && (accept->flags & SOCK_NONBLOCK))
>>>           accept->flags = (accept->flags & ~SOCK_NONBLOCK) | O_NONBLOCK;
>>> +    if (is_multishot) {
>>> +        req->flags |= REQ_F_APOLL_MULTISHOT;
>>> +        accept->flags &= ~IORING_ACCEPT_MULTISHOT;
>>> +    }
>>> +
>>>       return 0;
>>>   }
>>>  
>>
> 

-- 
Pavel Begunkov