Messages in this thread | | | From | Logan Gunthorpe <> | Date | Mon, 28 Oct 2019 10:58:56 -0600 | Subject | Re: [RFC PATCH 3/3] nvme: Introduce nvme_execute_passthru_rq_nowait() |
| |
On 2019-10-27 9:09 a.m., Christoph Hellwig wrote: > On Fri, Oct 25, 2019 at 02:25:35PM -0600, Logan Gunthorpe wrote: >> This function is similar to nvme_execute_passthru_rq() but does >> not wait and will call a callback when the request is complete. >> >> The new function can also be called in interrupt context, so if there >> are side effects, the request will be executed in a work queue to >> avoid sleeping. > > Why would you ever call it from interrupt context? All the target > submission handlers should run in process context.
Oh, I mis-understood this a bit and worded that incorrectly. The intent is to avoid having to call nvme_passthru_end() in the completion handler which can be in interrupt context.
>> +void nvme_execute_passthru_rq_nowait(struct request *rq, rq_end_io_fn *done) >> +{ >> + struct nvme_command *cmd = nvme_req(rq)->cmd; >> + struct nvme_ctrl *ctrl = nvme_req(rq)->ctrl; >> + struct nvme_ns *ns = rq->q->queuedata; >> + struct gendisk *disk = ns ? ns->disk : NULL; >> + u32 effects; >> + >> + /* >> + * This function may be called in interrupt context, so we cannot sleep >> + * but nvme_passthru_[start|end]() may sleep so we need to execute >> + * the command in a work queue. >> + */ >> + effects = nvme_command_effects(ctrl, ns, cmd->common.opcode); >> + if (effects) { >> + rq->end_io = done; >> + INIT_WORK(&nvme_req(rq)->work, nvme_execute_passthru_rq_work); >> + queue_work(nvme_wq, &nvme_req(rq)->work); > > But independent of the target code - I'd much rather leave this to the > caller. Just call nvme_command_effects in the target code, then if > there are not side effects use blk_execute_rq_nowait directly, else > schedule a workqueue in the target code and call > nvme_execute_passthru_rq from it.
Ok, that seems sensible. Except it conflicts a bit with Sagi's feedback: presumably we need to cancel the work items during nvme_stop_ctrl() and that's going to be rather difficult to do from the caller. Are we saying this is unnecessary? It's not clear to me if passthru_start/end is going to be affected by nvme_stop_ctrl() which I believe is the main concern.
Logan
| |