> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Wednesday, October 28, 2020 9:36 AM
> To: Xiong, Jianxin <jianxin.xiong@xxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky
> <leon@xxxxxxxxxx>; Sumit Semwal <sumit.semwal@xxxxxxxxxx>; Christian Koenig <christian.koenig@xxxxxxx>; Vetter, Daniel
> <daniel.vetter@xxxxxxxxx>
> Subject: Re: [PATCH v6 4/4] RDMA/mlx5: Support dma-buf based userspace memory region
>
> On Tue, Oct 27, 2020 at 08:33:52PM +0000, Xiong, Jianxin wrote:
> > > > @@ -801,6 +816,52 @@ static int pagefault_implicit_mr(struct mlx5_ib_mr *imr,
> > > > * Returns:
> > > > * -EFAULT: The io_virt->bcnt is not within the MR, it covers pages that are
> > > > * not accessible, or the MR is no longer valid.
> > > > + * -EAGAIN: The operation should be retried
> > > > + *
> > > > + * >0: Number of pages mapped
> > > > + */
> > > > +static int pagefault_dmabuf_mr(struct mlx5_ib_mr *mr, struct ib_umem *umem,
> > > > + u64 io_virt, size_t bcnt, u32 *bytes_mapped,
> > > > + u32 flags)
> > > > +{
> > > > + struct ib_umem_dmabuf *umem_dmabuf = to_ib_umem_dmabuf(umem);
> > > > + u64 user_va;
> > > > + u64 end;
> > > > + int npages;
> > > > + int err;
> > > > +
> > > > + if (unlikely(io_virt < mr->mmkey.iova))
> > > > + return -EFAULT;
> > > > + if (check_add_overflow(io_virt - mr->mmkey.iova,
> > > > + (u64)umem->address, &user_va))
> > > > + return -EFAULT;
> > > > + /* Overflow has alreddy been checked at the umem creation time */
> > > > + end = umem->address + umem->length;
> > > > + if (unlikely(user_va >= end || end - user_va < bcnt))
> > > > + return -EFAULT;
> > >
> > > Why duplicate this sequence? Caller does it
> >
> > The sequence in the caller is for umem_odp only.
>
> Nothing about umem_odp in this code though??
The code in the caller uses ib_umem_end(odp) instead of the 'end' here, but we
can consolidate that with some minor changes.
>
> > > > /* prefetch with write-access must be supported by the MR */
> > > > if (advice == IB_UVERBS_ADVISE_MR_ADVICE_PREFETCH_WRITE &&
> > > > - !odp->umem.writable)
> > > > + !mr->umem->writable)
> > >
> > > ??
>
> > There is no need to use umem_odp here, mr->umem is the same as &odp->umem.
> > This change makes the code works for both umem_odp and umem_dmabuf.
>
> Ok
>
> Can you please also think about how to test this? I very much prefer to see new pyverbs tests for new APIs.
>
> Distros are running the rdma-core test suite, if you want this to work widely we need a public test for it.
>
Will look into that.
> Thanks,
> Jason
