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
