Re: [PATCH v5 10/10] nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers

Logan Gunthorpe <[email protected]> Fri, 24 Jul 2026 12:50:18 -0600
Newsgroups org.kernel.vger.linux-raid,org.infradead.lists.linux-nvme,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.linux-rdma,org.kernel.vger.stable
Message-ID <[email protected]>

On 2026-07-23 2:42 p.m., Mykola Marzhan wrote:
> A P2P transfer the PCIe topology cannot route fails DMA mapping
> with -EREMOTEIO.  nvme-rdma folds every mapping error into -EIO, a
> retryable host-path error: multipath requeues the I/O forever, a
> single path burns its whole retry budget.
> 
> Propagate the real error code and return the unroutable case as the
> non-retryable BLK_STS_P2PDMA.  As in nvme-pci, -ENOMEM now requeues
> (BLK_STS_RESOURCE) and -EINVAL fails (BLK_STS_IOERR); -EIO stays a
> retryable host-path error.  While at it, start the request only
> after mapping succeeds -- nvme-pci's order -- and ratelimit the
> map-failure message.
> 
> Fixes: 23528aa3320a ("nvme: enable PCI P2PDMA support for RDMA transport")
> Cc: [email protected] # v7.1: requires "nvme-rdma: use ib_dma_map_sgtable_attrs()"
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Mykola Marzhan <[email protected]>

This is much more understandable now. Thanks. I have one nit below,
other than that:

Reviewed-by: Logan Gunthorpe <[email protected]>

> ---
>  drivers/nvme/host/rdma.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 63830334c73e..2f50509a7a61 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -1486,10 +1486,8 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
>  		.orig_nents	= req->data_sgl.nents,
>  	};
>  	ret = ib_dma_map_sgtable_attrs(ibdev, &sgt, rq_dma_dir(rq), 0);
> -	if (unlikely(ret)) {
> -		ret = -EIO;
> +	if (unlikely(ret))
>  		goto out_free_table;
> -	}
>  	*count = sgt.nents;
>  
>  	if (blk_integrity_rq(rq)) {
> @@ -1511,10 +1509,8 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
>  			.orig_nents	= req->metadata_sgl->nents,
>  		};
>  		ret = ib_dma_map_sgtable_attrs(ibdev, &sgt, rq_dma_dir(rq), 0);
> -		if (unlikely(ret)) {
> -			ret = -EIO;
> +		if (unlikely(ret))
>  			goto out_free_pi_table;
> -		}
>  		*pi_count = sgt.nents;
>  	}
>  
> @@ -2033,8 +2029,6 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	if (ret)
>  		goto unmap_qe;
>  
> -	nvme_start_request(rq);
> -
>  	if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) &&
>  	    queue->pi_support &&
>  	    (c->common.opcode == nvme_cmd_write ||
> @@ -2046,11 +2040,13 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
>  
>  	err = nvme_rdma_map_data(queue, rq, c);
>  	if (unlikely(err < 0)) {
> -		dev_err(queue->ctrl->ctrl.device,
> -			     "Failed to map data (%d)\n", err);
> +		dev_err_ratelimited(queue->ctrl->ctrl.device,
> +				    "Failed to map data (%d)\n", err);

This rate limit change probably doesn't belong in this patch. It's
pretty clear in the commit message with the "while at it" message. Worth
moving to another patch.

Logan