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

[Laura Abbott]

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 <prime.zeng@xxxxxxxxxxxxx>
> ---
>   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