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

[Keith Busch <kbusch@kernel.org>]

On Wed, Jan 18, 2023 at 06:33:06AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 17, 2023 at 09:22:44PM -0800, Keith Busch wrote:
> > 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 STARTED.
> > 
> > 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.
> 
> Hmm.  Using a started helper for something that by definition is started
> doesn't really make much sense.  I guess the problem here is that
> blk_mq_end_request_batch sets the state to MQ_RQ_IDLE afte calling
> blk_complete_request?  Maybe we just need an explicit check for
> MQ_RQ_IDLE here as started seems like the wrong implication here.

We're actually not batching here (no IOB in the timeout context), so we
are either:

  a. calling nvme_pci_complete_rq() inline with the cqe
  b. racing with smp ipi or softirq

If case (a), we will always see IDLE. If (b), we are racing and may see
either COMPLETED or IDLE, so we have to check that it's not either of
those. Since there's only one other state (STARTED) that was guaranteed
prior to entering the timeout handler, we can just make sure it's not
that one after the poll to know if abort escalation is needed.