Re: [PATCH] io_uring: complete request via task work in case of DEFER_TASKRUN

[Ming Lei <ming.lei@redhat.com>]

On Fri, Apr 14, 2023 at 02:01:26PM +0100, Pavel Begunkov wrote:
> On 4/14/23 08:53, Ming Lei wrote:
> > So far io_req_complete_post() only covers DEFER_TASKRUN by completing
> > request via task work when the request is completed from IOWQ.
> > 
> > However, uring command could be completed from any context, and if io
> > uring is setup with DEFER_TASKRUN, the command is required to be
> > completed from current context, otherwise wait on IORING_ENTER_GETEVENTS
> > can't be wakeup, and may hang forever.
> 
> fwiw, there is one legit exception, when the task is half dead
> task_work will be executed by a kthread. It should be fine as it
> locks the ctx down, but I can't help but wonder whether it's only
> ublk_cancel_queue() affected or there are more places in ublk?

No, it isn't.

It isn't triggered on nvme-pt just because command is always done
in task context.

And we know more uring command cases are coming.

> 
> One more thing, cmds should not be setting issue_flags but only
> forwarding what the core io_uring code passed, it'll get tons of
> bugs in no time otherwise.

Here io_uring_cmd_done() is changed to this way recently, and it
could be another topic.

> 
> static void ublk_cancel_queue(struct ublk_queue *ubq)
> {
>     ...
>     io_uring_cmd_done(io->cmd, UBLK_IO_RES_ABORT, 0,
>                       IO_URING_F_UNLOCKED);
> }
> 
> Can we replace it with task_work? It should be cold, and I
> assume ublk_cancel_queue() doesn't assume that all requests will
> put down by the end of the function as io_uring_cmd_done()
> can offload it in any case.

But it isn't specific for ublk, any caller of io_uring_cmd_done()
has such issue since io_uring_cmd_done() is one generic API.


thanks,
Ming