[RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

On 03/14/2017 07:47 AM, Benjamin Gaignard wrote:

2017-03-13 22:09 GMT+01:00 Laura Abbott [off-list ref]:

quoted

On 03/12/2017 12:05 PM, Daniel Vetter wrote:

quoted

On Sun, Mar 12, 2017 at 2:34 PM, Benjamin Gaignard
[off-list ref] wrote:

quoted

2017-03-09 18:38 GMT+01:00 Laura Abbott [off-list ref]:

quoted

On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:

quoted

2017-03-06 17:04 GMT+01:00 Daniel Vetter [off-list ref]:

quoted

On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:

quoted

On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:

quoted

No one gave a thing about android in upstream, so Greg KH just dumped it
all into staging/android/. We've discussed ION a bunch of times, recorded
anything we'd like to fix in staging/android/TODO, and Laura's patch
series here addresses a big chunk of that.

quoted

This is pretty much the same approach we (gpu folks) used to de-stage the
syncpt stuff.

Well, there's also the fact that quite a few people have issues with the
design (like Laurent).  It seems like a lot of them have either got more
comfortable with it over time, or at least not managed to come up with
any better ideas in the meantime.

See the TODO, it has everything a really big group (look at the patch for
the full Cc: list) figured needs to be improved at LPC 2015. We don't just
merge stuff because merging stuff is fun :-)

Laurent was even in that group ...
-Daniel

For me those patches are going in the right direction.

I still have few questions:
- since alignment management has been remove from ion-core, should it
be also removed from ioctl structure ?

Yes, I think I'm going to go with the suggestion to fixup the ABI
so we don't need the compat layer and as part of that I'm also
dropping the align argument.

quoted

- can you we ride off ion_handle (at least in userland) and only
export a dma-buf descriptor ?

Yes, I think this is the right direction given we're breaking
everything anyway. I was debating trying to keep the two but
moving to only dma bufs is probably cleaner. The only reason
I could see for keeping the handles is running out of file
descriptors for dma-bufs but that seems unlikely.

quoted

In the future how can we add new heaps ?
Some platforms have very specific memory allocation
requirements (just have a look in the number of gem custom allocator in drm)
Do you plan to add heap type/mask for each ?

Yes, that was my thinking.

My concern is about the policy to adding heaps, will you accept
"customs" heap per
platforms ? per devices ? or only generic ones ?
If you are too strict, we will have lot of out-of-tree heaps and if
you accept of of them
it will be a nightmare to maintain....

I think ion should expose any heap that's also directly accessible to
devices using dma_alloc(_coherent). That should leave very few things
left, like your SMA heap.

quoted

Another point is how can we put secure rules (like selinux policy) on
heaps since all the allocations
go to the same device (/dev/ion) ? For example, until now, in Android
we have to give the same
access rights to all the process that use ION.
It will become problem when we will add secure heaps because we won't
be able to distinguish secure
processes to standard ones or set specific policy per heaps.
Maybe I'm wrong here but I have never see selinux policy checking an
ioctl field but if that
exist it could be a solution.

Hm, we might want to expose all the heaps as individual
/dev/ion_$heapname nodes? Should we do this from the start, since
we're massively revamping the uapi anyway (imo not needed, current
state seems to work too)?
-Daniel

I thought about that. One advantage with separate /dev/ion_$heap

Should we use /devi/ion/$heap instead of /dev/ion_$heap ?
I think it would be easier for user to look into one directory rather
then in whole /dev to find the heaps

If we decide to move away from /dev/ion we can consider it.

quoted

is that we don't have to worry about a limit of 32 possible
heaps per system (32-bit heap id allocation field). But dealing
with an ioctl seems easier than names. Userspace might be less
likely to hardcode random id numbers vs. names as well.

In the futur I think that heap type will be replaced by a "get caps"
ioctl which will
describe heap capabilities. At least that is my understanding of kernel part
of "unix memory allocator" project

I don't think it will be completely replaced. A heap can have multiple
capabilities so I suspect there will need to be some cap -> allocation
instance translation. Of course all this is wild speculation since
much of the unix memory allocator isn't well defined yet.

Thanks,
Laura