On Sun, Oct 20, 2019 at 10:19:34AM +0300, Gal Pressman wrote: > On 24/09/2019 12:31, Michal Kalderon wrote: > >> From: Pressman, Gal <galpress@xxxxxxxxxx> > >> Sent: Tuesday, September 24, 2019 11:50 AM > >> > >> > >>> On 23 Sep 2019, at 18:22, Michal Kalderon <mkalderon@xxxxxxxxxxx> > >> 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. > >>> > >>> For EFA, it seemed the only operation delayed was freeing memory - I > >>> didn't see hw resources being freed... Gal? > >> > >> What do you mean by hw resources being freed? The BAR mappings are > >> under the device’s control and are associated to the lifetime of the UAR. > > The bar offset you get is from the device -> don't you release it back to the device > > So it can be allocated to a different application ? > > In efa_com_create_qp -> you get offsets from the device that you use for mapping > > The bar -> are these unique for every call ? are they released during destroy_qp ? > > Before this patch series mmap_entries_remove_free only freed the DMA pages, but > > Following this thread, it seemed the initial intention was that only the hw resources would > > Be delayed as the DMA pages are ref counted anyway. I didn't see any delay to returning > > The bar offsets to the device. Thanks. > The BAR pages are being "freed" back to the device once the UAR is freed. > These pages' lifetime is under the control of the device so there's nothing the > driver needs to do, just make sure no one else is using them. What frees the UAR? In the mlx drivers this was done during destruction of the ucontext, but with this new mmap stuff it could be moved to the mmap_free.. Jason
