[PATCH] staging: android: ion: Add chunk heaps instantiation

Laura Abbott labbott at redhat.com
Thu Dec 20 20:36:20 UTC 2018


On 12/16/18 2:46 AM, Alexey Skidanov wrote:
> Chunk heap instantiation should be supported for device tree platforms and
> non device tree platforms. For device tree platforms, it's a platform
> specific code responsibility to retrieve the heap configuration data
> and to call the appropriate API for heap creation. For non device tree
> platforms, there is no defined way to create the heaps.
> 
> This patch provides the way of chunk heaps creation using
> "ion_chunk_heap=name:size at start" kernel boot parameter.
> 

I've been thinking about this and I think it works but
I'm still kind of torn about not having devicetree bindings.
It doesn't _preclude_ devicetree bindings but I'd hate to
merge this and then end up with something incompatible.
I do want to support non-Android use cases too.

I'm also curious about the value of this heap with just PAGE_SIZE.
The original purpose of the chunk heap was a carveout where
you could easily get allocations large than PAGE_SIZE to
reduce TLB pressure. Do you have another use case for the
chunk heap?

Sumit, do you have any thoughts?

Thanks,
Laura

> Link: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128495.html
> Signed-off-by: Alexey Skidanov <alexey.skidanov at intel.com>
> ---
>   drivers/staging/android/ion/ion_chunk_heap.c | 96 +++++++++++++++++++++++++---
>   include/linux/ion.h                          | 18 ++++++
>   2 files changed, 105 insertions(+), 9 deletions(-)
>   create mode 100644 include/linux/ion.h
> 
> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c b/drivers/staging/android/ion/ion_chunk_heap.c
> index 102c093..1a8e3ad 100644
> --- a/drivers/staging/android/ion/ion_chunk_heap.c
> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
> @@ -21,8 +21,12 @@
>   #include <linux/scatterlist.h>
>   #include <linux/slab.h>
>   #include <linux/vmalloc.h>
> +#include <linux/ion.h>
>   #include "ion.h"
>   
> +static struct ion_chunk_heap_cfg chunk_heap_cfg[MAX_NUM_OF_CHUNK_HEAPS];
> +static unsigned int num_of_req_chunk_heaps;
> +
>   struct ion_chunk_heap {
>   	struct ion_heap heap;
>   	struct gen_pool *pool;
> @@ -117,15 +121,15 @@ static struct ion_heap_ops chunk_heap_ops = {
>   	.unmap_kernel = ion_heap_unmap_kernel,
>   };
>   
> -struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
> +static struct ion_heap *ion_chunk_heap_create(struct ion_chunk_heap_cfg *heap_cfg)
>   {
>   	struct ion_chunk_heap *chunk_heap;
>   	int ret;
>   	struct page *page;
>   	size_t size;
>   
> -	page = pfn_to_page(PFN_DOWN(heap_data->base));
> -	size = heap_data->size;
> +	page = pfn_to_page(PFN_DOWN(heap_cfg->base));
> +	size = heap_cfg->size;
>   
>   	ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
>   	if (ret)
> @@ -135,23 +139,27 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
>   	if (!chunk_heap)
>   		return ERR_PTR(-ENOMEM);
>   
> -	chunk_heap->chunk_size = (unsigned long)heap_data->priv;
> +	chunk_heap->chunk_size = heap_cfg->chunk_size;
>   	chunk_heap->pool = gen_pool_create(get_order(chunk_heap->chunk_size) +
>   					   PAGE_SHIFT, -1);
>   	if (!chunk_heap->pool) {
>   		ret = -ENOMEM;
>   		goto error_gen_pool_create;
>   	}
> -	chunk_heap->base = heap_data->base;
> -	chunk_heap->size = heap_data->size;
> +	chunk_heap->base = heap_cfg->base;
> +	chunk_heap->size = heap_cfg->size;
> +	chunk_heap->heap.name = heap_cfg->heap_name;
>   	chunk_heap->allocated = 0;
>   
> -	gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_data->size, -1);
> +	gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_cfg->size, -1);
>   	chunk_heap->heap.ops = &chunk_heap_ops;
>   	chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
>   	chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
> -	pr_debug("%s: base %pa size %zu\n", __func__,
> -		 &chunk_heap->base, heap_data->size);
> +
> +	pr_info("%s: name %s base %pa size %zu\n", __func__,
> +		heap_cfg->heap_name,
> +		&heap_cfg->base,
> +		heap_cfg->size);
>   
>   	return &chunk_heap->heap;
>   
> @@ -160,3 +168,73 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
>   	return ERR_PTR(ret);
>   }
>   
> +static int __init setup_heap(char *param)
> +{
> +	char *at_sign, *coma, *colon;
> +	size_t size_to_copy;
> +	struct ion_chunk_heap_cfg *cfg;
> +
> +	do {
> +		cfg = &chunk_heap_cfg[num_of_req_chunk_heaps];
> +
> +		/* heap name */
> +		colon = strchr(param, ':');
> +		if (!colon)
> +			return -EINVAL;
> +
> +		size_to_copy = min_t(size_t, MAX_CHUNK_HEAP_NAME_SIZE - 1,
> +				     (colon - param));
> +		strncpy(cfg->heap_name,	param, size_to_copy);
> +		cfg->heap_name[size_to_copy] = '\0';
> +
> +		/* heap size */
> +		cfg->size = memparse((colon + 1), &at_sign);
> +		if ((colon + 1) == at_sign)
> +			return -EINVAL;
> +
> +		/* heap base addr */
> +		if (*at_sign == '@')
> +			cfg->base = memparse(at_sign + 1, &coma);
> +		else
> +			return -EINVAL;
> +
> +		if (at_sign == coma)
> +			return -EINVAL;
> +
> +		/* Chunk size */
> +		cfg->chunk_size = PAGE_SIZE;
> +
> +		num_of_req_chunk_heaps++;
> +
> +		/* if one more heap configuration exists */
> +		if (*coma == ',')
> +			param = coma + 1;
> +		else
> +			param = NULL;
> +	} while (num_of_req_chunk_heaps < MAX_NUM_OF_CHUNK_HEAPS && param);
> +
> +	return 0;
> +}
> +
> +__setup("ion_chunk_heap=", setup_heap);
> +
> +int ion_add_chunk_heaps(struct ion_chunk_heap_cfg *cfg,
> +			unsigned int num_of_heaps)
> +{
> +	unsigned int i;
> +	struct ion_heap *heap;
> +
> +	for (i = 0; i < num_of_heaps; i++) {
> +		heap = ion_chunk_heap_create(&cfg[i]);
> +		if (heap)
> +			ion_device_add_heap(heap);
> +	}
> +	return 0;
> +}
> +
> +static int ion_add_chunk_heaps_from_boot_param(void)
> +{
> +	return ion_add_chunk_heaps(chunk_heap_cfg, num_of_req_chunk_heaps);
> +}
> +
> +device_initcall(ion_add_chunk_heaps_from_boot_param);
> diff --git a/include/linux/ion.h b/include/linux/ion.h
> new file mode 100644
> index 0000000..60296c9
> --- /dev/null
> +++ b/include/linux/ion.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_ION_H
> +#define _LINUX_ION_H
> +
> +#define MAX_NUM_OF_CHUNK_HEAPS 32
> +#define MAX_CHUNK_HEAP_NAME_SIZE 32
> +
> +struct ion_chunk_heap_cfg {
> +	char heap_name[MAX_CHUNK_HEAP_NAME_SIZE];
> +	phys_addr_t base;
> +	size_t size;
> +	size_t chunk_size;
> +};
> +
> +int ion_add_chunk_heaps(struct ion_chunk_heap_cfg *cfg,
> +			unsigned int  num_of_heaps);
> +
> +#endif /* _LINUX_ION_H */
> 



More information about the devel mailing list