[EARLY RFC][PATCH 3/4] ion: Add HEAP_INFO ioctl to be able to fetch heap type

[Andrew F. Davis]

On 2/19/19 3:13 PM, Laura Abbott wrote:
> On 2/15/19 12:24 PM, John Stultz wrote:
>> The per-device heaps don't support HEAP_QUERY ioctl, since
>> the name is provided in the devnode path and the heapid isn't
>> useful with the new interface (one uses the fd of heapdevice).
>>
>> But, one missing bit of functionality is a way to find the
>> heap type. So provide a HEAP_INFO ioctl which exposes the
>> heap type out so there is the potential for some sort of
>> dynamic heap matching/discovery.
>>
>> Most likely this IOCTL will be useful when extended to allow
>> some sort of opaque constraint bitfield to be shared so userland
>> can match heaps with devices in a fully dynamic way.
>>
> 
> We've been waiting on the constraint solving for a while and
> it's never really happened :(
> 

Most likely there will never be a one-size-fits-all solution here. So
allowing for an extensible ABI that permits new information to be
exported as needed will be important.

> It certainly works but I'm concerned about adding this and
> then finding (yet again) that it doesn't work. We're
> getting the heap name now but do we lose anything if we
> don't expose it as part of the ABI?
> 

We can always add more ioctls, we cant go back and remove the old ones
if we make them too clunky and have to remove something they expose. A
simple starting ABI seems to make the most sense here. Even heap type
doesn't look like a good thing to expose, it is just as static and
one-off as heap name, I don't see it having all that much use :/

Andrew

>> Cc: Laura Abbott <labbott at redhat.com>
>> Cc: Sumit Semwal <sumit.semwal at linaro.org>
>> Cc: Liam Mark <lmark at codeaurora.org>
>> Cc: Brian Starkey <Brian.Starkey at arm.com>
>> Cc: Andrew F. Davis <afd at ti.com>
>> Cc: Alistair Strachan <astrachan at google.com>
>> Cc: dri-devel at lists.freedesktop.org
>> Signed-off-by: John Stultz <john.stultz at linaro.org>
>> ---
>>   drivers/staging/android/ion/ion-ioctl.c | 12 ++++++++++++
>>   drivers/staging/android/uapi/ion.h      | 22 ++++++++++++++++++++++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>> b/drivers/staging/android/ion/ion-ioctl.c
>> index ea8d263..6db5969 100644
>> --- a/drivers/staging/android/ion/ion-ioctl.c
>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>> @@ -14,6 +14,7 @@ union ion_ioctl_arg {
>>       struct ion_allocation_data allocation;
>>       struct ion_heap_allocation_data heap_allocation;
>>       struct ion_heap_query query;
>> +    struct ion_heap_info heap_info;
>>       u32 version;
>>   };
>>   @@ -149,6 +150,17 @@ long ion_heap_ioctl(struct file *filp, unsigned
>> int cmd, unsigned long arg)
>>             break;
>>       }
>> +    case ION_IOC_HEAP_INFO:
>> +    {
>> +        struct miscdevice *miscdev = filp->private_data;
>> +        struct ion_heap *heap;
>> +
>> +        heap = container_of(miscdev, struct ion_heap, heap_dev);
>> +
>> +        data.heap_info.type = heap->type;
>> +
>> +        break;
>> +    }
>>       case ION_IOC_VERSION:
>>           data.version = ION_VERSION;
>>           break;
>> diff --git a/drivers/staging/android/uapi/ion.h
>> b/drivers/staging/android/uapi/ion.h
>> index 20db09f..1b3ca1e 100644
>> --- a/drivers/staging/android/uapi/ion.h
>> +++ b/drivers/staging/android/uapi/ion.h
>> @@ -111,6 +111,19 @@ struct ion_heap_data {
>>   };
>>     /**
>> + * struct ion_heap_info - Info about the heap
>> + *
>> + */
>> +struct ion_heap_info {
>> +    __u32 type;
>> +    __u32 reserved0;
>> +    __u32 reserved1;
>> +    __u32 reserved2;
>> +    __u32 reserved3;
>> +    __u32 reserved4;
>> +};
>> +
>> +/**
>>    * struct ion_heap_query - collection of data about all heaps
>>    * @cnt - total number of heaps to be copied
>>    * @heaps - buffer to copy heap data
>> @@ -159,4 +172,13 @@ struct ion_heap_query {
>>   #define ION_IOC_HEAP_ALLOC    _IOWR(ION_IOC_MAGIC, 10, \
>>                         struct ion_heap_allocation_data)
>>   +/**
>> + * DOC: ION_IOC_HEAP_INFO - allocate memory from heap
>> + *
>> + * Takes an ion_heap_query structure and populates information about
>> + * available Ion heaps.
>> + */
>> +#define ION_IOC_HEAP_INFO    _IOWR(ION_IOC_MAGIC, 11, \
>> +                      struct ion_heap_allocation_data)
>> +
>>   #endif /* _UAPI_LINUX_ION_H */
>>
>