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. > >> Moreover, if that's exit_files() which is interesting, then first >> it calls io_uring_cancel_task_requests(), which should remove all >> struct file from tctx->xa. I haven't tested it though. > > Yep, further cleanups are certainly possible there. > > I've queued this up, thanks. > -- Pavel Begunkov
