Date: Mon, 18 Jun 2018 15:02:55 +0200 From: Roger Pau =?utf-8?B?TW9ubsOp?= <royger@freebsd.org> To: Pratyush Yadav <pratyush@freebsd.org> Cc: FreeBSD-Xen <freebsd-xen@freebsd.org> Subject: Re: gnttab_end_foreign_access_ref() leaking grant entries? Message-ID: <20180618130255.3eflhi62m7wv7jpv@mac.bytemobile.com> In-Reply-To: <CA%2BX=3TKXTo6OjKXyhYwCBT=o5C_tXkuNf9OTQTcZLEW_gr0z%2BQ@mail.gmail.com> References: <CA%2BX=3TKXTo6OjKXyhYwCBT=o5C_tXkuNf9OTQTcZLEW_gr0z%2BQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jun 18, 2018 at 06:11:00PM +0530, Pratyush Yadav wrote: > Hi everyone, > > I was looking at gnttab_end_foreign_access_ref() and I notice that > while gnttab_end_foreign_access_ref() ends the foreign access, it does > not free the grant reference. > > gnttab_end_foreign_access() free the reference by calling put_free_entry(ref). > > gnttab_end_foreign_access_references() also frees the grant entries. > > Shouldn't gnttab_end_foreign_access_ref() also free the grant entry? > It is an inconsistency at best and a bug at worst. I think the point of using gnttab_end_foreign_access_ref is that it doesn't free the reference. The naming convention of grant reference related functions is just horrible, but I think this one is actually clear, it just removes the foreign access permissions of this reference, but it doesn't free it. I think I would rather rename gnttab_end_foreign_access to gnttab_end_foreign_access_and_free (and the same with the rest of the end_foreign_ family of functions). This is more descriptive. In any case, this is not a bug. Current netfront code makes use of this function in order to keep a pool of grant references, so by changing gnttab_end_foreign_access_ref to free the references you will break netfront. And now I realize this is something that you will have to take into account for the busdma grant reference project. You will have to introduce a flag that allows creating a dmamap with pre-allocated references that are not freed until the map is destroyed. But let's focus on that after the initial implementation is done. Roger.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180618130255.3eflhi62m7wv7jpv>