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
