[PATCH 2/2] nvme: avoid possible double completions for the same request
yebin
yebin at huaweicloud.com
Tue Jun 9 01:56:07 PDT 2026
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.
>
More information about the Linux-nvme
mailing list