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

From: Michal Kalderon <mkalderon@marvell.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Ariel Elior <aelior@marvell.com>,
	"dledford@redhat.com" <dledford@redhat.com>,
	"bmt@zurich.ibm.com" <bmt@zurich.ibm.com>,
	"galpress@amazon.com" <galpress@amazon.com>,
	"sleybo@amazon.com" <sleybo@amazon.com>,
	"leon@kernel.org" <leon@kernel.org>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: RE: [EXT] Re: [PATCH v11 rdma-next 6/7] RDMA/qedr: Add doorbell overflow recovery support
Date: Mon, 23 Sep 2019 09:30:54 +0000	[thread overview]
Message-ID: <MN2PR18MB318246963F0BD4E2FB7459F0A1850@MN2PR18MB3182.namprd18.prod.outlook.com> (raw)
In-Reply-To: <20190919180224.GE4132@ziepe.ca>

> 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 ? 

> 
> Jason