Re: [PATCH for-5.10] io_uring: remove req cancel in ->flush()

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

On 22/10/2020 07:42, Xiaoguang Wang wrote:
>> Every close(io_uring) causes cancellation of all inflight requests
>> carrying ->files. That's not nice but was neccessary up until recently.
>> Now task->files removal is handled in the core code, so that part of
>> flush can be removed.
> I don't catch up with newest io_uring codes yet, but have one question about
> the initial implementation "io_uring: io_uring: add support for async work
> inheriting files": https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fcb323cc53e29d9cc696d606bb42736b32dd9825
> There was such comments:
> +static int io_grab_files(struct io_ring_ctx *ctx, struct io_kiocb *req)
> +{
> +       int ret = -EBADF;
> +
> +       rcu_read_lock();
> +       spin_lock_irq(&ctx->inflight_lock);
> +       /*
> +        * We use the f_ops->flush() handler to ensure that we can flush
> +        * out work accessing these files if the fd is closed. Check if
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> I wonder why we only need to flush reqs specifically when they access current->files, are there
> any special reasons?
We could have taken a reference to ->files, so it doesn't get freed
while a request is using it, but that creates a circular dependency.

IIUC, because if there are ->files refs io_uring won't be closed on exit,
but io_uring would be holding ->files refs.

So, it was working without taking ->files references (i.e. weak refs)
on the basis that the files won't be destroyed until the task itself is
gone, and we can *kind of* intercept when task is exiting by ->flush()
and cancel all requests with ->files there.

-- 
Pavel Begunkov