Re: ib_umem_get and DMA_API_DEBUG question

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Aug 27, 2019 at 04:53:01PM +0300, Gal Pressman wrote:
> On 27/08/2019 16:37, Jason Gunthorpe wrote:
> > On Tue, Aug 27, 2019 at 04:22:51PM +0300, Gal Pressman wrote:
> >> On 27/08/2019 16:17, Jason Gunthorpe wrote:
> >>> On Tue, Aug 27, 2019 at 03:53:29PM +0300, Gal Pressman wrote:
> >>>> On 27/08/2019 15:00, Jason Gunthorpe wrote:
> >>>>> On Tue, Aug 27, 2019 at 11:28:20AM +0300, Gal Pressman wrote:
> >>>>>> On 26/08/2019 17:05, Gal Pressman wrote:
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> Lately I've been seeing DMA-API call traces on our automated testing runs which
> >>>>>>> complain about overlapping mappings of the same cacheline [1].
> >>>>>>> The problem is (most likely) caused due to multiple calls to ibv_reg_mr with the
> >>>>>>> same address, which as a result DMA maps the same physical addresses more than 7
> >>>>>>> (ACTIVE_CACHELINE_MAX_OVERLAP) times.
> >>>>>>
> >>>>>> BTW, on rare occasions I'm seeing the boundary check in check_sg_segment [1]
> >>>>>> fail as well. I don't have a stable repro for it though.
> >>>>>>
> >>>>>> Is this a known issue as well? The comment there states it might be a bug in the
> >>>>>> DMA API implementation, but I'm not sure.
> >>>>>>
> >>>>>> [1] https://elixir.bootlin.com/linux/v5.3-rc3/source/kernel/dma/debug.c#L1230
> >>>>>
> >>>>> Maybe we are missing a dma_set_seg_boundary ?
> >>>>>
> >>>>> PCI uses low defaults:
> >>>>>
> >>>>> 	dma_set_max_seg_size(&dev->dev, 65536);
> >>>>> 	dma_set_seg_boundary(&dev->dev, 0xffffffff);
> >>>>
> >>>> What would you set it to?
> >>>
> >>> Full 64 bits.
> >>>
> >>> For umem the driver is responsible to chop up the SGL as required, not
> >>> the core code.
> >>
> >> But wouldn't this possibly hide driver bugs? Perhaps even in other flows?
> > 
> > The block stack also uses this information, I've been meaning to check
> > if we should use dma_attrs in umem so we can have different
> > parameters.
> > 
> > I'm not aware of any issue with the 32 bit boundary on RDMA devices..
> 
> So something like this?
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 99c4a55545cf..2aa0e48f8dac 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -1199,8 +1199,9 @@ 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 */
> +	/* Setup default DMA properties for all IB devices */
>  	dma_set_max_seg_size(device->dma_device, SZ_2G);
> +	dma_set_seg_boundary(device->dma_device, U64_MAX);
>  
>  }

Hum. So there are two issues here, the SGL combiner in umem is
supposed to respect the DMA settings, so it should be fixed to check
boundary as well as seg_size too.

Then we can add the above as well. AFAIK all PCI-E HW is OK to do
arbitary DMAs.

Jason