mbox series

[RFC,0/2] Allow DMA BUF heaps to be loaded as modules

Message ID 20191025234834.28214-1-john.stultz@linaro.org
Headers show
Series Allow DMA BUF heaps to be loaded as modules | expand

Message

John Stultz Oct. 25, 2019, 11:48 p.m. UTC
Now that the DMA BUF heaps core code has been queued, I wanted
to send out some of the pending changes that I've been working
on.

For use with Android and their GKI effort, it is desired that
DMA BUF heaps are able to be loaded as modules. This is required
for migrating vendors off of ION which was also recently changed
to support modules.

So this patch series simply provides the necessary exported
symbols and allows the system and CMA drivers to be built
as modules.

Due to the fact that dmabuf's allocated from a heap may
be in use for quite some time, there isn't a way to safely
unload the driver once it has been loaded. Thus these
drivers do no implement module_exit() functions and will
show up in lsmod as "[permanent]"

Feedback and thoughts on this would be greatly appreciated!

thanks
-john

Cc: Laura Abbott <labbott@redhat.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Liam Mark <lmark@codeaurora.org>
Cc: Pratik Patel <pratikp@codeaurora.org>
Cc: Brian Starkey <Brian.Starkey@arm.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Yue Hu <huyue2@yulong.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Chenbo Feng <fengc@google.com>
Cc: Alistair Strachan <astrachan@google.com>
Cc: Sandeep Patil <sspatil@google.com>
Cc: Hridya Valsaraju <hridya@google.com>
Cc: dri-devel@lists.freedesktop.org

John Stultz (1):
  dma-buf: heaps: Allow system & cma heaps to be configured as a modules

Sandeep Patil (1):
  mm: cma: Export cma symbols for cma heap as a module

 drivers/dma-buf/dma-heap.c           | 2 ++
 drivers/dma-buf/heaps/Kconfig        | 4 ++--
 drivers/dma-buf/heaps/heap-helpers.c | 2 ++
 kernel/dma/contiguous.c              | 1 +
 mm/cma.c                             | 5 +++++
 5 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.17.1

Comments

Sandeep Patil Nov. 12, 2019, 12:49 a.m. UTC | #1
On Tue, Nov 05, 2019 at 09:41:44AM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> >

> > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:

> > > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> > > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:

> > > So even if the heaps are configured via DT (which at the moment there

> > > is no such binding, so that's not really a valid method yet), there's

> > > still the question of if the heap is necessary/makes sense on the

> > > device. And the DT would only control the availability of the heap

> > > interface, not if the heap driver is loaded or not.

> >

> > Hm I thought the cma regions are configured in DT? How does that work if

> > it's not using DT?

> 

> So yea, CMA regions are either configured by DT or setup at build time

> (I think there's a cmdline option to set it up as well).

> 

> But the CMA regions and the dmabuf cma heap driver are separate

> things. The latter uses the former.

> 

> > > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers

> > > for the display framebuffer. So gralloc uses ION to allocate from the

> > > CMA heap. However on the db845c, it has no such restrictions, so the

> > > CMA heap isn't necessary.

> >

> > Why do you have a CMA region for the 2nd board if you don't need it?

> > _That_ sounds like some serious memory waster, not a few lines of code

> > loaded for nothing :-)

> 

> ??? That's not what I said above.  If the db845c doesn't need CMA it

> won't have a CMA region.

> 

> The issue at hand is that we may want to avoid loading the dmabuf CMA

> heap driver on a board that doesn't use CMA.

> 

> 

> > > With Android's GKI effort, there needs to be one kernel that works on

> > > all the devices, and they are using modules to try to minimize the

> > > amount of memory spent on functionality that isn't universally needed.

> > > So on devices that don't need the CMA heap, they'd probably prefer not

> > > to load the CMA dmabuf heap driver, so it would be best if it could be

> > > built as a module.  If we want to build the CMA heap as a module, the

> > > symbols it uses need to be exported.

> >

> > Yeah, I guess I'm disagreeing on whether dma-buf heaps are core or not.

> 

> That's fine to dispute. I'm not really in a place to assert one way or

> not, but the Android folks have made their ION system and CMA heaps

> loadable via a module, so it would seem like having the dmabuf system

> and CMA heaps be modular would be useful to properly replace that

> usage.

> 

> For instance, the system heap as a module probably doesn't make much

> sense, as most boards that want to use the dmabuf heaps interface are

> likely to use that as well.  CMA is more optional as not all boards

> will use that one, so it might make sense to avoid loading it.

> 

> Sandeep: Can you chime in here as to how critical having the system

> and cma heaps be modules are?


With ion, we are making sure there are *standard* heaps that Android should
be able to rely on to exist in all kernels [1]. That list is based on what
default heaps ion had out-of-tree.

As of today, even from those that ion had, Android vendor independent code only
relies on 'system heap' and 'cma/dma heaps' so, can safely ignore the carveout
and other ion heaps.

system heap is really the one that is realistically 'hardware independent',
so that can be in kernel. The cma heaps and their existence is optional, so
it will be nice to be able to load them as modules.


<snip>

- ssp

1. https://android.googlesource.com/platform/system/core/+/refs/heads/master/libion/kernel-headers/linux/ion_4.19.h#28
Sandeep Patil Nov. 12, 2019, 12:56 a.m. UTC | #2
On Tue, Nov 05, 2019 at 11:47:53AM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 11:19 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> > On Tue, Nov 5, 2019 at 6:41 PM John Stultz <john.stultz@linaro.org> wrote:

> > > On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> > > >

> > > > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:

> > > > > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> > > > > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:

> > > > > So even if the heaps are configured via DT (which at the moment there

> > > > > is no such binding, so that's not really a valid method yet), there's

> > > > > still the question of if the heap is necessary/makes sense on the

> > > > > device. And the DT would only control the availability of the heap

> > > > > interface, not if the heap driver is loaded or not.

> > > >

> > > > Hm I thought the cma regions are configured in DT? How does that work if

> > > > it's not using DT?

> > >

> > > So yea, CMA regions are either configured by DT or setup at build time

> > > (I think there's a cmdline option to set it up as well).

> > >

> > > But the CMA regions and the dmabuf cma heap driver are separate

> > > things. The latter uses the former.

> >

> > Huh, I assumed the plan is that whenever there's a cma region, we want

> > to instantiate a dma-buf heap for it? Why/when would we not want to do

> > that?

> 

> Not quite. Andrew noted that we may not want to expose all CMA regions

> via dmabuf heaps, so right now we only expose the default region. I

> have follow on patches that I sent out earlier (which requires a

> to-be-finalized DT binding) which allows us to specify which other CMA

> regions to expose.

> 

> > > > > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers

> > > > > for the display framebuffer. So gralloc uses ION to allocate from the

> > > > > CMA heap. However on the db845c, it has no such restrictions, so the

> > > > > CMA heap isn't necessary.

> > > >

> > > > Why do you have a CMA region for the 2nd board if you don't need it?

> > > > _That_ sounds like some serious memory waster, not a few lines of code

> > > > loaded for nothing :-)

> > >

> > > ??? That's not what I said above.  If the db845c doesn't need CMA it

> > > won't have a CMA region.

> > >

> > > The issue at hand is that we may want to avoid loading the dmabuf CMA

> > > heap driver on a board that doesn't use CMA.

> >

> > So the CMA core code is also a module, or how does that work? Not

> 

> No.  CMA core isn't available as a module.

> 

> > loading the cma dma-buf heap, but keeping all the other cma code

> > around feels slightly silly. Do we have numbers that justify this

> > silliness?

> 

> I agree that is maybe not the most critical item on the list, but its

> one of many that do add up over time.

> 

> Again, I'll defer to Sandeep or other folks on the Google side to help

> with the importance here. Mostly I'm trying to ensure there is

> functional parity to ION so we don't give folks any reason they could

> object to deprecating it.


Parity with ION will definitely be nice. For now, however, even if we achieve
that parity with UAPI and think about the cma-heap-as-module bit later, I
guess that's ok.

The real problem is the need for these heaps to be a module in the first
place.  I'd much rather have an upstream user to show the need for cache
maintenance operations that have been talked about so many times, so we can
make them happen for dma-buf-heaps in upstream. None of this has to be
a module if that happens :(.

The reason for the "modularization"  for ion heaps is also the CMOs for
Android use cases. Unfortunately we haven't had any luck with proving the
need for. John, CMIIW.


- ssp