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

Logan Gunthorpe <[email protected]>
Newsgroups org.kernel.vger.linux-rdma,org.infradead.lists.linux-nvme,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.linux-raid,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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.