Re: [PATCH v12 1/5] dma-buf: Add dma-buf heaps framework

[Brian Starkey]

On Fri, Oct 18, 2019 at 11:26:52AM -0700, John Stultz wrote:
> On Fri, Oct 18, 2019 at 4:18 AM Brian Starkey <Brian.Starkey@xxxxxxx> wrote:
> > On Fri, Oct 18, 2019 at 05:23:19AM +0000, John Stultz wrote:
> >
> > As in v3:
> >
> >  * Avoid EXPORT_SYMBOL until we finalize modules (suggested by
> >    Brian)
> 
> Heh. I guess it has been awhile.  :)
> 
> > Did something change in that regard? I still think letting modules
> > register heaps without a way to remove them is a recipe for issues.
> 
> So yea, in recent months, work around Android with their GKI effort
> has made it necessary for ION heaps to be loadable from modules. I had
> some patches in WIP tree to enable this, and in the rework I did
> yesterday for the CMA module trivially collided with parts, and
> forgetting the discussion back in v3, I figured I'd just fold those
> bits in before I resubmitted for v12.

Ah yes, I can see that would be needed.

> 
> If it's an issue, I can pull it out, but I'm going to be submitting
> module enablement for review as soon as the core bits are queued, as
> its going to be important to support for Android to switch to this
> from ION.
> 

I don't know how to decide if it's an issue. My understanding is that
if you rmmod something which has exported buffers, various Bad Things
might happen; I believe including data leaks, corruption or crashing
the kernel. There's probably plenty of scope for that with dma-buf
exporters already, so it's not exactly "new" but it is a bit
unfortunate.

If "people" are OK with adding new code which has the same issue, then
I'm not going to make any more of a fuss, because perfection is the
enemy of progress. Perhaps an obvious comment pointing out the issue
would be prudent, though - as a reminder to people that add heaps from
their code (and wonder why there's no "dma_heap_remove" function).

Cheers,
-Brian

> thanks
> -john