On Mon, Apr 26, 2021 at 10:15 AM Keith Busch <kbusch@xxxxxxxxxx> wrote: > > On Mon, Apr 26, 2021 at 10:10:09AM -0700, Yuanyuan Zhong wrote: > > On Fri, Apr 23, 2021 at 3:06 PM Keith Busch <kbusch@xxxxxxxxxx> wrote: > > > > > > We don't have an nvme status to report if the driver's .queue_rq() > > > returns an error without dispatching the requested nvme command. Use the > > > return value from blk_execute_rq() for all passthrough commands so the > > > caller may know their command was not successful. > > > > > > If the command is from the target passthrough interface and fails to > > > dispatch, synthesize the response back to the host as a internal target > > > error. > > > > > > Signed-off-by: Keith Busch <kbusch@xxxxxxxxxx> > > > --- > > > drivers/nvme/host/core.c | 16 ++++++++++++---- > > > drivers/nvme/host/ioctl.c | 6 +----- > > > drivers/nvme/host/nvme.h | 2 +- > > > drivers/nvme/target/passthru.c | 8 ++++---- > > > 4 files changed, 18 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > > > index 10bb8406e067..62af5fe7a0ce 100644 > > > --- a/drivers/nvme/host/core.c > > > +++ b/drivers/nvme/host/core.c > > > @@ -972,12 +972,12 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd, > > > goto out; > > > } > > > > > > - blk_execute_rq(NULL, req, at_head); > > > + ret = blk_execute_rq(NULL, req, at_head); > > > if (result) > > > *result = nvme_req(req)->result; > > > if (nvme_req(req)->flags & NVME_REQ_CANCELLED) > > > ret = -EINTR; > > > - else > > > + else if (nvme_req(req)->status) > > > > Since nvme_req(req)->status is uninitialized for a command failed to dispatch, > > it's valid only if ret from blk_execute_rq() is 0. > > That's not quite right. If queue_rq() succeeds, but the SSD returns an > error, blk_execute_rq() returns a non-zero value with a valid nvme_req > status. Agreed. But after that, freeing the req let nvme_req(req)->status from SSD stay. If the same req get re-allocated and get dispatching failure, shouldn't the dispatching error take precedence over the stale nvme_req(req)->status?
