On Thu, Nov 05, 2020 at 12:36:25AM +0000, Xiong, Jianxin wrote: > > From: Jason Gunthorpe <[email protected]> > > Sent: Wednesday, November 04, 2020 4:07 PM > > To: Xiong, Jianxin <[email protected]> > > Cc: [email protected]; [email protected]; Doug > > Ledford <[email protected]>; Leon Romanovsky > > <[email protected]>; Sumit Semwal <[email protected]>; Christian Koenig > > <[email protected]>; Vetter, Daniel > > <[email protected]> > > Subject: Re: [PATCH v7 4/5] RDMA/mlx5: Support dma-buf based userspace > > memory region > > > > On Wed, Nov 04, 2020 at 02:06:34PM -0800, Jianxin Xiong wrote: > > > + umem = ib_umem_dmabuf_get(&dev->ib_dev, offset, length, fd, > > > access_flags, > > > + &mlx5_ib_dmabuf_attach_ops); > > > + if (IS_ERR(umem)) { > > > + mlx5_ib_dbg(dev, "umem get failed (%ld)\n", PTR_ERR(umem)); > > > + return ERR_PTR(PTR_ERR(umem)); > > > + } > > > + > > > + mr = alloc_mr_from_cache(pd, umem, virt_addr, access_flags); > > > > It is very subtle, but this calls mlx5_umem_find_best_pgsz() which calls > > ib_umem_find_best_pgsz() which goes over the SGL to determine > > the page size to use. > > > > When this is called here, the umem sglist is still NULL because > dma_buf_map_attachment() > is not called until a page fault occurs. In patch 1/5, the function > ib_umem_find_best_pgsz() > has been modified to always return PAGE_SIZE for dma-buf based MR.
Oh.. That isn't a good idea. ib_umem_find_best_pgsz() must be run on any SGL list to validate it against the constraints, making it un-runable for the dmabuf case means we can never support large page size or even validate that the SGL is properly formed. So I think this need to change the alloc_mr_from_cache() to early exit for dma_buf ones And it still need to call ib_umem_find_best_pgsz() but just check the page size. > > Edit the last SGE to have a reduced length > > Do you still think modifying the SGL in place needed given the above > explanation? I do see some benefits of doing so -- hiding the > discrepancy of sgl and addr/length from the device drivers and avoid > special handling in the code that use the sgl. Yes, a umem SGL should always be properly formed or I will have a meltdown trying to keep all the drivers working :\ Jason _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
