Re: [PATCH for-next] io_uring: ensure IOSQE_ASYNC file table grabbing works, with SQPOLL

[Josef <josef.grieb@gmail.com>]

BTW I can confirm this patch works for me, thanks a lot :)

---
Josef

On Wed, 9 Sep 2020 at 00:55, Jens Axboe <axboe@kernel.dk> 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.
>
> --
> Jens Axboe
>