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

[Christoph Hellwig <hch@lst.de>]

On Wed, Jan 18, 2023 at 08:21:58AM -0700, Keith Busch wrote:
> > The point is still that "started" is the wrong check here and relies
> > on an implementation detail.  I think we're better off with an explicit
> > IDLE check and a big fat comment.
> 
> So you want the check to look like this instead?
> 
> ---
> @@ -1362,7 +1362,8 @@ 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_request_completed(req) ||
> +           blk_mq_rq_state(req) == MQ_RQ_IDLE) {
>                 dev_warn(dev->ctrl.device,
>                          "I/O %d QID %d timeout, completion polled\n",
>                          req->tag, nvmeq->qid);
> --

Or maybe just:

	if (blk_mq_rq_state(req) != MQ_RQ_IN_FLIGHT) {
		..

> Alternatively, I also considered moving the IDLE state setting to when
> the request is actually freed, which might make more sense and works
> without changing the nvme driver:

I like that idea, but this touches gnarly code, so it'll need good
testing.