Re: [PATCH 2/2] nvme: avoid possible double completions for the same request

[yebin <yebin@huaweicloud.com>]

On 2026/6/8 23:17, Keith Busch wrote:
> On Mon, Jun 08, 2026 at 07:34:25PM +0800, Ye Bin wrote:
>> To avoid the preceding problem, the NVME_REQ_COMPLETE flag is added by
>> referring to the implementation of scsi commit f1342709d18a ("scsi: Do not
>> rely on blk-mq for double completions").
>
> That scsi commit was solving a different problem for a racing
> interaction between the low level driver and the timeout handler and
> error injection. It wasn't about protecting against misbehaving
> hardware.
>
>>   static inline struct nvme_request *nvme_req(struct request *req)
>> @@ -807,6 +808,8 @@ static inline bool nvme_try_complete_req(struct request *req, __le16 status,
>>   	nvme_should_fail(req);
>>   	if (unlikely(blk_should_fake_timeout(req->q)))
>>   		return true;
>> +	if (unlikely(test_and_set_bit(NVME_REQ_COMPLETE, &rq->flags)))
>> +		return true;
>
> I think you need to invert this flag from "COMPLETE" to "INFLIGHT",
> because the default allocated state is that this flag is cleared, so
> this check as you have it wouldn't catch a phantom completion to a
> request the host never sent.
>
> You also have this check after the driver updated its internal
> generation counter for a bogus completion, so now our actual state could
> be off from what's actually being prepared.
>
> This previous proposal for a similar problem was probably more on the
> right track:
>
> https://lore.kernel.org/linux-nvme/20260522153034.2168862-1-coshi036@gmail.com/
>

Thanks for your reply.
This solution is indeed more accurate.

> However, I'm really skeptical controllers actually behave this way. You
> wouldn't have a viable storage device if it was doing this.
>