> From: Jason Gunthorpe <jgg@xxxxxxxx> > Sent: Monday, September 23, 2019 4:30 PM > > External Email > > ---------------------------------------------------------------------- > On Mon, Sep 23, 2019 at 09:21:37AM +0000, Michal Kalderon wrote: > > > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- > > > owner@xxxxxxxxxxxxxxx> 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 > Ok, but the ucontext "hack" will need to remain for other drivers. > Jason
