Re: [PATCH rdma-next 2/6] RDMA/umem: Add ODP type indicator within ib_umem_odp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 07, 2019 at 09:35:10AM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 07, 2019 at 03:13:35PM +0300, Leon Romanovsky wrote:
> > On Wed, Aug 07, 2019 at 11:44:16AM +0000, Jason Gunthorpe wrote:
> > > On Wed, Aug 07, 2019 at 01:33:59PM +0300, Leon Romanovsky wrote:
> > > > From: Erez Alfasi <ereza@xxxxxxxxxxxx>
> > > >
> > > > ODP type can be divided into 2 subclasses:
> > > > Explicit and Implicit ODP.
> > > >
> > > > Adding a type enums and an odp type flag within
> > > > ib_umem_odp will give us an indication whether a
> > > > given MR is ODP implicit/explicit registered.
> > > >
> > > > Signed-off-by: Erez Alfasi <ereza@xxxxxxxxxxxx>
> > > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > >  drivers/infiniband/core/umem.c    |  1 +
> > > >  include/rdma/ib_umem_odp.h        | 14 ++++++++++++++
> > > >  include/uapi/rdma/ib_user_verbs.h |  5 +++++
> > > >  3 files changed, 20 insertions(+)
> > >
> > > No for this patch, I've got a series cleaning up this
> > > implicit/explicit nonsense, and the result is much cleaner than this.
> >
> > It doesn't really clean anything, just stores implicit/explicit information.
> >
> > >
> > > This series will have to wait.
> >
> > The information exposed in this series is already available in uverbs,
> > so whatever cleanup will come, we still need to expose ODP MR type.
> >
> > This patch is tiny part of whole series, why should we block whole
> > series and iproute2?
>
> This whole approach is really ugly, I even object to the very idea of
> patch #1

How did patch #1 relate? It simply removed same code from drivers and
put it in one place.

>
> The umem is an internal detail and should not be exposed out of the
> driver for nldev to use.

We are exporting ODP MR property, users are not aware of umem thing
underneath at all.

Thanks

>
> Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux