RE: [PATCH v11 rdma-next 6/7] RDMA/qedr: Add doorbell overflow recovery support

[Michal Kalderon <mkalderon@marvell.com>]

> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> 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 ?