| Message ID | 20250311024144.1762333-2-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: nvme: fix blktests nvme/039 failure | expand |
On 3/11/25 03:41, Shin'ichiro Kawasaki wrote: > Before the Commit 1f47ed294a2b ("block: cleanup and fix batch completion > adding conditions"), blk_mq_add_to_batch() did not add failed > passthrough requests to batch, and returned false. After the commit, > blk_mq_add_to_batch() always adds passthrough requests to batch > regardless of whether the request failed or not, and returns true. This > affected error logging feature in the NVME driver. > > Before the commit, the call chain of failed passthrough request was as > follows: > > nvme_handle_cqe() > blk_mq_add_to_batch() .. false is returned, then call nvme_pci_complete_rq() > nvme_pci_complete_rq() > nvme_complete_rq() > nvme_end_req() > nvme_log_err_passthru() .. error logging > __nvme_end_req() .. end of the rqeuest > > After the commit, the call chain is as follows: > > nvme_handle_cqe() > blk_mq_add_to_batch() .. true is returned, then set nvme_pci_complete_batch() > .. > nvme_pci_complete_batch() > nvme_complete_batch() > nvme_complete_batch_req() > __nvme_end_req() .. end of the request, without error logging > > To make the error logging feature work again for passthrough requests, move the > nvme_log_err_passthru() call from nvme_end_req() to __nvme_end_req(). > > While at it, move nvme_log_error() call for non-passthrough requests together > with nvme_log_err_passthru(). Even though the trigger commit does not affect > non-passthrough requests, move it together for code simplicity. > > Fixes: 1f47ed294a2b ("block: cleanup and fix batch completion adding conditions") > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > --- > drivers/nvme/host/core.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index f028913e2e62..8359d0aa0e44 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -431,6 +431,12 @@ static inline void nvme_end_req_zoned(struct request *req) static inline void __nvme_end_req(struct request *req) { + if (unlikely(nvme_req(req)->status && !(req->rq_flags & RQF_QUIET))) { + if (blk_rq_is_passthrough(req)) + nvme_log_err_passthru(req); + else + nvme_log_error(req); + } nvme_end_req_zoned(req); nvme_trace_bio_complete(req); if (req->cmd_flags & REQ_NVME_MPATH) @@ -441,12 +447,6 @@ void nvme_end_req(struct request *req) { blk_status_t status = nvme_error_status(nvme_req(req)->status); - if (unlikely(nvme_req(req)->status && !(req->rq_flags & RQF_QUIET))) { - if (blk_rq_is_passthrough(req)) - nvme_log_err_passthru(req); - else - nvme_log_error(req); - } __nvme_end_req(req); blk_mq_end_request(req, status); }
Before the Commit 1f47ed294a2b ("block: cleanup and fix batch completion adding conditions"), blk_mq_add_to_batch() did not add failed passthrough requests to batch, and returned false. After the commit, blk_mq_add_to_batch() always adds passthrough requests to batch regardless of whether the request failed or not, and returns true. This affected error logging feature in the NVME driver. Before the commit, the call chain of failed passthrough request was as follows: nvme_handle_cqe() blk_mq_add_to_batch() .. false is returned, then call nvme_pci_complete_rq() nvme_pci_complete_rq() nvme_complete_rq() nvme_end_req() nvme_log_err_passthru() .. error logging __nvme_end_req() .. end of the rqeuest After the commit, the call chain is as follows: nvme_handle_cqe() blk_mq_add_to_batch() .. true is returned, then set nvme_pci_complete_batch() .. nvme_pci_complete_batch() nvme_complete_batch() nvme_complete_batch_req() __nvme_end_req() .. end of the request, without error logging To make the error logging feature work again for passthrough requests, move the nvme_log_err_passthru() call from nvme_end_req() to __nvme_end_req(). While at it, move nvme_log_error() call for non-passthrough requests together with nvme_log_err_passthru(). Even though the trigger commit does not affect non-passthrough requests, move it together for code simplicity. Fixes: 1f47ed294a2b ("block: cleanup and fix batch completion adding conditions") Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- drivers/nvme/host/core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)