drivers/nvme/target/pci-epf.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
Have nvmet_req_init() and req->execute() complete failed commands.
Description of the problem:
nvmet_req_init() calls __nvmet_req_complete() internally upon failure,
e.g., unsupported opcode, which calls the "queue_response" callback,
this results in nvmet_pci_epf_queue_response() being called, which will
call nvmet_pci_epf_complete_iod() if data_len is 0 or if dma_dir is
different than DMA_TO_DEVICE. This results in a double completion as
nvmet_pci_epf_exec_iod_work() also calls nvmet_pci_epf_complete_iod()
when nvmet_req_init() fails.
Steps to reproduce:
On the host send a command with an unsupported opcode with nvme-cli,
For example the admin command "security receive"
$ sudo nvme security-recv /dev/nvme0n1 -n1 -x4096
This triggers a double completion as nvmet_req_init() fails and
nvmet_pci_epf_queue_response() is called, here iod->dma_dir is still
in the default state of "DMA_NONE" as set by default in
nvmet_pci_epf_alloc_iod(), so nvmet_pci_epf_complete_iod() is called.
Because nvmet_req_init() failed nvmet_pci_epf_complete_iod() is also
called in nvmet_pci_epf_exec_iod_work() leading to a doubple completion.
This patch lets nvmet_req_init() and req->execute() complete all failed
commands, and removes the double completion case in
nvmet_pci_epf_exec_iod_work() therefore fixing the edge cases where
double completions occurred.
Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
drivers/nvme/target/pci-epf.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index a4295a5b8d28..aad828eb72d6 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -1243,7 +1243,7 @@ static void nvmet_pci_epf_queue_response(struct nvmet_req *req)
iod->status = le16_to_cpu(req->cqe->status) >> 1;
/* If we have no data to transfer, directly complete the command. */
- if (!iod->data_len || iod->dma_dir != DMA_TO_DEVICE) {
+ if (iod->status || !iod->data_len || iod->dma_dir != DMA_TO_DEVICE) {
nvmet_pci_epf_complete_iod(iod);
return;
}
@@ -1604,8 +1604,13 @@ static void nvmet_pci_epf_exec_iod_work(struct work_struct *work)
goto complete;
}
+ /*
+ * If nvmet_req_init() fails (e.g., unsupported opcode) it will call
+ * __nvmet_req_complete() internally which will call
+ * nvmet_pci_epf_queue_response() and will complete the command directly.
+ */
if (!nvmet_req_init(req, &iod->sq->nvme_sq, &nvmet_pci_epf_fabrics_ops))
- goto complete;
+ return;
iod->data_len = nvmet_req_transfer_len(req);
if (iod->data_len) {
@@ -1643,10 +1648,11 @@ static void nvmet_pci_epf_exec_iod_work(struct work_struct *work)
wait_for_completion(&iod->done);
- if (iod->status == NVME_SC_SUCCESS) {
- WARN_ON_ONCE(!iod->data_len || iod->dma_dir != DMA_TO_DEVICE);
- nvmet_pci_epf_transfer_iod_data(iod);
- }
+ if (iod->status != NVME_SC_SUCCESS)
+ return;
+
+ WARN_ON_ONCE(!iod->data_len || iod->dma_dir != DMA_TO_DEVICE);
+ nvmet_pci_epf_transfer_iod_data(iod);
complete:
nvmet_pci_epf_complete_iod(iod);
--
2.25.1On 7/15/25 18:18, Rick Wertenbroek wrote:
> Have nvmet_req_init() and req->execute() complete failed commands.
>
> Description of the problem:
> nvmet_req_init() calls __nvmet_req_complete() internally upon failure,
> e.g., unsupported opcode, which calls the "queue_response" callback,
> this results in nvmet_pci_epf_queue_response() being called, which will
> call nvmet_pci_epf_complete_iod() if data_len is 0 or if dma_dir is
> different than DMA_TO_DEVICE. This results in a double completion as
> nvmet_pci_epf_exec_iod_work() also calls nvmet_pci_epf_complete_iod()
> when nvmet_req_init() fails.
>
> Steps to reproduce:
> On the host send a command with an unsupported opcode with nvme-cli,
> For example the admin command "security receive"
> $ sudo nvme security-recv /dev/nvme0n1 -n1 -x4096
>
> This triggers a double completion as nvmet_req_init() fails and
> nvmet_pci_epf_queue_response() is called, here iod->dma_dir is still
> in the default state of "DMA_NONE" as set by default in
> nvmet_pci_epf_alloc_iod(), so nvmet_pci_epf_complete_iod() is called.
> Because nvmet_req_init() failed nvmet_pci_epf_complete_iod() is also
> called in nvmet_pci_epf_exec_iod_work() leading to a doubple completion.
>
> This patch lets nvmet_req_init() and req->execute() complete all failed
> commands, and removes the double completion case in
> nvmet_pci_epf_exec_iod_work() therefore fixing the edge cases where
> double completions occurred.
>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
Forgot: this needs a fixes tag and cc-stable:
Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")
Cc: stable@vger.kernel.org
--
Damien Le Moal
Western Digital Research
On 7/15/25 18:18, Rick Wertenbroek wrote:
> Have nvmet_req_init() and req->execute() complete failed commands.
>
> Description of the problem:
> nvmet_req_init() calls __nvmet_req_complete() internally upon failure,
> e.g., unsupported opcode, which calls the "queue_response" callback,
> this results in nvmet_pci_epf_queue_response() being called, which will
> call nvmet_pci_epf_complete_iod() if data_len is 0 or if dma_dir is
> different than DMA_TO_DEVICE. This results in a double completion as
> nvmet_pci_epf_exec_iod_work() also calls nvmet_pci_epf_complete_iod()
> when nvmet_req_init() fails.
>
> Steps to reproduce:
> On the host send a command with an unsupported opcode with nvme-cli,
> For example the admin command "security receive"
> $ sudo nvme security-recv /dev/nvme0n1 -n1 -x4096
>
> This triggers a double completion as nvmet_req_init() fails and
> nvmet_pci_epf_queue_response() is called, here iod->dma_dir is still
> in the default state of "DMA_NONE" as set by default in
> nvmet_pci_epf_alloc_iod(), so nvmet_pci_epf_complete_iod() is called.
> Because nvmet_req_init() failed nvmet_pci_epf_complete_iod() is also
> called in nvmet_pci_epf_exec_iod_work() leading to a doubple completion.
>
> This patch lets nvmet_req_init() and req->execute() complete all failed
> commands, and removes the double completion case in
> nvmet_pci_epf_exec_iod_work() therefore fixing the edge cases where
> double completions occurred.
>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
> drivers/nvme/target/pci-epf.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
> index a4295a5b8d28..aad828eb72d6 100644
> --- a/drivers/nvme/target/pci-epf.c
> +++ b/drivers/nvme/target/pci-epf.c
> @@ -1243,7 +1243,7 @@ static void nvmet_pci_epf_queue_response(struct nvmet_req *req)
> iod->status = le16_to_cpu(req->cqe->status) >> 1;
>
> /* If we have no data to transfer, directly complete the command. */
Maybe change this comment to:
/*
* If the command failed or we have no data to transfer, complete
* the command immediately.
*/
Other than this, this looks good, so feel free to add:
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> - if (!iod->data_len || iod->dma_dir != DMA_TO_DEVICE) {
> + if (iod->status || !iod->data_len || iod->dma_dir != DMA_TO_DEVICE) {
> nvmet_pci_epf_complete_iod(iod);
> return;
> }
> @@ -1604,8 +1604,13 @@ static void nvmet_pci_epf_exec_iod_work(struct work_struct *work)
> goto complete;
> }
>
> + /*
> + * If nvmet_req_init() fails (e.g., unsupported opcode) it will call
> + * __nvmet_req_complete() internally which will call
> + * nvmet_pci_epf_queue_response() and will complete the command directly.
> + */
> if (!nvmet_req_init(req, &iod->sq->nvme_sq, &nvmet_pci_epf_fabrics_ops))
> - goto complete;
> + return;
>
> iod->data_len = nvmet_req_transfer_len(req);
> if (iod->data_len) {
> @@ -1643,10 +1648,11 @@ static void nvmet_pci_epf_exec_iod_work(struct work_struct *work)
>
> wait_for_completion(&iod->done);
>
> - if (iod->status == NVME_SC_SUCCESS) {
> - WARN_ON_ONCE(!iod->data_len || iod->dma_dir != DMA_TO_DEVICE);
> - nvmet_pci_epf_transfer_iod_data(iod);
> - }
> + if (iod->status != NVME_SC_SUCCESS)
> + return;
> +
> + WARN_ON_ONCE(!iod->data_len || iod->dma_dir != DMA_TO_DEVICE);
> + nvmet_pci_epf_transfer_iod_data(iod);
>
> complete:
> nvmet_pci_epf_complete_iod(iod);
--
Damien Le Moal
Western Digital Research
On Tue, Jul 15, 2025 at 11:18 AM Rick Wertenbroek
<rick.wertenbroek@gmail.com> wrote:
>
> Have nvmet_req_init() and req->execute() complete failed commands.
>
> Description of the problem:
> nvmet_req_init() calls __nvmet_req_complete() internally upon failure,
> e.g., unsupported opcode, which calls the "queue_response" callback,
> this results in nvmet_pci_epf_queue_response() being called, which will
> call nvmet_pci_epf_complete_iod() if data_len is 0 or if dma_dir is
> different than DMA_TO_DEVICE. This results in a double completion as
> nvmet_pci_epf_exec_iod_work() also calls nvmet_pci_epf_complete_iod()
> when nvmet_req_init() fails.
>
> Steps to reproduce:
> On the host send a command with an unsupported opcode with nvme-cli,
> For example the admin command "security receive"
> $ sudo nvme security-recv /dev/nvme0n1 -n1 -x4096
>
> This triggers a double completion as nvmet_req_init() fails and
> nvmet_pci_epf_queue_response() is called, here iod->dma_dir is still
> in the default state of "DMA_NONE" as set by default in
> nvmet_pci_epf_alloc_iod(), so nvmet_pci_epf_complete_iod() is called.
> Because nvmet_req_init() failed nvmet_pci_epf_complete_iod() is also
> called in nvmet_pci_epf_exec_iod_work() leading to a doubple completion.
>
s/doubple/double/
> This patch lets nvmet_req_init() and req->execute() complete all failed
> commands, and removes the double completion case in
> nvmet_pci_epf_exec_iod_work() therefore fixing the edge cases where
> double completions occurred.
>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
> drivers/nvme/target/pci-epf.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
> index a4295a5b8d28..aad828eb72d6 100644
> --- a/drivers/nvme/target/pci-epf.c
> +++ b/drivers/nvme/target/pci-epf.c
> @@ -1243,7 +1243,7 @@ static void nvmet_pci_epf_queue_response(struct nvmet_req *req)
> iod->status = le16_to_cpu(req->cqe->status) >> 1;
>
> /* If we have no data to transfer, directly complete the command. */
> - if (!iod->data_len || iod->dma_dir != DMA_TO_DEVICE) {
> + if (iod->status || !iod->data_len || iod->dma_dir != DMA_TO_DEVICE) {
> nvmet_pci_epf_complete_iod(iod);
> return;
> }
> @@ -1604,8 +1604,13 @@ static void nvmet_pci_epf_exec_iod_work(struct work_struct *work)
> goto complete;
> }
>
> + /*
> + * If nvmet_req_init() fails (e.g., unsupported opcode) it will call
> + * __nvmet_req_complete() internally which will call
> + * nvmet_pci_epf_queue_response() and will complete the command directly.
> + */
> if (!nvmet_req_init(req, &iod->sq->nvme_sq, &nvmet_pci_epf_fabrics_ops))
> - goto complete;
> + return;
>
> iod->data_len = nvmet_req_transfer_len(req);
> if (iod->data_len) {
> @@ -1643,10 +1648,11 @@ static void nvmet_pci_epf_exec_iod_work(struct work_struct *work)
>
> wait_for_completion(&iod->done);
>
> - if (iod->status == NVME_SC_SUCCESS) {
> - WARN_ON_ONCE(!iod->data_len || iod->dma_dir != DMA_TO_DEVICE);
> - nvmet_pci_epf_transfer_iod_data(iod);
> - }
> + if (iod->status != NVME_SC_SUCCESS)
> + return;
> +
> + WARN_ON_ONCE(!iod->data_len || iod->dma_dir != DMA_TO_DEVICE);
> + nvmet_pci_epf_transfer_iod_data(iod);
>
> complete:
> nvmet_pci_epf_complete_iod(iod);
> --
> 2.25.1
>
© 2016 - 2025 Red Hat, Inc.