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
