Re: [RFC][PATCH 0/6] ion: improved ABI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laura,

On Mon, Jun 06, 2016 at 11:23:27AM -0700, Laura Abbott wrote:
The ABI for Ion's ioctl interface are a pain to work with. The heap IDs
are a 32-bit non-discoverable namespace that form part of the ABI. There's
no way to determine what ABI version is in use which leads to problems
if the ABI changes or needs to be updated.

This series is a first approach to give a better ABI for Ion. This includes:

- Following the advice in botching-up-ioctls.txt
- Ioctl for ABI version
- Dynamic assignment of heap ids
- queryable heap ids
- Runtime mapping of heap ids, including fallbacks. This avoids the need to
 encode the fallbacks as an ABI.

I'm most interested in feedback if this ABI is actually an improvement and
usable. The heap id map/query interface seems error prone but I didn't have
a cleaner solution. There aren't any kernel APIs for the new features as the
focus was on a userspace API but I anticipate that following easily once
the userspace API is established.


If I understand it right, the big improvement here is that userspace can
find out what heap IDs are available, and what type of heap they
correspond to? This seems good.

I'm not sure about how userspace is meant to use the usage mappings
though. Who calls ION_IOC_ID_MAP?

(also, map/mapping is pretty overloaded. How about groups/groupings?)

If I assume that the thing calling ION_IOC_ID_MAP is the same thing
doing the allocating, then there doesn't seem to be much need for
creating mappings. The combined mapper/allocator would necessarily need
some knowledge about which types can satisfy which usage, and so could
follow something like this:
 1. The heaps can be queried, finding their IDs and types
 2. A mask of heap IDs can be created which satisfies a "usage", based
    on the queried types
 3. Allocation operations can then simply use this constructed mask

On the other hand, if I assume that the thing doing the allocating is
different to the thing doing the usage mapping (i.e. the allocator
doesn't need to know about heap _types_, only usages); then I can't see
a way for the allocator to figure out which usage_id it's meant to
allocate with - which puts it right back to the old problem of opaque
heap IDs (-> opaque usage_ids), except it's worse because they can
change dynamically.

Thanks,
Brian


Thanks,
Laura

P.S. Not to turn this into a bike shedding session but if you have suggestions
for a name for this framework other than Ion I would be interested to hear
them. Too many other things are already named Ion.

Laura Abbott (6):
 staging: android: ion: return error value for ion_device_add_heap
 staging: android: ion: Switch to using an idr to manage heaps
 staging: android: ion: Drop heap type masks
 staging: android: ion: Pull out ion ioctls to a separate file
 staging: android: ion: Add an ioctl for ABI checking
 staging: android: ion: Introduce new ioctls for dynamic heaps

drivers/staging/android/ion/Makefile    |   3 +-
drivers/staging/android/ion/ion-ioctl.c | 243 ++++++++++++++++++
drivers/staging/android/ion/ion.c       | 438 ++++++++++++++++----------------
drivers/staging/android/ion/ion_priv.h  | 109 +++++++-
drivers/staging/android/uapi/ion.h      | 164 +++++++++++-
5 files changed, 728 insertions(+), 229 deletions(-)
create mode 100644 drivers/staging/android/ion/ion-ioctl.c

-- 2.5.5
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux