diff mbox series

[for-next,2/2] RDMA/hns: Fix wrong maximum DMA segment size

Message ID 20250327114724.3454268-3-huangjunxian6@hisilicon.com (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series RDMA/hns: Cleanup and Bugfix | expand

Commit Message

Junxian Huang March 27, 2025, 11:47 a.m. UTC 
From: Chengchang Tang <tangchengchang@huawei.com>

Set maximum DMA segment size to 2G instead of UINT_MAX due to HW limit.

Fixes: e0477b34d9d1 ("RDMA: Explicitly pass in the dma_device to ib_register_device")
Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
---
 drivers/infiniband/hw/hns/hns_roce_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jason Gunthorpe April 1, 2025, 4:39 p.m. UTC | #1 
On Thu, Mar 27, 2025 at 07:47:24PM +0800, Junxian Huang wrote:
> @@ -763,7 +763,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>  		if (ret)
>  			return ret;
>  	}
> -	dma_set_max_seg_size(dev, UINT_MAX);
> +	dma_set_max_seg_size(dev, SZ_2G);

Are you sure? What do you think this does in the RDMA stack?

Jason
Junxian Huang April 2, 2025, 4:05 a.m. UTC | #2 
On 2025/4/2 0:39, Jason Gunthorpe wrote:
> On Thu, Mar 27, 2025 at 07:47:24PM +0800, Junxian Huang wrote:
>> @@ -763,7 +763,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>>  		if (ret)
>>  			return ret;
>>  	}
>> -	dma_set_max_seg_size(dev, UINT_MAX);
>> +	dma_set_max_seg_size(dev, SZ_2G);
> 
> Are you sure? What do you think this does in the RDMA stack?
> 

This is the maximum DMA segment size when mapping ULP's scatter/gather
list to DMA address, right?

In some cases, it is possible to map a DMA segment exceeding 2G. This
exceeds the limit of our HW.

Junxian

> Jason
Jason Gunthorpe April 4, 2025, 1:27 p.m. UTC | #3 
On Wed, Apr 02, 2025 at 12:05:36PM +0800, Junxian Huang wrote:
> 
> 
> On 2025/4/2 0:39, Jason Gunthorpe wrote:
> > On Thu, Mar 27, 2025 at 07:47:24PM +0800, Junxian Huang wrote:
> >> @@ -763,7 +763,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
> >>  		if (ret)
> >>  			return ret;
> >>  	}
> >> -	dma_set_max_seg_size(dev, UINT_MAX);
> >> +	dma_set_max_seg_size(dev, SZ_2G);
> > 
> > Are you sure? What do you think this does in the RDMA stack?
> > 
> 
> This is the maximum DMA segment size when mapping ULP's scatter/gather
> list to DMA address, right?

Yes, but only for ib_sge

But I think there are other possible problems if your HW cannot
implement the full ib_sge :\

Jason
Junxian Huang April 7, 2025, 1:51 p.m. UTC | #4 
On 2025/4/4 21:27, Jason Gunthorpe wrote:
> On Wed, Apr 02, 2025 at 12:05:36PM +0800, Junxian Huang wrote:
>>
>>
>> On 2025/4/2 0:39, Jason Gunthorpe wrote:
>>> On Thu, Mar 27, 2025 at 07:47:24PM +0800, Junxian Huang wrote:
>>>> @@ -763,7 +763,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>>>>  		if (ret)
>>>>  			return ret;
>>>>  	}
>>>> -	dma_set_max_seg_size(dev, UINT_MAX);
>>>> +	dma_set_max_seg_size(dev, SZ_2G);
>>>
>>> Are you sure? What do you think this does in the RDMA stack?
>>>
>>
>> This is the maximum DMA segment size when mapping ULP's scatter/gather
>> list to DMA address, right?
> 
> Yes, but only for ib_sge
> 
> But I think there are other possible problems if your HW cannot
> implement the full ib_sge :\

Why? According to IB spec, the maximum RDMA messgae size is 2GB,
so IMHO supporting ib_sge with DMA size larger than 2GB doesn't
seem to have much practical meaning.

Junxian

> 
> Jason
Jason Gunthorpe April 7, 2025, 1:54 p.m. UTC | #5 
On Mon, Apr 07, 2025 at 09:51:37PM +0800, Junxian Huang wrote:
> 
> 
> On 2025/4/4 21:27, Jason Gunthorpe wrote:
> > On Wed, Apr 02, 2025 at 12:05:36PM +0800, Junxian Huang wrote:
> >>
> >>
> >> On 2025/4/2 0:39, Jason Gunthorpe wrote:
> >>> On Thu, Mar 27, 2025 at 07:47:24PM +0800, Junxian Huang wrote:
> >>>> @@ -763,7 +763,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
> >>>>  		if (ret)
> >>>>  			return ret;
> >>>>  	}
> >>>> -	dma_set_max_seg_size(dev, UINT_MAX);
> >>>> +	dma_set_max_seg_size(dev, SZ_2G);
> >>>
> >>> Are you sure? What do you think this does in the RDMA stack?
> >>>
> >>
> >> This is the maximum DMA segment size when mapping ULP's scatter/gather
> >> list to DMA address, right?
> > 
> > Yes, but only for ib_sge
> > 
> > But I think there are other possible problems if your HW cannot
> > implement the full ib_sge :\
> 
> Why? According to IB spec, the maximum RDMA messgae size is 2GB,
> so IMHO supporting ib_sge with DMA size larger than 2GB doesn't
> seem to have much practical meaning.

I'm not sure we comprehensively check that everywhere..

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index ae24c81c9812..18a78f577c1b 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -763,7 +763,7 @@  static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
 		if (ret)
 			return ret;
 	}
-	dma_set_max_seg_size(dev, UINT_MAX);
+	dma_set_max_seg_size(dev, SZ_2G);
 	ret = ib_register_device(ib_dev, "hns_%d", dev);
 	if (ret) {
 		dev_err(dev, "ib_register_device failed!\n");