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

[Jens Axboe <axboe@kernel.dk>]

On 10/20/20 10:29 AM, Pavel Begunkov wrote:
> On 20/10/2020 15:09, Jens Axboe wrote:
>> On 10/19/20 5:40 PM, Pavel Begunkov wrote:
>>> On 19/10/2020 21:08, Jens Axboe wrote:
>>>> On 10/19/20 9:45 AM, Pavel Begunkov 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.
>>>>
>>>> Why not just keep the !data for task_drop? Would make the diff take
>>>> away just the hunk we're interested in. Even adding a comment would be
>>>> better, imho.
>>>
>>> That would look cleaner, but I just left what already was there. TBH,
>>> I don't even entirely understand why exiting=!data. Looking up how
>>> exit_files() works, it passes down non-NULL files to
>>> put_files_struct() -> ... filp_close() -> f_op->flush().
>>>
>>> I'm curious how does this filp_close(file, files=NULL) happens?
>>
>> It doesn't, we just clear it internall to match all requests, not just
>> files backed ones.
> 
> Then my "bool exiting = !data;" at the start doesn't make sense since
> passed in @data is always non-NULL.

Right, only if we retain this part:

if (fatal_signal_pending(current) || (current->flags & PF_EXITING))
	data = NULL;

in there. I suspect we'll want something ala the below instead.

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 09e7a5f20060..4870db000f04 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -8695,14 +8695,18 @@ static void __io_uring_attempt_task_drop(struct file *file)
  * Drop task note for this file if we're the only ones that hold it after
  * pending fput()
  */
-static void io_uring_attempt_task_drop(struct file *file, bool exiting)
+static void io_uring_attempt_task_drop(struct file *file)
 {
+	bool exiting;
+
 	if (!current->io_uring)
 		return;
+
 	/*
 	 * fput() is pending, will be 2 if the only other ref is our potential
 	 * task file note. If the task is exiting, drop regardless of count.
 	 */
+	exiting = fatal_signal_pending(current) || (current->flags & PF_EXITING);
 	if (!exiting && atomic_long_read(&file->f_count) != 2)
 		return;
 
@@ -8764,16 +8768,7 @@ void __io_uring_task_cancel(void)
 
 static int io_uring_flush(struct file *file, void *data)
 {
-	struct io_ring_ctx *ctx = file->private_data;
-
-	/*
-	 * If the task is going away, cancel work it may have pending
-	 */
-	if (fatal_signal_pending(current) || (current->flags & PF_EXITING))
-		data = NULL;
-
-	io_uring_cancel_task_requests(ctx, data);
-	io_uring_attempt_task_drop(file, !data);
+	io_uring_attempt_task_drop(file);
 	return 0;
 }
 

-- 
Jens Axboe