On 1/17/21 3:52 AM, Pavel Begunkov wrote: > On 17/01/2021 04:04, Jens Axboe wrote: >> We used to have task exit tied to canceling files_struct ownership, but we >> really should just simplify this and cancel any request that the task has >> pending when it exits. Instead of handling files ownership specially, we >> do the same regardless of request type. >> >> This can be further simplified in the next major kernel release, unifying >> how we cancel across exec and exit. > > Looks good in general. See a comment below, but otherwise > Reviewed-by: Pavel Begunkov <asml.silence@xxxxxxxxx> > > btw, I wonder if we can incite syzbot to try to break it. > >> >> Cc: stable@xxxxxxxxxxxxxxx # 5.9+ >> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> >> >> --- >> >> diff --git a/fs/io_uring.c b/fs/io_uring.c >> index 383ff6ed3734..1190296fc95f 100644 >> --- a/fs/io_uring.c >> +++ b/fs/io_uring.c >> @@ -9029,7 +9029,7 @@ static void io_uring_remove_task_files(struct io_uring_task *tctx) >> io_uring_del_task_file(file); >> } >> >> -void __io_uring_files_cancel(struct files_struct *files) >> +static void __io_uring_files_cancel(void) >> { >> struct io_uring_task *tctx = current->io_uring; >> struct file *file; >> @@ -9038,11 +9038,10 @@ void __io_uring_files_cancel(struct files_struct *files) >> /* make sure overflow events are dropped */ >> atomic_inc(&tctx->in_idle); >> xa_for_each(&tctx->xa, index, file) >> - io_uring_cancel_task_requests(file->private_data, files); >> + io_uring_cancel_task_requests(file->private_data, NULL); >> atomic_dec(&tctx->in_idle); >> >> - if (files) >> - io_uring_remove_task_files(tctx); >> + io_uring_remove_task_files(tctx); > > This restricts cancellations to only one iteration. Just delete it, > __io_uring_task_cancel() calls it already. Ah indeed, makes it even better. I removed it, thanks. -- Jens Axboe