On 19/01/2021 08:00, Joseph Qi wrote: > > > On 1/19/21 10:38 AM, Pavel Begunkov wrote: >> On 19/01/2021 01:58, Joseph Qi wrote: >>>> Hmm, I hastened, for files we need IO_WQ_WORK_FILES, >>>> +IO_WQ_WORK_BLKCG for same reasons. needs_file would make >>>> it to grab a struct file, that is wrong. >>>> Probably worked out because it just grabbed fd=0/stdin. >>>> >>> >>> I think IO_WQ_WORK_FILES can work since it will acquire >>> files when initialize async cancel request. >> >> That the one controlling files in the first place, need_file >> just happened to grab them submission. >> >>> Don't quite understand why we should have IO_WQ_WORK_BLKCG. >> >> Because it's set for IORING_OP_CLOSE, and similar situation >> may happen but with async_cancel from io-wq. >> > So how about do switch and restore in io_run_cancel(), seems it can > take care of direct request, sqthread and io-wq cases. It will get ugly pretty quickly, + this nesting of io-wq handlers async_handler() -> io_close() is not great... I'm more inclined to skip them in io_wqe_cancel_pending_work() to not execute inline. That may need to do some waiting on the async_cancel side though to not change the semantics. Can you try out this direction? > >> Actually, it's even nastier than that, and neither of io_op_def >> flags would work because for io-wq case you can end up doing >> close() with different from original files. I'll think how it >> can be done tomorrow. >> -- Pavel Begunkov