Re: [PATCH 2/4] RDMA/core: Set DMA parameters correctly

[Bart Van Assche <bvanassche@acm.org>]

On 10/21/19 10:44 AM, Saleem, Shiraz wrote:
>> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
>> index a667636f74bf..a523d844ad9d 100644
>> --- a/drivers/infiniband/core/device.c
>> +++ b/drivers/infiniband/core/device.c
>> @@ -1199,9 +1199,21 @@ static void setup_dma_device(struct ib_device *device)
>>    		WARN_ON_ONCE(!parent);
>>    		device->dma_device = parent;
>>    	}
>> -	/* Setup default max segment size for all IB devices */
>> -	dma_set_max_seg_size(device->dma_device, SZ_2G);
>>
>> +	if (!device->dev.dma_parms) {
>> +		if (parent) {
>> +			/*
>> +			 * The caller did not provide DMA parameters, so
>> +			 * 'parent' probably represents a PCI device. The PCI
>> +			 * core sets the maximum segment size to 64
>> +			 * KB. Increase this parameter to 2G.
>> +			 */
>> +			device->dev.dma_parms = parent->dma_parms;
>> +			dma_set_max_seg_size(device->dma_device, SZ_2G);
> 
> Did you mean dma_set_max_seg_size(&device->dev, SZ_2G)?

Have you realized that that call has the same effect as what I proposed 
since both devices share the dma_parms parameter?

> device->dma_device could be pointing to parent if the caller
> did not provide dma_ops. So wont this update the parent device
> dma params?

That's correct, this will update the parent device DMA parameters.

> Also do we want to ensure all callers device max_seg_sz
> params >= threshold (=2G)? If so, perhaps we can do something
> similar to vb2_dma_contig_set_max_seg_size()
> 
> https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/media/common/videobuf2/videobuf2-dma-contig.c#L734

It depends on what PCIe RDMA adapters support. If all PCIe RDMA adapters 
supported by the Linux kernel support max_segment_size >= 2G the above 
code is probably the easiest approach.

Thanks,

Bart.