> From: Jason Gunthorpe <jgg@xxxxxxxx> > 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