Date:   Wed, 15 Jan 2020 15:51:04 -0400
From:   Jason Gunthorpe <jgg@ziepe.ca>
To:     Gal Pressman <galpress@amazon.com>
Cc:     Doug Ledford <dledford@redhat.com>, linux-rdma@vger.kernel.org,
        Alexander Matushevsky <matua@amazon.com>,
        Firas JahJah <firasj@amazon.com>,
        Yossi Leybovich <sleybo@amazon.com>
Subject: Re: [PATCH for-next 6/6] RDMA/efa: Do not delay freeing of DMA pages
Message-ID: <20200115195104.GA929@ziepe.ca>
References: <20200114085706.82229-1-galpress@amazon.com>
 <20200114085706.82229-7-galpress@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200114085706.82229-7-galpress@amazon.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-rdma-owner@vger.kernel.org
Precedence: bulk

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?

> +	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.

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.

Jason