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

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Sep 23, 2019 at 09:30:54AM +0000, Michal Kalderon wrote:
> > From: Jason Gunthorpe <jgg@ziepe.ca>
> > Sent: Thursday, September 19, 2019 9:02 PM
> > 
> > External Email
> > 
> > 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.
> > 
> > I'm also not sure why qedr is using a phys_addr for a struct page object,
> > seems wrong.
> As mentioned in previous email, I misunderstood this part before. I'll move the free
> To object teardown. 
> What we need here is simply a shared page between kernel + user that both have
> Virtual pointers to, user writes to the page, kernel needs to read the data. 
> 
> The reason I used phys here is because the entry->address is defines as u64
> As it is common whether it is an address to the bar or a page... 
> Should I define a union based on the entry type ? and for a page use
> struct page object ? 

Yes, a union with a void * would be OK

Jason