Re: [PATCH v11 rdma-next 5/7] RDMA/qedr: Use the common mmap API

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Sep 23, 2019 at 09:21:37AM +0000, Michal Kalderon wrote:
> > From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> > owner@vger.kernel.org> On Behalf Of Gal Pressman
> > 
> > On 19/09/2019 20:55, Jason Gunthorpe wrote:
> > > Huh. If you recall we did all this work with the XA and the free
> > > callback because you said qedr was mmaping BAR pages that had some HW
> > > lifetime associated with them, and the HW resource was not to be
> > > reallocated until all users were gone.
> > >
> > > I think it would be a better example of this API if you pulled the
> > >
> > >  	dev->ops->rdma_remove_user(dev->rdma_ctx, ctx->dpi);
> > >
> > > Into qedr_mmap_free().
> > >
> > > Then the rdma_user_mmap_entry_remove() will call it naturally as it
> > > does entry_put() and if we are destroying the ucontext we already know
> > > the mmaps are destroyed.
> > >
> > > Maybe the same basic comment for EFA, not sure. Gal?
> > 
> > That's what EFA already does in this series, no?
> > We no longer remove entries on dealloc_ucontext, only when the entry is
> > freed.
> 
> Actually, I think most of the discussions you had on the topic were with Gal, but
> Some apply to qedr as well, however, for qedr, the only hw resource we allocate (bar)
> is on alloc_ucontext , therefore we were safe to free it on dealloc_ucontext as all mappings
> were already zapped. Making the mmap_free a bit redundant for qedr except for the need
> to free the entry. 

I think if we are changing things to use this new interface then you
should consistenly code so that the lifetime of the object in the
mmap_entry is entirely controlled by the mmap_entry and not also split
to the ucontext.

Having mmapable object lifetimes linked to the ucontext is a bit of a
hack really

Jason