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