On 21/10/2019 20:33, Jason Gunthorpe wrote: > 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.. Dealloc UAR is currently being called during dealloc_ucontext. The mmap_free callback is per entry, how can dealloc_uar be moved there?
