public inbox for [email protected]
 help / color / mirror / Atom feed
From: Pavel Begunkov <[email protected]>
To: Jens Axboe <[email protected]>, io-uring <[email protected]>
Subject: Re: [PATCH for-next] io_uring: ensure IOSQE_ASYNC file table grabbing works, with SQPOLL
Date: Wed, 9 Sep 2020 10:09:28 +0300	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 09/09/2020 01:54, Jens Axboe wrote:
> On 9/8/20 3:22 PM, Jens Axboe wrote:
>> On 9/8/20 2:58 PM, Pavel Begunkov wrote:
>>> On 08/09/2020 20:48, Jens Axboe wrote:
>>>> Fd instantiating commands like IORING_OP_ACCEPT now work with SQPOLL, but
>>>> we have an error in grabbing that if IOSQE_ASYNC is set. Ensure we assign
>>>> the ring fd/file appropriately so we can defer grab them.
>>>
>>> IIRC, for fcheck() in io_grab_files() to work it should be under fdget(),
>>> that isn't the case with SQPOLL threads. Am I mistaken?
>>>
>>> And it looks strange that the following snippet will effectively disable
>>> such requests.
>>>
>>> fd = dup(ring_fd)
>>> close(ring_fd)
>>> ring_fd = fd
>>
>> Not disagreeing with that, I think my initial posting made it clear
>> it was a hack. Just piled it in there for easier testing in terms
>> of functionality.
>>
>> But the next question is how to do this right...> 
> Looking at this a bit more, and I don't necessarily think there's a
> better option. If you dup+close, then it just won't work. We have no
> way of knowing if the 'fd' changed, but we can detect if it was closed
> and then we'll end up just EBADF'ing the requests.
> 
> So right now the answer is that we can support this just fine with
> SQPOLL, but you better not dup and close the original fd. Which is not
> ideal, but better than NOT being able to support it.
> 
> Only other option I see is to to provide an io_uring_register()
> command to update the fd/file associated with it. Which may be useful,
> it allows a process to indeed to this, if it absolutely has to.

Let's put aside such dirty hacks, at least until someone actually
needs it. Ideally, for many reasons I'd prefer to get rid of
fcheck(ctx->ring_fd) in favour of synchronisation in
io_grab_files(), but I wish I knew how.

-- 
Pavel Begunkov

  parent reply	other threads:[~2020-09-09  7:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-08 17:48 [PATCH for-next] io_uring: ensure IOSQE_ASYNC file table grabbing works, with SQPOLL Jens Axboe
2020-09-08 20:58 ` Pavel Begunkov
2020-09-08 21:22   ` Jens Axboe
2020-09-08 22:54     ` Jens Axboe
2020-09-09  0:48       ` Josef
2020-09-09  7:09       ` Pavel Begunkov [this message]
2020-09-09 13:10         ` Jens Axboe
2020-09-09 13:53           ` Jens Axboe
2020-09-09 15:48           ` Pavel Begunkov
2020-09-09 16:07             ` Jens Axboe
2020-09-10 12:37               ` Pavel Begunkov
2020-09-10 13:11                 ` Jens Axboe
2020-09-10 18:18                   ` Jens Axboe
2020-09-10 21:01                     ` Jens Axboe
2020-09-10 22:11                       ` Jens Axboe
2020-09-10 23:04                         ` Jens Axboe
2020-09-11 19:23                     ` Pavel Begunkov
2020-09-11 20:06                       ` 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 \
    [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