Re: [RFC][PATCH 0/6] ion: improved ABI
On 06/08/2016 08:15 AM, Brian Starkey wrote: 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? The thought was daemons (surfaceflinger, mediaserver etc.) would set this up. (also, map/mapping is pretty overloaded. How about groups/groupings?) That's a good suggestion. 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. I see your point about the mapping. My thought was that whoever was doing the mapping would have a way to pass information to the allocator or the allocator could do a query. Relying on the passing of information may not be plausible and I realize there isn't enough information from a query to determine what usage id to use. I like the suggestion of just using the query. One of the goals I initially had with this series was to get rid of the heap mask. The 32-bit heap mask became a name space that needed to be controlled across all targets and the fallbacks were difficult to change. The problem that actually wants to be solved is giving userspace enough information to determine what heap masks to allocate from. Just a query ioctl should be able to give that information as long as the requirement is that userspace clients match on the heap name + type only and not rely on the heap ids being constant (I don't think I made that clear with this version of the query ioctl so I'll make sure to clarify). The platform registration can adjust the priority order of the heap ids as necessary. Different names should be able to take care of any changes to the platform configuration. I'll think about this more when I work on v2. Thanks, Laura 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 de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Linaro-mm-sig] [RFC][PATCH 0/6] ion: improved ABI
On 06/06/2016 11:59 PM, Chen Feng wrote: The idea is good, define the heap ids in header file is inconvenient. But if we query the heaps information from user-space. It need to maintain this ids and name userspace one by one. The code may be complicated in different module user-space. In android, the gralloc and other lib will all use ion to alloc memory. This will make it more difficult to maintain user-space code. This was a concern I had had as well. Anything that involves dynamic probing is going to involve more tracking. Do you think some library code in libion would help? Thanks, Laura But beyond this, The new alloc2 with not-handle flag is good. And the pull out of ioctl interface is also a good cleanup. On 2016/6/7 2:23, 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. 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC][PATCH 0/6] ion: improved ABI
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 de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Linaro-mm-sig] [RFC][PATCH 0/6] ion: improved ABI
The idea is good, define the heap ids in header file is inconvenient. But if we query the heaps information from user-space. It need to maintain this ids and name userspace one by one. The code may be complicated in different module user-space. In android, the gralloc and other lib will all use ion to alloc memory. This will make it more difficult to maintain user-space code. But beyond this, The new alloc2 with not-handle flag is good. And the pull out of ioctl interface is also a good cleanup. On 2016/6/7 2:23, 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. > > > 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 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC][PATCH 0/6] ion: improved ABI
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. 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 de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
