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 Jason
