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 07:43:15PM +0530, Kanchan Joshi wrote:
> On Fri, Apr 14, 2023 at 09:53:15PM +0800, Ming Lei wrote:
> > 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.
> 
> FWIW, the model I had in mind (in initial days) was this -
> 1) io_uring_cmd_done is a simple API, it just posts one/two results into
> reuglar/big SQE
> 2) for anything complex completion (that requires task-work), it will
> use another API io_uring_cmd_complete_in_task with the provider-specific
> callback (that will call above simple API eventually)
 
IMO, the current two APIs type are fine, from ublk viewpoint at least.

io_uring setup/setting is transparent/invisible to driver, and it is reasonable
for the two interfaces to hide any io_uring implementation details.

Meantime driver should be free to choose either of the two.


Thanks, 
Ming