Re: [PATCH for-next 6/6] RDMA/efa: Do not delay freeing of DMA pages

[Gal Pressman <galpress@amazon.com>]

On 15/01/2020 21:51, Jason Gunthorpe wrote:
> On Tue, Jan 14, 2020 at 10:57:06AM +0200, Gal Pressman wrote:
>> When destroying a DMA mmapped object, there is no need to delay the
>> pages freeing to dealloc_ucontext as the kernel itself will keep
>> reference count for these pages.
> 
> Why does the commit message talk about dealloc_ucontext but doesn't
> change dealloc_ucontext?

The commit message is wrong :\, we should not delay the free_pages_exact to the
mmap_free callback. We can "put" them on destroy flow and the pages will be
freed by the last consumer (either unmap or destroy).

> 
>> +	free_pages_exact(cq->cpu_addr, cq->size);
>> 	rdma_user_mmap_entry_remove(cq->mmap_entry);
> 
> This is out of order, the pages can't be freed until the entry is
> removed.

Right, I think the order is correct except rdma_user_mmap_entry_remove should be
moved to the beginning of the function.

> 
> There is also a bug in rdma_user_mmap_entry_remove(),
> entry->driver_removed needs to be set while holding the xa_lock or
> this is not the required fence.

I see you sent a patch, I'll take a look.
Thanks for the review.