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

[Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>]

hi,

> 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.
Now I see :) will help me to understand current codes better.
Really thanks for your explanations.

Regards,
Xiaoguang Wang
>