> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- > owner@xxxxxxxxxxxxxxx> On Behalf Of Gal Pressman > > On 20/09/2019 16:38, Jason Gunthorpe wrote: > > On Fri, Sep 20, 2019 at 04:30:52PM +0300, Gal Pressman wrote: > >> On 19/09/2019 21:02, Jason Gunthorpe wrote: > >>> On Thu, Sep 05, 2019 at 01:01:16PM +0300, Michal Kalderon wrote: > >>> > >>>> @@ -347,6 +360,9 @@ void qedr_mmap_free(struct > rdma_user_mmap_entry > >>>> *rdma_entry) { > >>>> struct qedr_user_mmap_entry *entry = > >>>> get_qedr_mmap_entry(rdma_entry); > >>>> > >>>> + if (entry->mmap_flag == QEDR_USER_MMAP_PHYS_PAGE) > >>>> + free_page((unsigned long)phys_to_virt(entry->address)); > >>>> + > >>> > >>> While it isn't wrong it do it this way, we don't need this > >>> mmap_free() stuff for normal CPU pages. Those are refcounted and > >>> qedr can simply call free_page() during the teardown of the uobject > >>> that is using the this page. This is what other drivers already do. > >> > >> This is pretty much what EFA does as well. When we allocate pages > >> for the user (CQ for example), we DMA map them and later on mmap > them > >> to the user. We expect those pages to remain until the entry is > >> freed, how can we call free_page, who is holding a refcount on those > >> except for the driver? > > > > free_page is kind of a lie, it is more like put_page (see > > __free_pages). I think the difference is that it assumes the page came > > from alloc_page and skips some generic stuff when freeing it. > > > > When the driver does vm_insert_page the vma holds another refcount and > > the refcount does not go to zero until that page drops out of the vma > > (ie at the same time mmap_free above is called). > > > > Then __put_page will do the free_unref_page(), etc. > > > > So for CPU pages it is fine to not use mmap_free so long as > > vm_insert_page is used > > Thanks, I did not know this, it simplifies things. > In that case, maybe the mmap_free callback is redundant. We still need it to free the entry though - right ? Gal, where do you free your hardware resources ?