From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757292AbcFHR3R (ORCPT ); Wed, 8 Jun 2016 13:29:17 -0400 Received: from mail-qg0-f47.google.com ([209.85.192.47]:35670 "EHLO mail-qg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753106AbcFHR3M (ORCPT ); Wed, 8 Jun 2016 13:29:12 -0400 Subject: Re: [Linaro-mm-sig] [RFC][PATCH 0/6] ion: improved ABI To: Chen Feng , Sumit Semwal , John Stultz , =?UTF-8?Q?Arve_Hj=c3=b8nnev=c3=a5g?= , Riley Andrews References: <1465237413-10549-1-git-send-email-labbott@redhat.com> <575670BF.5070808@hisilicon.com> Cc: devel@driverdev.osuosl.org, Jon Medhurst , Android Kernel Team , Liviu Dudau , linux-kernel@vger.kernel.org, linaro-mm-sig@lists.linaro.org, Jeremy Gebben , Eun Taik Lee , Greg Kroah-Hartman From: Laura Abbott Message-ID: <9871ed15-9188-410c-cac8-c5b887538c61@redhat.com> Date: Wed, 8 Jun 2016 10:29:05 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <575670BF.5070808@hisilicon.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> >