Re: [PATCH 00/14] Misc ION cleanups and adding unmapped heap

From: Laura Abbott
Date: Mon Jan 14 2019 - 21:41:27 EST


On 1/11/19 10:05 AM, Andrew F. Davis wrote:
Hello all,

This is a set of (hopefully) non-controversial cleanups for the ION
framework and current set of heaps. These were found as I start to
familiarize myself with the framework to help in whatever way I
can in getting all this up to the standards needed for de-staging.

I would like to get some ideas of what is left to work on to get ION
out of staging. Has there been some kind of agreement on what ION should
eventually end up being? To me it looks like it is being whittled away at
to it's most core functions. To me that is looking like being a DMA-BUF
user-space front end, simply advertising available memory backings in a
system and providing allocations as DMA-BUF handles. If this is the case
then it looks close to being ready to me at least, but I would love to
hear any other opinions and concerns.


Yes, at this point the only functionality that people are really
depending on is the ability to allocate a dma_buf easily from userspace.

Back to this patchset, the last patch may be a bit different than the
others, it adds an unmapped heaps type and creation helper. I wanted to
get this in to show off another heap type and maybe some issues we may
have with the current ION framework. The unmapped heap is used when the
backing memory should not (or cannot) be touched. Currently this kind
of heap is used for firewalled secure memory that can be allocated like
normal heap memory but only used by secure devices (OP-TEE, crypto HW,
etc). It is basically just copied from the "carveout" heap type with the
only difference being it is not mappable to userspace and we do not clear
the memory (as we should not map it either). So should this really be a
new heap type? Or maybe advertised as a carveout heap but with an
additional allocation flag? Perhaps we do away with "types" altogether
and just have flags, coherent/non-coherent, mapped/unmapped, etc.

Maybe more thinking will be needed afterall..


So the cleanup looks okay (I need to finish reviewing) but I'm not a
fan of adding another heaptype without solving the problem of adding
some sort of devicetree binding or other method of allocating and
placing Ion heaps. That plus uncached buffers are one of the big
open problems that need to be solved for destaging Ion. See
https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
for some background on that problem.

Thanks,
Laura

Thanks,
Andrew

Andrew F. Davis (14):
staging: android: ion: Add proper header information
staging: android: ion: Remove empty ion_ioctl_dir() function
staging: android: ion: Merge ion-ioctl.c into ion.c
staging: android: ion: Remove leftover comment
staging: android: ion: Remove struct ion_platform_heap
staging: android: ion: Fixup some white-space issues
staging: android: ion: Sync comment docs with struct ion_buffer
staging: android: ion: Remove base from ion_carveout_heap
staging: android: ion: Remove base from ion_chunk_heap
staging: android: ion: Remove unused headers
staging: android: ion: Allow heap name to be null
staging: android: ion: Declare helpers for carveout and chunk heaps
staging: android: ion: Do not sync CPU cache on map/unmap
staging: android: ion: Add UNMAPPED heap type and helper

drivers/staging/android/ion/Kconfig | 10 ++
drivers/staging/android/ion/Makefile | 3 +-
drivers/staging/android/ion/ion-ioctl.c | 98 --------------
drivers/staging/android/ion/ion.c | 93 +++++++++++--
drivers/staging/android/ion/ion.h | 87 ++++++++-----
.../staging/android/ion/ion_carveout_heap.c | 19 +--
drivers/staging/android/ion/ion_chunk_heap.c | 25 ++--
drivers/staging/android/ion/ion_cma_heap.c | 6 +-
drivers/staging/android/ion/ion_heap.c | 8 +-
drivers/staging/android/ion/ion_page_pool.c | 2 +-
drivers/staging/android/ion/ion_system_heap.c | 8 +-
.../staging/android/ion/ion_unmapped_heap.c | 123 ++++++++++++++++++
drivers/staging/android/uapi/ion.h | 3 +
13 files changed, 307 insertions(+), 178 deletions(-)
delete mode 100644 drivers/staging/android/ion/ion-ioctl.c
create mode 100644 drivers/staging/android/ion/ion_unmapped_heap.c