> -----Original Message-----
> From: Jason Gunthorpe <j...@ziepe.ca>
> Sent: Wednesday, October 28, 2020 9:36 AM
> To: Xiong, Jianxin <jianxin.xi...@intel.com>
> Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford 
> <dledf...@redhat.com>; Leon Romanovsky
> <l...@kernel.org>; Sumit Semwal <sumit.sem...@linaro.org>; Christian Koenig 
> <christian.koe...@amd.com>; Vetter, Daniel
> <daniel.vet...@intel.com>
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to