2018-12-19 11:42:14

by Zengtao (B)

[permalink] [raw]
Subject: [PATCH] staging: android: ion: add buffer flag update ioctl

In some usecases, the buffer cached attribute is not determined at
allocation time, it's determined just before the real cpu mapping.
And from the memory view of point, a buffer should not have the cached
attribute util is really mapped by the cpu. So in this patch, we
introduced the new ioctl command to target the requirement.

Signed-off-by: Zeng Tao <[email protected]>
---
drivers/staging/android/ion/ion-ioctl.c | 4 ++++
drivers/staging/android/ion/ion.c | 17 +++++++++++++++++
drivers/staging/android/ion/ion.h | 1 +
drivers/staging/android/uapi/ion.h | 22 ++++++++++++++++++++++
4 files changed, 44 insertions(+)

diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
index a8d3cc4..60bb702 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -12,6 +12,7 @@

union ion_ioctl_arg {
struct ion_allocation_data allocation;
+ struct ion_buffer_flag_data update;
struct ion_heap_query query;
};

@@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

break;
}
+ case ION_IOC_BUFFER_UPDATE:
+ ret = ion_buffer_update(data.update.fd, data.update.flags);
+ break;
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(&data.query);
break;
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 9907332..f1404dc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
return fd;
}

+int ion_buffer_update(unsigned int fd, unsigned int flags)
+{
+ struct dma_buf *dmabuf;
+ struct ion_buffer *buffer;
+
+ dmabuf = dma_buf_get(fd);
+
+ if (!dmabuf)
+ return -EINVAL;
+
+ buffer = dmabuf->priv;
+ buffer->flags = flags;
+ dma_buf_put(dmabuf);
+
+ return 0;
+}
+
int ion_query_heaps(struct ion_heap_query *query)
{
struct ion_device *dev = internal_dev;
diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
index c006fc1..99bf9ab 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);
int ion_alloc(size_t len,
unsigned int heap_id_mask,
unsigned int flags);
+int ion_buffer_update(unsigned int fd, unsigned int flags);

/**
* ion_heap_init_shrinker
diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
index 5d70098..99753fc 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -74,6 +74,20 @@ struct ion_allocation_data {
__u32 unused;
};

+/**
+ * struct ion_buffer_flag_data - metadata passed from userspace for update
+ * buffer flags
+ * @fd: file descriptor of the buffer
+ * @flags: flags passed to the buffer
+ *
+ * Provided by userspace as an argument to the ioctl
+ */
+
+struct ion_buffer_flag_data {
+ __u32 fd;
+ __u32 flags;
+}
+
#define MAX_HEAP_NAME 32

/**
@@ -116,6 +130,14 @@ struct ion_heap_query {
struct ion_allocation_data)

/**
+ * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer flags
+ *
+ * Takes an ion_buffer_flag_data structure and returns the result of the
+ * buffer flag update operation.
+ */
+#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
+ struct ion_buffer_flag_data)
+/**
* DOC: ION_IOC_HEAP_QUERY - information about available heaps
*
* Takes an ion_heap_query structure and populates information about
--
2.7.4



2018-12-19 17:53:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl

Hi Zeng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.20-rc7 next-20181219]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Zeng-Tao/staging-android-ion-add-buffer-flag-update-ioctl/20181219-231147
config: i386-randconfig-x079-201850 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All errors (new ones prefixed by >>):

In file included from drivers/staging/android/ion/ion.h:22:0,
from drivers/staging/android/ion/ion.c:30:
>> drivers/staging/android/ion/../uapi/ion.h:99:1: error: expected ';', identifier or '(' before 'struct'
struct ion_heap_data {
^~~~~~

vim +99 drivers/staging/android/ion/../uapi/ion.h

02b23803 Laura Abbott 2016-09-07 92
02b23803 Laura Abbott 2016-09-07 93 /**
02b23803 Laura Abbott 2016-09-07 94 * struct ion_heap_data - data about a heap
02b23803 Laura Abbott 2016-09-07 95 * @name - first 32 characters of the heap name
02b23803 Laura Abbott 2016-09-07 96 * @type - heap type
02b23803 Laura Abbott 2016-09-07 97 * @heap_id - heap id for the heap
02b23803 Laura Abbott 2016-09-07 98 */
02b23803 Laura Abbott 2016-09-07 @99 struct ion_heap_data {
02b23803 Laura Abbott 2016-09-07 100 char name[MAX_HEAP_NAME];
02b23803 Laura Abbott 2016-09-07 101 __u32 type;
02b23803 Laura Abbott 2016-09-07 102 __u32 heap_id;
02b23803 Laura Abbott 2016-09-07 103 __u32 reserved0;
02b23803 Laura Abbott 2016-09-07 104 __u32 reserved1;
02b23803 Laura Abbott 2016-09-07 105 __u32 reserved2;
02b23803 Laura Abbott 2016-09-07 106 };
02b23803 Laura Abbott 2016-09-07 107

:::::: The code at line 99 was first introduced by commit
:::::: 02b23803c6af399473703e26703f74cfff3f22f8 staging: android: ion: Add ioctl to query available heaps

:::::: TO: Laura Abbott <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.28 kB)
.config.gz (29.50 kB)
Download all attachments

2018-12-19 20:56:23

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl

On 12/19/18 9:19 AM, Zeng Tao wrote:
> In some usecases, the buffer cached attribute is not determined at
> allocation time, it's determined just before the real cpu mapping.
> And from the memory view of point, a buffer should not have the cached
> attribute util is really mapped by the cpu. So in this patch, we
> introduced the new ioctl command to target the requirement.
>

This is racy and error prone. Can you explain more what
problem you are trying to solve?

> Signed-off-by: Zeng Tao <[email protected]>
> ---
> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
> drivers/staging/android/ion/ion.c | 17 +++++++++++++++++
> drivers/staging/android/ion/ion.h | 1 +
> drivers/staging/android/uapi/ion.h | 22 ++++++++++++++++++++++
> 4 files changed, 44 insertions(+)
>
> diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
> index a8d3cc4..60bb702 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -12,6 +12,7 @@
>
> union ion_ioctl_arg {
> struct ion_allocation_data allocation;
> + struct ion_buffer_flag_data update;
> struct ion_heap_query query;
> };
>
> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>
> break;
> }
> + case ION_IOC_BUFFER_UPDATE:
> + ret = ion_buffer_update(data.update.fd, data.update.flags);
> + break;
> case ION_IOC_HEAP_QUERY:
> ret = ion_query_heaps(&data.query);
> break;
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 9907332..f1404dc 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
> return fd;
> }
>
> +int ion_buffer_update(unsigned int fd, unsigned int flags)
> +{
> + struct dma_buf *dmabuf;
> + struct ion_buffer *buffer;
> +
> + dmabuf = dma_buf_get(fd);
> +
> + if (!dmabuf)
> + return -EINVAL;
> +
> + buffer = dmabuf->priv;
> + buffer->flags = flags;
> + dma_buf_put(dmabuf);
> +
> + return 0;
> +}
> +
> int ion_query_heaps(struct ion_heap_query *query)
> {
> struct ion_device *dev = internal_dev;
> diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
> index c006fc1..99bf9ab 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);
> int ion_alloc(size_t len,
> unsigned int heap_id_mask,
> unsigned int flags);
> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>
> /**
> * ion_heap_init_shrinker
> diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
> index 5d70098..99753fc 100644
> --- a/drivers/staging/android/uapi/ion.h
> +++ b/drivers/staging/android/uapi/ion.h
> @@ -74,6 +74,20 @@ struct ion_allocation_data {
> __u32 unused;
> };
>
> +/**
> + * struct ion_buffer_flag_data - metadata passed from userspace for update
> + * buffer flags
> + * @fd: file descriptor of the buffer
> + * @flags: flags passed to the buffer
> + *
> + * Provided by userspace as an argument to the ioctl
> + */
> +
> +struct ion_buffer_flag_data {
> + __u32 fd;
> + __u32 flags;
> +}
> +
> #define MAX_HEAP_NAME 32
>
> /**
> @@ -116,6 +130,14 @@ struct ion_heap_query {
> struct ion_allocation_data)
>
> /**
> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer flags
> + *
> + * Takes an ion_buffer_flag_data structure and returns the result of the
> + * buffer flag update operation.
> + */
> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
> + struct ion_buffer_flag_data)
> +/**
> * DOC: ION_IOC_HEAP_QUERY - information about available heaps
> *
> * Takes an ion_heap_query structure and populates information about
>


2018-12-20 05:42:31

by Zengtao (B)

[permalink] [raw]
Subject: RE: [PATCH] staging: android: ion: add buffer flag update ioctl

Hi laura:

>-----Original Message-----
>From: Laura Abbott [mailto:[email protected]]
>Sent: Thursday, December 20, 2018 2:10 AM
>To: Zengtao (B) <[email protected]>; [email protected]
>Cc: Greg Kroah-Hartman <[email protected]>; Arve Hjønnevåg
><[email protected]>; Todd Kjos <[email protected]>; Martijn Coenen
><[email protected]>; Joel Fernandes <[email protected]>;
>[email protected]; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/19/18 9:19 AM, Zeng Tao wrote:
>> In some usecases, the buffer cached attribute is not determined at
>> allocation time, it's determined just before the real cpu mapping.
>> And from the memory view of point, a buffer should not have the
>cached
>> attribute util is really mapped by the cpu. So in this patch, we
>> introduced the new ioctl command to target the requirement.
>>
>
>This is racy and error prone. Can you explain more what problem you are
>trying to solve?

My use case is like this:
1. There are two process A and B, A takes case of ion buffer allocation, and
pass the buffer fd to B, then B maps and uses it.
2. Process B need to map the buffer with different cached attribute for
different use case, for example, if the buffer is used for pure software algorithm,
then we need to map it as cached, otherwise non-cached, and B needs to deal
with both cases.
And unfortunately the mmap syscall takes no cached flags and we can't decide
the cache attribute when we are doing the mmap, so I introduce new the ioctl
even though I think the solution is not as good.


>
>> Signed-off-by: Zeng Tao <[email protected]>
>> ---
>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
>> drivers/staging/android/ion/ion.c | 17 +++++++++++++++++
>> drivers/staging/android/ion/ion.h | 1 +
>> drivers/staging/android/uapi/ion.h | 22
>++++++++++++++++++++++
>> 4 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>> b/drivers/staging/android/ion/ion-ioctl.c
>> index a8d3cc4..60bb702 100644
>> --- a/drivers/staging/android/ion/ion-ioctl.c
>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>> @@ -12,6 +12,7 @@
>>
>> union ion_ioctl_arg {
>> struct ion_allocation_data allocation;
>> + struct ion_buffer_flag_data update;
>> struct ion_heap_query query;
>> };
>>
>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
>> unsigned long arg)
>>
>> break;
>> }
>> + case ION_IOC_BUFFER_UPDATE:
>> + ret = ion_buffer_update(data.update.fd, data.update.flags);
>> + break;
>> case ION_IOC_HEAP_QUERY:
>> ret = ion_query_heaps(&data.query);
>> break;
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..f1404dc 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> return fd;
>> }
>>
>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>> + struct dma_buf *dmabuf;
>> + struct ion_buffer *buffer;
>> +
>> + dmabuf = dma_buf_get(fd);
>> +
>> + if (!dmabuf)
>> + return -EINVAL;
>> +
>> + buffer = dmabuf->priv;
>> + buffer->flags = flags;
>> + dma_buf_put(dmabuf);
>> +
>> + return 0;
>> +}
>> +
>> int ion_query_heaps(struct ion_heap_query *query)
>> {
>> struct ion_device *dev = internal_dev; diff --git
>> a/drivers/staging/android/ion/ion.h
>> b/drivers/staging/android/ion/ion.h
>> index c006fc1..99bf9ab 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page,
>size_t size, pgprot_t pgprot);
>> int ion_alloc(size_t len,
>> unsigned int heap_id_mask,
>> unsigned int flags);
>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>
>> /**
>> * ion_heap_init_shrinker
>> diff --git a/drivers/staging/android/uapi/ion.h
>> b/drivers/staging/android/uapi/ion.h
>> index 5d70098..99753fc 100644
>> --- a/drivers/staging/android/uapi/ion.h
>> +++ b/drivers/staging/android/uapi/ion.h
>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>> __u32 unused;
>> };
>>
>> +/**
>> + * struct ion_buffer_flag_data - metadata passed from userspace for
>> +update
>> + * buffer flags
>> + * @fd: file descriptor of the buffer
>> + * @flags: flags passed to the buffer
>> + *
>> + * Provided by userspace as an argument to the ioctl */
>> +
>> +struct ion_buffer_flag_data {
>> + __u32 fd;
>> + __u32 flags;
>> +}
>> +
>> #define MAX_HEAP_NAME 32
>>
>> /**
>> @@ -116,6 +130,14 @@ struct ion_heap_query {
>> struct ion_allocation_data)
>>
>> /**
>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer
>flags
>> + *
>> + * Takes an ion_buffer_flag_data structure and returns the result of
>> +the
>> + * buffer flag update operation.
>> + */
>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
>> + struct ion_buffer_flag_data)
>> +/**
>> * DOC: ION_IOC_HEAP_QUERY - information about available heaps
>> *
>> * Takes an ion_heap_query structure and populates information
>about
>>

2018-12-21 07:30:21

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl

On 12/19/18 5:39 PM, Zengtao (B) wrote:
> Hi laura:
>
>> -----Original Message-----
>> From: Laura Abbott [mailto:[email protected]]
>> Sent: Thursday, December 20, 2018 2:10 AM
>> To: Zengtao (B) <[email protected]>; [email protected]
>> Cc: Greg Kroah-Hartman <[email protected]>; Arve Hjønnevåg
>> <[email protected]>; Todd Kjos <[email protected]>; Martijn Coenen
>> <[email protected]>; Joel Fernandes <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>>
>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>> In some usecases, the buffer cached attribute is not determined at
>>> allocation time, it's determined just before the real cpu mapping.
>>> And from the memory view of point, a buffer should not have the
>> cached
>>> attribute util is really mapped by the cpu. So in this patch, we
>>> introduced the new ioctl command to target the requirement.
>>>
>>
>> This is racy and error prone. Can you explain more what problem you are
>> trying to solve?
>
> My use case is like this:
> 1. There are two process A and B, A takes case of ion buffer allocation, and
> pass the buffer fd to B, then B maps and uses it.
> 2. Process B need to map the buffer with different cached attribute for
> different use case, for example, if the buffer is used for pure software algorithm,
> then we need to map it as cached, otherwise non-cached, and B needs to deal
> with both cases.
> And unfortunately the mmap syscall takes no cached flags and we can't decide
> the cache attribute when we are doing the mmap, so I introduce new the ioctl
> even though I think the solution is not as good.
>
>

Thanks for the explanation, this was about the use case I expected.
I'm pretty sure I had this exact problem once upon a time and we
didn't come up with a solution. I'd still like to get rid of
uncached buffers in general and just use cached buffers
(see http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128842.html)
What's your usecase for uncached buffers?

>>
>>> Signed-off-by: Zeng Tao <[email protected]>
>>> ---
>>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
>>> drivers/staging/android/ion/ion.c | 17 +++++++++++++++++
>>> drivers/staging/android/ion/ion.h | 1 +
>>> drivers/staging/android/uapi/ion.h | 22
>> ++++++++++++++++++++++
>>> 4 files changed, 44 insertions(+)
>>>
>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>> b/drivers/staging/android/ion/ion-ioctl.c
>>> index a8d3cc4..60bb702 100644
>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>> @@ -12,6 +12,7 @@
>>>
>>> union ion_ioctl_arg {
>>> struct ion_allocation_data allocation;
>>> + struct ion_buffer_flag_data update;
>>> struct ion_heap_query query;
>>> };
>>>
>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
>>> unsigned long arg)
>>>
>>> break;
>>> }
>>> + case ION_IOC_BUFFER_UPDATE:
>>> + ret = ion_buffer_update(data.update.fd, data.update.flags);
>>> + break;
>>> case ION_IOC_HEAP_QUERY:
>>> ret = ion_query_heaps(&data.query);
>>> break;
>>> diff --git a/drivers/staging/android/ion/ion.c
>>> b/drivers/staging/android/ion/ion.c
>>> index 9907332..f1404dc 100644
>>> --- a/drivers/staging/android/ion/ion.c
>>> +++ b/drivers/staging/android/ion/ion.c
>>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>> heap_id_mask, unsigned int flags)
>>> return fd;
>>> }
>>>
>>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>>> + struct dma_buf *dmabuf;
>>> + struct ion_buffer *buffer;
>>> +
>>> + dmabuf = dma_buf_get(fd);
>>> +
>>> + if (!dmabuf)
>>> + return -EINVAL;
>>> +
>>> + buffer = dmabuf->priv;
>>> + buffer->flags = flags;
>>> + dma_buf_put(dmabuf);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> int ion_query_heaps(struct ion_heap_query *query)
>>> {
>>> struct ion_device *dev = internal_dev; diff --git
>>> a/drivers/staging/android/ion/ion.h
>>> b/drivers/staging/android/ion/ion.h
>>> index c006fc1..99bf9ab 100644
>>> --- a/drivers/staging/android/ion/ion.h
>>> +++ b/drivers/staging/android/ion/ion.h
>>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page,
>> size_t size, pgprot_t pgprot);
>>> int ion_alloc(size_t len,
>>> unsigned int heap_id_mask,
>>> unsigned int flags);
>>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>>
>>> /**
>>> * ion_heap_init_shrinker
>>> diff --git a/drivers/staging/android/uapi/ion.h
>>> b/drivers/staging/android/uapi/ion.h
>>> index 5d70098..99753fc 100644
>>> --- a/drivers/staging/android/uapi/ion.h
>>> +++ b/drivers/staging/android/uapi/ion.h
>>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>>> __u32 unused;
>>> };
>>>
>>> +/**
>>> + * struct ion_buffer_flag_data - metadata passed from userspace for
>>> +update
>>> + * buffer flags
>>> + * @fd: file descriptor of the buffer
>>> + * @flags: flags passed to the buffer
>>> + *
>>> + * Provided by userspace as an argument to the ioctl */
>>> +
>>> +struct ion_buffer_flag_data {
>>> + __u32 fd;
>>> + __u32 flags;
>>> +}
>>> +
>>> #define MAX_HEAP_NAME 32
>>>
>>> /**
>>> @@ -116,6 +130,14 @@ struct ion_heap_query {
>>> struct ion_allocation_data)
>>>
>>> /**
>>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer
>> flags
>>> + *
>>> + * Takes an ion_buffer_flag_data structure and returns the result of
>>> +the
>>> + * buffer flag update operation.
>>> + */
>>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
>>> + struct ion_buffer_flag_data)
>>> +/**
>>> * DOC: ION_IOC_HEAP_QUERY - information about available heaps
>>> *
>>> * Takes an ion_heap_query structure and populates information
>> about
>>>
>


2018-12-24 02:48:33

by Zengtao (B)

[permalink] [raw]
Subject: RE: [PATCH] staging: android: ion: add buffer flag update ioctl

Hi laura:

>-----Original Message-----
>From: Laura Abbott [mailto:[email protected]]
>Sent: Friday, December 21, 2018 4:50 AM
>To: Zengtao (B) <[email protected]>; [email protected]
>Cc: Greg Kroah-Hartman <[email protected]>; Arve Hjønnevåg
><[email protected]>; Todd Kjos <[email protected]>; Martijn Coenen
><[email protected]>; Joel Fernandes <[email protected]>;
>[email protected]; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/19/18 5:39 PM, Zengtao (B) wrote:
>> Hi laura:
>>
>>> -----Original Message-----
>>> From: Laura Abbott [mailto:[email protected]]
>>> Sent: Thursday, December 20, 2018 2:10 AM
>>> To: Zengtao (B) <[email protected]>;
>[email protected]
>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>Hjønnevåg
>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>Coenen
>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>> In some usecases, the buffer cached attribute is not determined at
>>>> allocation time, it's determined just before the real cpu mapping.
>>>> And from the memory view of point, a buffer should not have the
>>> cached
>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>> introduced the new ioctl command to target the requirement.
>>>>
>>>
>>> This is racy and error prone. Can you explain more what problem you
>>> are trying to solve?
>>
>> My use case is like this:
>> 1. There are two process A and B, A takes case of ion buffer allocation,
>and
>> pass the buffer fd to B, then B maps and uses it.
>> 2. Process B need to map the buffer with different cached attribute
>> for different use case, for example, if the buffer is used for pure
>> software algorithm, then we need to map it as cached, otherwise
>> non-cached, and B needs to deal with both cases.
>> And unfortunately the mmap syscall takes no cached flags and we can't
>> decide the cache attribute when we are doing the mmap, so I introduce
>> new the ioctl even though I think the solution is not as good.
>>
>>
>
>Thanks for the explanation, this was about the use case I expected.
>I'm pretty sure I had this exact problem once upon a time and we didn't
>come up with a solution. I'd still like to get rid of uncached buffers in
>general and just use cached buffers (see
>http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-N
>ovember/128842.html)
>What's your usecase for uncached buffers?

Some buffers are only used by HW, in this case, we tend to use uncached buffers.
But sometimes the SW need to read few buffer contents for debug purpose and
no synchronization is needed.

>
>>>
>>>> Signed-off-by: Zeng Tao <[email protected]>
>>>> ---
>>>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
>>>> drivers/staging/android/ion/ion.c | 17
>+++++++++++++++++
>>>> drivers/staging/android/ion/ion.h | 1 +
>>>> drivers/staging/android/uapi/ion.h | 22
>>> ++++++++++++++++++++++
>>>> 4 files changed, 44 insertions(+)
>>>>
>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>>> b/drivers/staging/android/ion/ion-ioctl.c
>>>> index a8d3cc4..60bb702 100644
>>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>>> @@ -12,6 +12,7 @@
>>>>
>>>> union ion_ioctl_arg {
>>>> struct ion_allocation_data allocation;
>>>> + struct ion_buffer_flag_data update;
>>>> struct ion_heap_query query;
>>>> };
>>>>
>>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
>>>> cmd, unsigned long arg)
>>>>
>>>> break;
>>>> }
>>>> + case ION_IOC_BUFFER_UPDATE:
>>>> + ret = ion_buffer_update(data.update.fd, data.update.flags);
>>>> + break;
>>>> case ION_IOC_HEAP_QUERY:
>>>> ret = ion_query_heaps(&data.query);
>>>> break;
>>>> diff --git a/drivers/staging/android/ion/ion.c
>>>> b/drivers/staging/android/ion/ion.c
>>>> index 9907332..f1404dc 100644
>>>> --- a/drivers/staging/android/ion/ion.c
>>>> +++ b/drivers/staging/android/ion/ion.c
>>>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>>> heap_id_mask, unsigned int flags)
>>>> return fd;
>>>> }
>>>>
>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>>>> + struct dma_buf *dmabuf;
>>>> + struct ion_buffer *buffer;
>>>> +
>>>> + dmabuf = dma_buf_get(fd);
>>>> +
>>>> + if (!dmabuf)
>>>> + return -EINVAL;
>>>> +
>>>> + buffer = dmabuf->priv;
>>>> + buffer->flags = flags;
>>>> + dma_buf_put(dmabuf);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> int ion_query_heaps(struct ion_heap_query *query)
>>>> {
>>>> struct ion_device *dev = internal_dev; diff --git
>>>> a/drivers/staging/android/ion/ion.h
>>>> b/drivers/staging/android/ion/ion.h
>>>> index c006fc1..99bf9ab 100644
>>>> --- a/drivers/staging/android/ion/ion.h
>>>> +++ b/drivers/staging/android/ion/ion.h
>>>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page,
>>> size_t size, pgprot_t pgprot);
>>>> int ion_alloc(size_t len,
>>>> unsigned int heap_id_mask,
>>>> unsigned int flags);
>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>>>
>>>> /**
>>>> * ion_heap_init_shrinker
>>>> diff --git a/drivers/staging/android/uapi/ion.h
>>>> b/drivers/staging/android/uapi/ion.h
>>>> index 5d70098..99753fc 100644
>>>> --- a/drivers/staging/android/uapi/ion.h
>>>> +++ b/drivers/staging/android/uapi/ion.h
>>>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>>>> __u32 unused;
>>>> };
>>>>
>>>> +/**
>>>> + * struct ion_buffer_flag_data - metadata passed from userspace for
>>>> +update
>>>> + * buffer flags
>>>> + * @fd: file descriptor of the buffer
>>>> + * @flags: flags passed to the buffer
>>>> + *
>>>> + * Provided by userspace as an argument to the ioctl */
>>>> +
>>>> +struct ion_buffer_flag_data {
>>>> + __u32 fd;
>>>> + __u32 flags;
>>>> +}
>>>> +
>>>> #define MAX_HEAP_NAME 32
>>>>
>>>> /**
>>>> @@ -116,6 +130,14 @@ struct ion_heap_query {
>>>> struct ion_allocation_data)
>>>>
>>>> /**
>>>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer
>>> flags
>>>> + *
>>>> + * Takes an ion_buffer_flag_data structure and returns the result
>>>> +of the
>>>> + * buffer flag update operation.
>>>> + */
>>>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
>>>> + struct ion_buffer_flag_data)
>>>> +/**
>>>> * DOC: ION_IOC_HEAP_QUERY - information about available
>heaps
>>>> *
>>>> * Takes an ion_heap_query structure and populates information
>>> about
>>>>
>>

2019-01-03 02:53:59

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl

On 12/23/18 6:47 PM, Zengtao (B) wrote:
> Hi laura:
>
>> -----Original Message-----
>> From: Laura Abbott [mailto:[email protected]]
>> Sent: Friday, December 21, 2018 4:50 AM
>> To: Zengtao (B) <[email protected]>; [email protected]
>> Cc: Greg Kroah-Hartman <[email protected]>; Arve Hjønnevåg
>> <[email protected]>; Todd Kjos <[email protected]>; Martijn Coenen
>> <[email protected]>; Joel Fernandes <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>>
>> On 12/19/18 5:39 PM, Zengtao (B) wrote:
>>> Hi laura:
>>>
>>>> -----Original Message-----
>>>> From: Laura Abbott [mailto:[email protected]]
>>>> Sent: Thursday, December 20, 2018 2:10 AM
>>>> To: Zengtao (B) <[email protected]>;
>> [email protected]
>>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>> Hjønnevåg
>>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>> Coenen
>>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>>> [email protected]; [email protected];
>>>> [email protected]; [email protected]
>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>>> ioctl
>>>>
>>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>>> In some usecases, the buffer cached attribute is not determined at
>>>>> allocation time, it's determined just before the real cpu mapping.
>>>>> And from the memory view of point, a buffer should not have the
>>>> cached
>>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>>> introduced the new ioctl command to target the requirement.
>>>>>
>>>>
>>>> This is racy and error prone. Can you explain more what problem you
>>>> are trying to solve?
>>>
>>> My use case is like this:
>>> 1. There are two process A and B, A takes case of ion buffer allocation,
>> and
>>> pass the buffer fd to B, then B maps and uses it.
>>> 2. Process B need to map the buffer with different cached attribute
>>> for different use case, for example, if the buffer is used for pure
>>> software algorithm, then we need to map it as cached, otherwise
>>> non-cached, and B needs to deal with both cases.
>>> And unfortunately the mmap syscall takes no cached flags and we can't
>>> decide the cache attribute when we are doing the mmap, so I introduce
>>> new the ioctl even though I think the solution is not as good.
>>>
>>>
>>
>> Thanks for the explanation, this was about the use case I expected.
>> I'm pretty sure I had this exact problem once upon a time and we didn't
>> come up with a solution. I'd still like to get rid of uncached buffers in
>> general and just use cached buffers (see
>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-N
>> ovember/128842.html)
>> What's your usecase for uncached buffers?
>
> Some buffers are only used by HW, in this case, we tend to use uncached buffers.
> But sometimes the SW need to read few buffer contents for debug purpose and
> no synchronization is needed.
>

If this is primarily for debug, that's not really a compelling reason to
support uncached buffers. It's incredibly difficult to do uncached
buffers correctly I'd rather spend effort on making the cached
use cases work better.

Thanks,
Laura

>>
>>>>
>>>>> Signed-off-by: Zeng Tao <[email protected]>
>>>>> ---
>>>>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
>>>>> drivers/staging/android/ion/ion.c | 17
>> +++++++++++++++++
>>>>> drivers/staging/android/ion/ion.h | 1 +
>>>>> drivers/staging/android/uapi/ion.h | 22
>>>> ++++++++++++++++++++++
>>>>> 4 files changed, 44 insertions(+)
>>>>>
>>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>>>> b/drivers/staging/android/ion/ion-ioctl.c
>>>>> index a8d3cc4..60bb702 100644
>>>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>>>> @@ -12,6 +12,7 @@
>>>>>
>>>>> union ion_ioctl_arg {
>>>>> struct ion_allocation_data allocation;
>>>>> + struct ion_buffer_flag_data update;
>>>>> struct ion_heap_query query;
>>>>> };
>>>>>
>>>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
>>>>> cmd, unsigned long arg)
>>>>>
>>>>> break;
>>>>> }
>>>>> + case ION_IOC_BUFFER_UPDATE:
>>>>> + ret = ion_buffer_update(data.update.fd, data.update.flags);
>>>>> + break;
>>>>> case ION_IOC_HEAP_QUERY:
>>>>> ret = ion_query_heaps(&data.query);
>>>>> break;
>>>>> diff --git a/drivers/staging/android/ion/ion.c
>>>>> b/drivers/staging/android/ion/ion.c
>>>>> index 9907332..f1404dc 100644
>>>>> --- a/drivers/staging/android/ion/ion.c
>>>>> +++ b/drivers/staging/android/ion/ion.c
>>>>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>>>> heap_id_mask, unsigned int flags)
>>>>> return fd;
>>>>> }
>>>>>
>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>>>>> + struct dma_buf *dmabuf;
>>>>> + struct ion_buffer *buffer;
>>>>> +
>>>>> + dmabuf = dma_buf_get(fd);
>>>>> +
>>>>> + if (!dmabuf)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + buffer = dmabuf->priv;
>>>>> + buffer->flags = flags;
>>>>> + dma_buf_put(dmabuf);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> int ion_query_heaps(struct ion_heap_query *query)
>>>>> {
>>>>> struct ion_device *dev = internal_dev; diff --git
>>>>> a/drivers/staging/android/ion/ion.h
>>>>> b/drivers/staging/android/ion/ion.h
>>>>> index c006fc1..99bf9ab 100644
>>>>> --- a/drivers/staging/android/ion/ion.h
>>>>> +++ b/drivers/staging/android/ion/ion.h
>>>>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page,
>>>> size_t size, pgprot_t pgprot);
>>>>> int ion_alloc(size_t len,
>>>>> unsigned int heap_id_mask,
>>>>> unsigned int flags);
>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>>>>
>>>>> /**
>>>>> * ion_heap_init_shrinker
>>>>> diff --git a/drivers/staging/android/uapi/ion.h
>>>>> b/drivers/staging/android/uapi/ion.h
>>>>> index 5d70098..99753fc 100644
>>>>> --- a/drivers/staging/android/uapi/ion.h
>>>>> +++ b/drivers/staging/android/uapi/ion.h
>>>>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>>>>> __u32 unused;
>>>>> };
>>>>>
>>>>> +/**
>>>>> + * struct ion_buffer_flag_data - metadata passed from userspace for
>>>>> +update
>>>>> + * buffer flags
>>>>> + * @fd: file descriptor of the buffer
>>>>> + * @flags: flags passed to the buffer
>>>>> + *
>>>>> + * Provided by userspace as an argument to the ioctl */
>>>>> +
>>>>> +struct ion_buffer_flag_data {
>>>>> + __u32 fd;
>>>>> + __u32 flags;
>>>>> +}
>>>>> +
>>>>> #define MAX_HEAP_NAME 32
>>>>>
>>>>> /**
>>>>> @@ -116,6 +130,14 @@ struct ion_heap_query {
>>>>> struct ion_allocation_data)
>>>>>
>>>>> /**
>>>>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer
>>>> flags
>>>>> + *
>>>>> + * Takes an ion_buffer_flag_data structure and returns the result
>>>>> +of the
>>>>> + * buffer flag update operation.
>>>>> + */
>>>>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
>>>>> + struct ion_buffer_flag_data)
>>>>> +/**
>>>>> * DOC: ION_IOC_HEAP_QUERY - information about available
>> heaps
>>>>> *
>>>>> * Takes an ion_heap_query structure and populates information
>>>> about
>>>>>
>>>
>


2019-01-04 06:05:10

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl

On 1/3/19 5:42 PM, Zengtao (B) wrote:
>> -----Original Message-----
>> From: Laura Abbott [mailto:[email protected]]
>> Sent: Thursday, January 03, 2019 6:31 AM
>> To: Zengtao (B) <[email protected]>; [email protected]
>> Cc: Greg Kroah-Hartman <[email protected]>; Arve Hjønnevåg
>> <[email protected]>; Todd Kjos <[email protected]>; Martijn Coenen
>> <[email protected]>; Joel Fernandes <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>>
>> On 12/23/18 6:47 PM, Zengtao (B) wrote:
>>> Hi laura:
>>>
>>>> -----Original Message-----
>>>> From: Laura Abbott [mailto:[email protected]]
>>>> Sent: Friday, December 21, 2018 4:50 AM
>>>> To: Zengtao (B) <[email protected]>;
>> [email protected]
>>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>> Hjønnevåg
>>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>> Coenen
>>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>>> [email protected]; [email protected];
>>>> [email protected]; [email protected]
>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>>> ioctl
>>>>
>>>> On 12/19/18 5:39 PM, Zengtao (B) wrote:
>>>>> Hi laura:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Laura Abbott [mailto:[email protected]]
>>>>>> Sent: Thursday, December 20, 2018 2:10 AM
>>>>>> To: Zengtao (B) <[email protected]>;
>>>> [email protected]
>>>>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>>>> Hjønnevåg
>>>>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>>>> Coenen
>>>>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>>>>> [email protected]; [email protected];
>>>>>> [email protected]; [email protected]
>>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>>>>> ioctl
>>>>>>
>>>>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>>>>> In some usecases, the buffer cached attribute is not determined at
>>>>>>> allocation time, it's determined just before the real cpu mapping.
>>>>>>> And from the memory view of point, a buffer should not have the
>>>>>> cached
>>>>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>>>>> introduced the new ioctl command to target the requirement.
>>>>>>>
>>>>>>
>>>>>> This is racy and error prone. Can you explain more what problem
>> you
>>>>>> are trying to solve?
>>>>>
>>>>> My use case is like this:
>>>>> 1. There are two process A and B, A takes case of ion buffer
>>>>> allocation,
>>>> and
>>>>> pass the buffer fd to B, then B maps and uses it.
>>>>> 2. Process B need to map the buffer with different cached attribute
>>>>> for different use case, for example, if the buffer is used for pure
>>>>> software algorithm, then we need to map it as cached, otherwise
>>>>> non-cached, and B needs to deal with both cases.
>>>>> And unfortunately the mmap syscall takes no cached flags and we
>>>>> can't decide the cache attribute when we are doing the mmap, so I
>>>>> introduce new the ioctl even though I think the solution is not as
>> good.
>>>>>
>>>>>
>>>>
>>>> Thanks for the explanation, this was about the use case I expected.
>>>> I'm pretty sure I had this exact problem once upon a time and we
>>>> didn't come up with a solution. I'd still like to get rid of uncached
>>>> buffers in general and just use cached buffers (see
>>>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/201
>>>> 8-N
>>>> ovember/128842.html)
>>>> What's your usecase for uncached buffers?
>>>
>>> Some buffers are only used by HW, in this case, we tend to use
>> uncached buffers.
>>> But sometimes the SW need to read few buffer contents for debug
>>> purpose and no synchronization is needed.
>>>
>>
>> If this is primarily for debug, that's not really a compelling reason to
>> support uncached buffers. It's incredibly difficult to do uncached buffers
>> correctly I'd rather spend effort on making the cached use cases work
>> better.
>>
> No, not only for debug, I meant if the buffers are uncached, the SW will only mmap
> them for debug or even don't need to mmap them.
> So for the two kinds of buffers:
> 1. uncached: only the HW need to access it, the SW don't need to mmap it or just for debug.
> 2. cached: both the HW and the SW need to access it.
> The ION is designed as a generalized memory manager, I think we should cover as many
> requirements as we can in the ION design, so I 'd rather that we keep the uncached support.
> And I don’t quite understand the difficulty you mentioned to support the uncached buffers, would
> you explain a little more about it.
>

We end up with aliasing problems. Each kernel page still has a cached
mapping so it's very difficult to keep the two mappings in sync.
https://lore.kernel.org/lkml/[email protected]/
This thread does a better job of explaining the problem and the
objections to uncached buffers.

And if the software never touches the buffer, why does it
matter if the buffer is uncached?

Laura

> Thanks
> Zengtao
>
>>
>>>>
>>>>>>
>>>>>>> Signed-off-by: Zeng Tao <[email protected]>
>>>>>>> ---
>>>>>>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
>>>>>>> drivers/staging/android/ion/ion.c | 17
>>>> +++++++++++++++++
>>>>>>> drivers/staging/android/ion/ion.h | 1 +
>>>>>>> drivers/staging/android/uapi/ion.h | 22
>>>>>> ++++++++++++++++++++++
>>>>>>> 4 files changed, 44 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>>>>>> b/drivers/staging/android/ion/ion-ioctl.c
>>>>>>> index a8d3cc4..60bb702 100644
>>>>>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>>>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>>>>>> @@ -12,6 +12,7 @@
>>>>>>>
>>>>>>> union ion_ioctl_arg {
>>>>>>> struct ion_allocation_data allocation;
>>>>>>> + struct ion_buffer_flag_data update;
>>>>>>> struct ion_heap_query query;
>>>>>>> };
>>>>>>>
>>>>>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
>>>>>>> cmd, unsigned long arg)
>>>>>>>
>>>>>>> break;
>>>>>>> }
>>>>>>> + case ION_IOC_BUFFER_UPDATE:
>>>>>>> + ret = ion_buffer_update(data.update.fd, data.update.flags);
>>>>>>> + break;
>>>>>>> case ION_IOC_HEAP_QUERY:
>>>>>>> ret = ion_query_heaps(&data.query);
>>>>>>> break;
>>>>>>> diff --git a/drivers/staging/android/ion/ion.c
>>>>>>> b/drivers/staging/android/ion/ion.c
>>>>>>> index 9907332..f1404dc 100644
>>>>>>> --- a/drivers/staging/android/ion/ion.c
>>>>>>> +++ b/drivers/staging/android/ion/ion.c
>>>>>>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>>>>>> heap_id_mask, unsigned int flags)
>>>>>>> return fd;
>>>>>>> }
>>>>>>>
>>>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>>>>>>> + struct dma_buf *dmabuf;
>>>>>>> + struct ion_buffer *buffer;
>>>>>>> +
>>>>>>> + dmabuf = dma_buf_get(fd);
>>>>>>> +
>>>>>>> + if (!dmabuf)
>>>>>>> + return -EINVAL;
>>>>>>> +
>>>>>>> + buffer = dmabuf->priv;
>>>>>>> + buffer->flags = flags;
>>>>>>> + dma_buf_put(dmabuf);
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> int ion_query_heaps(struct ion_heap_query *query)
>>>>>>> {
>>>>>>> struct ion_device *dev = internal_dev; diff --git
>>>>>>> a/drivers/staging/android/ion/ion.h
>>>>>>> b/drivers/staging/android/ion/ion.h
>>>>>>> index c006fc1..99bf9ab 100644
>>>>>>> --- a/drivers/staging/android/ion/ion.h
>>>>>>> +++ b/drivers/staging/android/ion/ion.h
>>>>>>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page
>> *page,
>>>>>> size_t size, pgprot_t pgprot);
>>>>>>> int ion_alloc(size_t len,
>>>>>>> unsigned int heap_id_mask,
>>>>>>> unsigned int flags);
>>>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>>>>>>
>>>>>>> /**
>>>>>>> * ion_heap_init_shrinker
>>>>>>> diff --git a/drivers/staging/android/uapi/ion.h
>>>>>>> b/drivers/staging/android/uapi/ion.h
>>>>>>> index 5d70098..99753fc 100644
>>>>>>> --- a/drivers/staging/android/uapi/ion.h
>>>>>>> +++ b/drivers/staging/android/uapi/ion.h
>>>>>>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>>>>>>> __u32 unused;
>>>>>>> };
>>>>>>>
>>>>>>> +/**
>>>>>>> + * struct ion_buffer_flag_data - metadata passed from userspace
>>>>>>> +for update
>>>>>>> + * buffer flags
>>>>>>> + * @fd: file descriptor of the buffer
>>>>>>> + * @flags: flags passed to the buffer
>>>>>>> + *
>>>>>>> + * Provided by userspace as an argument to the ioctl */
>>>>>>> +
>>>>>>> +struct ion_buffer_flag_data {
>>>>>>> + __u32 fd;
>>>>>>> + __u32 flags;
>>>>>>> +}
>>>>>>> +
>>>>>>> #define MAX_HEAP_NAME 32
>>>>>>>
>>>>>>> /**
>>>>>>> @@ -116,6 +130,14 @@ struct ion_heap_query {
>>>>>>> struct ion_allocation_data)
>>>>>>>
>>>>>>> /**
>>>>>>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion
>> buffer
>>>>>> flags
>>>>>>> + *
>>>>>>> + * Takes an ion_buffer_flag_data structure and returns the result
>>>>>>> +of the
>>>>>>> + * buffer flag update operation.
>>>>>>> + */
>>>>>>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
>>>>>>> + struct ion_buffer_flag_data)
>>>>>>> +/**
>>>>>>> * DOC: ION_IOC_HEAP_QUERY - information about available
>>>> heaps
>>>>>>> *
>>>>>>> * Takes an ion_heap_query structure and populates
>> information
>>>>>> about
>>>>>>>
>>>>>
>>>
>


2019-01-04 06:05:10

by Zengtao (B)

[permalink] [raw]
Subject: RE: [PATCH] staging: android: ion: add buffer flag update ioctl

>-----Original Message-----
>From: Laura Abbott [mailto:[email protected]]
>Sent: Thursday, January 03, 2019 6:31 AM
>To: Zengtao (B) <[email protected]>; [email protected]
>Cc: Greg Kroah-Hartman <[email protected]>; Arve Hjønnevåg
><[email protected]>; Todd Kjos <[email protected]>; Martijn Coenen
><[email protected]>; Joel Fernandes <[email protected]>;
>[email protected]; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/23/18 6:47 PM, Zengtao (B) wrote:
>> Hi laura:
>>
>>> -----Original Message-----
>>> From: Laura Abbott [mailto:[email protected]]
>>> Sent: Friday, December 21, 2018 4:50 AM
>>> To: Zengtao (B) <[email protected]>;
>[email protected]
>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>Hjønnevåg
>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>Coenen
>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/19/18 5:39 PM, Zengtao (B) wrote:
>>>> Hi laura:
>>>>
>>>>> -----Original Message-----
>>>>> From: Laura Abbott [mailto:[email protected]]
>>>>> Sent: Thursday, December 20, 2018 2:10 AM
>>>>> To: Zengtao (B) <[email protected]>;
>>> [email protected]
>>>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>>> Hjønnevåg
>>>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>>> Coenen
>>>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>>>> [email protected]; [email protected];
>>>>> [email protected]; [email protected]
>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>>>> ioctl
>>>>>
>>>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>>>> In some usecases, the buffer cached attribute is not determined at
>>>>>> allocation time, it's determined just before the real cpu mapping.
>>>>>> And from the memory view of point, a buffer should not have the
>>>>> cached
>>>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>>>> introduced the new ioctl command to target the requirement.
>>>>>>
>>>>>
>>>>> This is racy and error prone. Can you explain more what problem
>you
>>>>> are trying to solve?
>>>>
>>>> My use case is like this:
>>>> 1. There are two process A and B, A takes case of ion buffer
>>>> allocation,
>>> and
>>>> pass the buffer fd to B, then B maps and uses it.
>>>> 2. Process B need to map the buffer with different cached attribute
>>>> for different use case, for example, if the buffer is used for pure
>>>> software algorithm, then we need to map it as cached, otherwise
>>>> non-cached, and B needs to deal with both cases.
>>>> And unfortunately the mmap syscall takes no cached flags and we
>>>> can't decide the cache attribute when we are doing the mmap, so I
>>>> introduce new the ioctl even though I think the solution is not as
>good.
>>>>
>>>>
>>>
>>> Thanks for the explanation, this was about the use case I expected.
>>> I'm pretty sure I had this exact problem once upon a time and we
>>> didn't come up with a solution. I'd still like to get rid of uncached
>>> buffers in general and just use cached buffers (see
>>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/201
>>> 8-N
>>> ovember/128842.html)
>>> What's your usecase for uncached buffers?
>>
>> Some buffers are only used by HW, in this case, we tend to use
>uncached buffers.
>> But sometimes the SW need to read few buffer contents for debug
>> purpose and no synchronization is needed.
>>
>
>If this is primarily for debug, that's not really a compelling reason to
>support uncached buffers. It's incredibly difficult to do uncached buffers
>correctly I'd rather spend effort on making the cached use cases work
>better.
>
No, not only for debug, I meant if the buffers are uncached, the SW will only mmap
them for debug or even don't need to mmap them.
So for the two kinds of buffers:
1. uncached: only the HW need to access it, the SW don't need to mmap it or just for debug.
2. cached: both the HW and the SW need to access it.
The ION is designed as a generalized memory manager, I think we should cover as many
requirements as we can in the ION design, so I 'd rather that we keep the uncached support.
And I don’t quite understand the difficulty you mentioned to support the uncached buffers, would
you explain a little more about it.

Thanks
Zengtao

>
>>>
>>>>>
>>>>>> Signed-off-by: Zeng Tao <[email protected]>
>>>>>> ---
>>>>>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
>>>>>> drivers/staging/android/ion/ion.c | 17
>>> +++++++++++++++++
>>>>>> drivers/staging/android/ion/ion.h | 1 +
>>>>>> drivers/staging/android/uapi/ion.h | 22
>>>>> ++++++++++++++++++++++
>>>>>> 4 files changed, 44 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>>>>> b/drivers/staging/android/ion/ion-ioctl.c
>>>>>> index a8d3cc4..60bb702 100644
>>>>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>>>>> @@ -12,6 +12,7 @@
>>>>>>
>>>>>> union ion_ioctl_arg {
>>>>>> struct ion_allocation_data allocation;
>>>>>> + struct ion_buffer_flag_data update;
>>>>>> struct ion_heap_query query;
>>>>>> };
>>>>>>
>>>>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
>>>>>> cmd, unsigned long arg)
>>>>>>
>>>>>> break;
>>>>>> }
>>>>>> + case ION_IOC_BUFFER_UPDATE:
>>>>>> + ret = ion_buffer_update(data.update.fd, data.update.flags);
>>>>>> + break;
>>>>>> case ION_IOC_HEAP_QUERY:
>>>>>> ret = ion_query_heaps(&data.query);
>>>>>> break;
>>>>>> diff --git a/drivers/staging/android/ion/ion.c
>>>>>> b/drivers/staging/android/ion/ion.c
>>>>>> index 9907332..f1404dc 100644
>>>>>> --- a/drivers/staging/android/ion/ion.c
>>>>>> +++ b/drivers/staging/android/ion/ion.c
>>>>>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>>>>> heap_id_mask, unsigned int flags)
>>>>>> return fd;
>>>>>> }
>>>>>>
>>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>>>>>> + struct dma_buf *dmabuf;
>>>>>> + struct ion_buffer *buffer;
>>>>>> +
>>>>>> + dmabuf = dma_buf_get(fd);
>>>>>> +
>>>>>> + if (!dmabuf)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + buffer = dmabuf->priv;
>>>>>> + buffer->flags = flags;
>>>>>> + dma_buf_put(dmabuf);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> int ion_query_heaps(struct ion_heap_query *query)
>>>>>> {
>>>>>> struct ion_device *dev = internal_dev; diff --git
>>>>>> a/drivers/staging/android/ion/ion.h
>>>>>> b/drivers/staging/android/ion/ion.h
>>>>>> index c006fc1..99bf9ab 100644
>>>>>> --- a/drivers/staging/android/ion/ion.h
>>>>>> +++ b/drivers/staging/android/ion/ion.h
>>>>>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page
>*page,
>>>>> size_t size, pgprot_t pgprot);
>>>>>> int ion_alloc(size_t len,
>>>>>> unsigned int heap_id_mask,
>>>>>> unsigned int flags);
>>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>>>>>
>>>>>> /**
>>>>>> * ion_heap_init_shrinker
>>>>>> diff --git a/drivers/staging/android/uapi/ion.h
>>>>>> b/drivers/staging/android/uapi/ion.h
>>>>>> index 5d70098..99753fc 100644
>>>>>> --- a/drivers/staging/android/uapi/ion.h
>>>>>> +++ b/drivers/staging/android/uapi/ion.h
>>>>>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>>>>>> __u32 unused;
>>>>>> };
>>>>>>
>>>>>> +/**
>>>>>> + * struct ion_buffer_flag_data - metadata passed from userspace
>>>>>> +for update
>>>>>> + * buffer flags
>>>>>> + * @fd: file descriptor of the buffer
>>>>>> + * @flags: flags passed to the buffer
>>>>>> + *
>>>>>> + * Provided by userspace as an argument to the ioctl */
>>>>>> +
>>>>>> +struct ion_buffer_flag_data {
>>>>>> + __u32 fd;
>>>>>> + __u32 flags;
>>>>>> +}
>>>>>> +
>>>>>> #define MAX_HEAP_NAME 32
>>>>>>
>>>>>> /**
>>>>>> @@ -116,6 +130,14 @@ struct ion_heap_query {
>>>>>> struct ion_allocation_data)
>>>>>>
>>>>>> /**
>>>>>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion
>buffer
>>>>> flags
>>>>>> + *
>>>>>> + * Takes an ion_buffer_flag_data structure and returns the result
>>>>>> +of the
>>>>>> + * buffer flag update operation.
>>>>>> + */
>>>>>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
>>>>>> + struct ion_buffer_flag_data)
>>>>>> +/**
>>>>>> * DOC: ION_IOC_HEAP_QUERY - information about available
>>> heaps
>>>>>> *
>>>>>> * Takes an ion_heap_query structure and populates
>information
>>>>> about
>>>>>>
>>>>
>>

2019-01-15 04:23:55

by Zengtao (B)

[permalink] [raw]
Subject: RE: [PATCH] staging: android: ion: add buffer flag update ioctl

>-----Original Message-----
>From: Laura Abbott [mailto:[email protected]]
>Sent: Friday, January 04, 2019 9:53 AM
>To: Zengtao (B) <[email protected]>; [email protected]
>Cc: Greg Kroah-Hartman <[email protected]>; Arve Hjønnevåg
><[email protected]>; Todd Kjos <[email protected]>; Martijn Coenen
><[email protected]>; Joel Fernandes <[email protected]>;
>[email protected]; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 1/3/19 5:42 PM, Zengtao (B) wrote:
>>> -----Original Message-----
>>> From: Laura Abbott [mailto:[email protected]]
>>> Sent: Thursday, January 03, 2019 6:31 AM
>>> To: Zengtao (B) <[email protected]>;
>[email protected]
>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>Hjønnevåg
>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>Coenen
>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/23/18 6:47 PM, Zengtao (B) wrote:
>>>> Hi laura:
>>>>
>>>>> -----Original Message-----
>>>>> From: Laura Abbott [mailto:[email protected]]
>>>>> Sent: Friday, December 21, 2018 4:50 AM
>>>>> To: Zengtao (B) <[email protected]>;
>>> [email protected]
>>>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>>> Hjønnevåg
>>>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>>> Coenen
>>>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>>>> [email protected]; [email protected];
>>>>> [email protected]; [email protected]
>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>>>> ioctl
>>>>>
>>>>> On 12/19/18 5:39 PM, Zengtao (B) wrote:
>>>>>> Hi laura:
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Laura Abbott [mailto:[email protected]]
>>>>>>> Sent: Thursday, December 20, 2018 2:10 AM
>>>>>>> To: Zengtao (B) <[email protected]>;
>>>>> [email protected]
>>>>>>> Cc: Greg Kroah-Hartman <[email protected]>; Arve
>>>>> Hjønnevåg
>>>>>>> <[email protected]>; Todd Kjos <[email protected]>; Martijn
>>>>> Coenen
>>>>>>> <[email protected]>; Joel Fernandes <[email protected]>;
>>>>>>> [email protected]; [email protected];
>>>>>>> [email protected]; [email protected]
>>>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag
>>>>>>> update ioctl
>>>>>>>
>>>>>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>>>>>> In some usecases, the buffer cached attribute is not determined
>>>>>>>> at allocation time, it's determined just before the real cpu
>mapping.
>>>>>>>> And from the memory view of point, a buffer should not have
>the
>>>>>>> cached
>>>>>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>>>>>> introduced the new ioctl command to target the requirement.
>>>>>>>>
>>>>>>>
>>>>>>> This is racy and error prone. Can you explain more what problem
>>> you
>>>>>>> are trying to solve?
>>>>>>
>>>>>> My use case is like this:
>>>>>> 1. There are two process A and B, A takes case of ion buffer
>>>>>> allocation,
>>>>> and
>>>>>> pass the buffer fd to B, then B maps and uses it.
>>>>>> 2. Process B need to map the buffer with different cached
>>>>>> attribute for different use case, for example, if the buffer is
>>>>>> used for pure software algorithm, then we need to map it as
>>>>>> cached, otherwise non-cached, and B needs to deal with both
>cases.
>>>>>> And unfortunately the mmap syscall takes no cached flags and we
>>>>>> can't decide the cache attribute when we are doing the mmap, so I
>>>>>> introduce new the ioctl even though I think the solution is not as
>>> good.
>>>>>>
>>>>>>
>>>>>
>>>>> Thanks for the explanation, this was about the use case I expected.
>>>>> I'm pretty sure I had this exact problem once upon a time and we
>>>>> didn't come up with a solution. I'd still like to get rid of
>>>>> uncached buffers in general and just use cached buffers (see
>>>>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2
>>>>> 01
>>>>> 8-N
>>>>> ovember/128842.html)
>>>>> What's your usecase for uncached buffers?
>>>>
>>>> Some buffers are only used by HW, in this case, we tend to use
>>> uncached buffers.
>>>> But sometimes the SW need to read few buffer contents for debug
>>>> purpose and no synchronization is needed.
>>>>
>>>
>>> If this is primarily for debug, that's not really a compelling reason
>>> to support uncached buffers. It's incredibly difficult to do uncached
>>> buffers correctly I'd rather spend effort on making the cached use
>>> cases work better.
>>>
>> No, not only for debug, I meant if the buffers are uncached, the SW
>> will only mmap them for debug or even don't need to mmap them.
>> So for the two kinds of buffers:
>> 1. uncached: only the HW need to access it, the SW don't need to
>mmap it or just for debug.
>> 2. cached: both the HW and the SW need to access it.
>> The ION is designed as a generalized memory manager, I think we
>should
>> cover as many requirements as we can in the ION design, so I 'd rather
>that we keep the uncached support.
>> And I don’t quite understand the difficulty you mentioned to support
>> the uncached buffers, would you explain a little more about it.
>>
>
>We end up with aliasing problems. Each kernel page still has a cached
>mapping so it's very difficult to keep the two mappings in sync.
>https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@DESK
>TOP-E1NTVVP.localdomain/
>This thread does a better job of explaining the problem and the objections
>to uncached buffers.
>
>And if the software never touches the buffer, why does it matter if the
>buffer is uncached?
>

Thanks for the detail info, I get your point, and we know the aliasing issue
but we have never met, so it's hard to explain that we really have that issue.

>Laura
>
>> Thanks
>> Zengtao
>>
>>>
>>>>>
>>>>>>>
>>>>>>>> Signed-off-by: Zeng Tao <[email protected]>
>>>>>>>> ---
>>>>>>>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++
>>>>>>>> drivers/staging/android/ion/ion.c | 17
>>>>> +++++++++++++++++
>>>>>>>> drivers/staging/android/ion/ion.h | 1 +
>>>>>>>> drivers/staging/android/uapi/ion.h | 22
>>>>>>> ++++++++++++++++++++++
>>>>>>>> 4 files changed, 44 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>>>>>>> b/drivers/staging/android/ion/ion-ioctl.c
>>>>>>>> index a8d3cc4..60bb702 100644
>>>>>>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>>>>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>>>>>>> @@ -12,6 +12,7 @@
>>>>>>>>
>>>>>>>> union ion_ioctl_arg {
>>>>>>>> struct ion_allocation_data allocation;
>>>>>>>> + struct ion_buffer_flag_data update;
>>>>>>>> struct ion_heap_query query;
>>>>>>>> };
>>>>>>>>
>>>>>>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
>>>>>>>> cmd, unsigned long arg)
>>>>>>>>
>>>>>>>> break;
>>>>>>>> }
>>>>>>>> + case ION_IOC_BUFFER_UPDATE:
>>>>>>>> + ret = ion_buffer_update(data.update.fd,
>data.update.flags);
>>>>>>>> + break;
>>>>>>>> case ION_IOC_HEAP_QUERY:
>>>>>>>> ret = ion_query_heaps(&data.query);
>>>>>>>> break;
>>>>>>>> diff --git a/drivers/staging/android/ion/ion.c
>>>>>>>> b/drivers/staging/android/ion/ion.c
>>>>>>>> index 9907332..f1404dc 100644
>>>>>>>> --- a/drivers/staging/android/ion/ion.c
>>>>>>>> +++ b/drivers/staging/android/ion/ion.c
>>>>>>>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>>>>>>> heap_id_mask, unsigned int flags)
>>>>>>>> return fd;
>>>>>>>> }
>>>>>>>>
>>>>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>>>>>>>> + struct dma_buf *dmabuf;
>>>>>>>> + struct ion_buffer *buffer;
>>>>>>>> +
>>>>>>>> + dmabuf = dma_buf_get(fd);
>>>>>>>> +
>>>>>>>> + if (!dmabuf)
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + buffer = dmabuf->priv;
>>>>>>>> + buffer->flags = flags;
>>>>>>>> + dma_buf_put(dmabuf);
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> int ion_query_heaps(struct ion_heap_query *query)
>>>>>>>> {
>>>>>>>> struct ion_device *dev = internal_dev; diff --git
>>>>>>>> a/drivers/staging/android/ion/ion.h
>>>>>>>> b/drivers/staging/android/ion/ion.h
>>>>>>>> index c006fc1..99bf9ab 100644
>>>>>>>> --- a/drivers/staging/android/ion/ion.h
>>>>>>>> +++ b/drivers/staging/android/ion/ion.h
>>>>>>>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page
>>> *page,
>>>>>>> size_t size, pgprot_t pgprot);
>>>>>>>> int ion_alloc(size_t len,
>>>>>>>> unsigned int heap_id_mask,
>>>>>>>> unsigned int flags);
>>>>>>>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>>>>>>>
>>>>>>>> /**
>>>>>>>> * ion_heap_init_shrinker
>>>>>>>> diff --git a/drivers/staging/android/uapi/ion.h
>>>>>>>> b/drivers/staging/android/uapi/ion.h
>>>>>>>> index 5d70098..99753fc 100644
>>>>>>>> --- a/drivers/staging/android/uapi/ion.h
>>>>>>>> +++ b/drivers/staging/android/uapi/ion.h
>>>>>>>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>>>>>>>> __u32 unused;
>>>>>>>> };
>>>>>>>>
>>>>>>>> +/**
>>>>>>>> + * struct ion_buffer_flag_data - metadata passed from
>userspace
>>>>>>>> +for update
>>>>>>>> + * buffer flags
>>>>>>>> + * @fd: file descriptor of the buffer
>>>>>>>> + * @flags: flags passed to the buffer
>>>>>>>> + *
>>>>>>>> + * Provided by userspace as an argument to the ioctl */
>>>>>>>> +
>>>>>>>> +struct ion_buffer_flag_data {
>>>>>>>> + __u32 fd;
>>>>>>>> + __u32 flags;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> #define MAX_HEAP_NAME 32
>>>>>>>>
>>>>>>>> /**
>>>>>>>> @@ -116,6 +130,14 @@ struct ion_heap_query {
>>>>>>>> struct ion_allocation_data)
>>>>>>>>
>>>>>>>> /**
>>>>>>>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion
>>> buffer
>>>>>>> flags
>>>>>>>> + *
>>>>>>>> + * Takes an ion_buffer_flag_data structure and returns the
>>>>>>>> +result of the
>>>>>>>> + * buffer flag update operation.
>>>>>>>> + */
>>>>>>>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC,
>1, \
>>>>>>>> + struct ion_buffer_flag_data)
>>>>>>>> +/**
>>>>>>>> * DOC: ION_IOC_HEAP_QUERY - information about
>available
>>>>> heaps
>>>>>>>> *
>>>>>>>> * Takes an ion_heap_query structure and populates
>>> information
>>>>>>> about
>>>>>>>>
>>>>>>
>>>>
>>