Re: [PATCH] nvme: reject completions for requests that are not in flight

[Jens Axboe <axboe@kernel.dk>]

On 11/3/26 12:54 PM, Mauricio Faria de Oliveira wrote:
> On 2026-11-03 16:43, Keith Busch wrote:
>> On Tue, Nov 03, 2026 at 04:18:25PM -0300, Mauricio Faria de Oliveira wrote:
>>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>>> index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 100644
>>> --- a/drivers/nvme/host/nvme.h
>>> +++ b/drivers/nvme/host/nvme.h
>>> @@ -708,6 +708,11 @@ static inline struct request *nvme_find_rq(struct blk_mq_tags *tags,
>>>  			tag, genctr, nvme_genctr_mask(nvme_req(rq)->genctr));
>>>  		return NULL;
>>>  	}
>>> +	if (unlikely(blk_mq_rq_state(rq) != MQ_RQ_IN_FLIGHT)) {
>>> +		dev_err(nvme_req(rq)->ctrl->device,
>>> +			"completion for request %#x not in flight\n", tag);
>>> +		return NULL;
>>> +	}
>>>  	return rq;
>>>  }
>>
>> Is there something special about the NVMe protocol that makes this
>> problem unique to it? If not, why doesn't this check go into the generic
>> block layer instead?
> 
> Thanks for taking a look.
> 
> I noticed you asked the latter question in the previous
> submission/review (Link: trailers), and addressed it in the commit
> message; pasting it here for reference:
> 
> In Chao Shi's patch, there's been feedback/question of whether this
> should
> be done in blk-mq, as this is not specific to nvme. I'd say no, at
> first:
> - Some blk-mq calls for completion have no return value (e.g.,
> completion
>   on other CPUs), which suggests maybe an API change and updates to many
>   callers, all for an edge case.
> - An alternative available currently might be blk_mq_ops.complete(), but
>   that is handled in the driver anyway, with risk of hitting an error in
>   blk-mq (like this) as the code changes, and seems semantically wrong,
>   i.e., completing a request that should not have been completed.
> - Keeping in mind a small fix for stable, for now, should this be
> merged.
> - But happy to take suggestions if blk-mq is indeed the way to go.
> 
> For the former question, I'd imagine other block drivers might be
> subject to a similar issue, under incorrect completions, and can take a
> look at this later. I'd appreciate feedback on the points above before
> trying to address it generically in the block layer, if at all possible.

This is either a protocol problem, in which case the driver should
handle it. Or it's not a problem at all in the first place, and
this is some kind of misguided defensive programming. In which case
the patch should go away.

If this is possible because of a poor API in the kernel, then we
fix the poor API. We don't add silly work-arounds.

-- 
Jens Axboe