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

[Jason Gunthorpe <jgg@ziepe.ca>]

On Sun, Oct 20, 2019 at 07:10:28PM -0700, Bart Van Assche wrote:
> The effect of the dma_set_max_seg_size() call in setup_dma_device() is
> as follows:
> - If device->dev.dma_parms is NULL, that call has no effect at all.
> - If device->dev.dma_parms is not NULL, since that pointer points at
>   the DMA parameters of the parent device, modify the DMA limits of the
>   parent device.
> 
> Both actions are wrong. Instead of changing the DMA parameters of the
> parent device, use the DMA parameters of the parent device if these
> parameters are available.
> 
> Compile-tested only.
> 
> Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Adit Ranadive <aditr@vmware.com>
> Cc: Shiraz Saleem <shiraz.saleem@intel.com>
> Cc: Gal Pressman <galpress@amazon.com>
> Cc: Selvin Xavier <selvin.xavier@broadcom.com>
> Fixes: d10bcf947a3e ("RDMA/umem: Combine contiguous PAGE_SIZE regions in SGEs")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>  drivers/infiniband/core/device.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 536310fb6510..b33d684a2a99 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -1199,9 +1199,18 @@ 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. Use the
> +			 * DMA parameters of the parent device.
> +			 */
> +			device->dev.dma_parms = parent->dma_parms;
> +		} else {
> +			WARN_ON_ONCE(true);
> +		}
> +	}
>  }

We really do want to, by default, increase the max_seg_size above 64k
though, so this approach doesn't seem right either.

Jason