Re: [RFC PATCH v1 4/8] xprtrdma: return -EREMOTEIO on P2PDMA map failure

[Leon Romanovsky]

On Tue, Oct 27, 2026 at 11:57:25AM +0000, Pranjal Shrivastava wrote:
> On Mon, Oct 12, 2026 at 10:01:34AM +0300, Leon Romanovsky wrote:
> > On Tue, Oct 06, 2026 at 11:32:44PM +0000, Pranjal Shrivastava wrote:
> > > ib_dma_map_sg() returns 0 on any failure, so frwr_map() cannot tell
> > > why a mapping failed and reports -EIO. Use ib_dma_map_sgtable_attrs()
> > > instead, which returns the reason. Pass up -EREMOTEIO, which means
> > > the device cannot reach the P2PDMA memory, so the RPC fails with the
> > > same error as other P2PDMA refusals. Every other mapping failure
> > > still returns -EIO.
> > > 
> > > Signed-off-by: Pranjal Shrivastava <praan@xxxxxxxxxx>
> > > ---
> > >  net/sunrpc/xprtrdma/frwr_ops.c | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
> > > index e83cef19e656..a55d5e11c0e2 100644
> > > --- a/net/sunrpc/xprtrdma/frwr_ops.c
> > > +++ b/net/sunrpc/xprtrdma/frwr_ops.c
> > > @@ -293,7 +293,8 @@ int frwr_map(struct rpcrdma_xprt *r_xprt,
> > >  	bool sg_gaps = ep->re_mrtype == IB_MR_TYPE_SG_GAPS;
> > >  	unsigned int max_depth = ep->re_max_fr_depth;
> > >  	struct ib_reg_wr *reg_wr;
> > > -	int i, n, dma_nents;
> > > +	struct sg_table sgt;
> > > +	int i, n, dma_nents, ret;
> > >  	struct ib_mr *ibmr;
> > >  	u8 key;
> > >  
> > > @@ -381,10 +382,13 @@ int frwr_map(struct rpcrdma_xprt *r_xprt,
> > >  	mr->mr_dir = rpcrdma_data_dir(writing);
> > >  	mr->mr_nents = i;
> > >  
> > > -	dma_nents = ib_dma_map_sg(ep->re_id->device, mr->mr_sg, mr->mr_nents,
> > > -				  mr->mr_dir);
> > > -	if (!dma_nents)
> > > +	/* Unlike ib_dma_map_sg(), this reports why a mapping failed */
> > > +	sgt.sgl = mr->mr_sg;
> > > +	sgt.orig_nents = mr->mr_nents;
> > > +	ret = ib_dma_map_sgtable_attrs(ep->re_id->device, &sgt, mr->mr_dir, 0);
> > 
> > Please avoid using this function. We are gradually moving away from SG tables
> > in the RDMA code.
> > 
> > In general, it is never good thing to built API around try-and-fail
> > approach. It will require from us to be aware of "specific" usage for
> > the return value from one subsystem by another.
> > 
> 
> Fair, I'll drop ib_dma_map_sgtable_attrs(), keep ib_dma_map_sg() as it 
> is today, and check reachability explicitly before mapping instead.
> 
> rpcrdma_p2pdma_allowed() already asks ib_dma_pci_p2p_dma_supported()
> whether the device can do P2PDMA at all. It would also run
> pci_p2pdma_state() against the device's dma_device for the payload
> pages, and refuse the RPC with -EREMOTEIO on
> PCI_P2PDMA_MAP_NOT_SUPPORTED. A mapping failure then stays a plain
> -EIO, and nothing depends on the DMA layer's return value.
> 
> xprtrdma can be built as a module, though, and pci_p2pdma_state() is
> an inline wrapper around __pci_p2pdma_update_state(), which isn't
> exported. I see two ways to handle that: export
> __pci_p2pdma_update_state() as is, or add a small exported helper in
> p2pdma.c that returns the map type for a page and a device, e.g.
> pci_p2pdma_page_map_type(). Which would you prefer?

This isn't my decision to make, but my understanding is that these functions
should not be exported. They are intended for use by subsystem core code,
which should export the processed results to its local callers.

Thanks

> 
> Thanks,
> Praan
>