[PATCHv3] nvme-pci: fix timeout request state check

[PATCHv3] nvme-pci: fix timeout request state check

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

Polling the completion can progress the request state to IDLE, either
inline with the completion, or through softirq. Either way, the state
may not be COMPLETED, so don't check for that. We only care if the state
isn't IN_FLIGHT.

This is fixing an issue where the driver aborts an IO that we just
completed. Seeing the "aborting" message instead of "polled" is very
misleading as to where the timeout problem resides.

Fixes: bf392a5dc02a9b ("nvme-pci: Remove tag from process cq")
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v2->v3: changed state check to explicitly look for not IN_FLIGHT

 drivers/nvme/host/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index a2553b7d9bb8e..1ff8843bc4b36 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1362,7 +1362,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req)
 	else
 		nvme_poll_irqdisable(nvmeq);
 
-	if (blk_mq_request_completed(req)) {
+	if (blk_mq_rq_state(req) != MQ_RQ_IN_FLIGHT) {
 		dev_warn(dev->ctrl.device,
 			 "I/O %d QID %d timeout, completion polled\n",
 			 req->tag, nvmeq->qid);
-- 
2.30.2

Re: [PATCHv3] nvme-pci: fix timeout request state check

[Christoph Hellwig]

Thanks,

applied to nvme-6.2.

Re: [PATCHv3] nvme-pci: fix timeout request state check

[Sagi Grimberg]

> From: Keith Busch <kbusch@kernel.org>
> 
> Polling the completion can progress the request state to IDLE, either
> inline with the completion, or through softirq. Either way, the state
> may not be COMPLETED, so don't check for that. We only care if the state
> isn't IN_FLIGHT.
> 
> This is fixing an issue where the driver aborts an IO that we just
> completed. Seeing the "aborting" message instead of "polled" is very
> misleading as to where the timeout problem resides.
> 
> Fixes: bf392a5dc02a9b ("nvme-pci: Remove tag from process cq")
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v2->v3: changed state check to explicitly look for not IN_FLIGHT
> 
>   drivers/nvme/host/pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index a2553b7d9bb8e..1ff8843bc4b36 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1362,7 +1362,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req)
>   	else
>   		nvme_poll_irqdisable(nvmeq);
>   
> -	if (blk_mq_request_completed(req)) {
> +	if (blk_mq_rq_state(req) != MQ_RQ_IN_FLIGHT) {

I think that this change also applies to nvmf_complete_timed_out_request
no?