[RFC v3] genalloc:support memory-allocation with bytes-alignment to genalloc

[RFC v3] genalloc:support memory-allocation with bytes-alignment to genalloc

[Zhao Qiang]

Bytes alignment is required to manage some special RAM,
so add gen_pool_first_fit_align to genalloc,
meanwhile add gen_pool_alloc_data to pass data to
gen_pool_first_fit_align(modify gen_pool_alloc as a wrapper)

Signed-off-by: Zhao Qiang <qiang.zhao@freescale.com>
---
*v2:
changes:
title has been modified, original patch link: 
http://patchwork.ozlabs.org/patch/493297/

original patch add a func gen_pool_alloc_align, 
then pass alignment to it as an parameter.
after discussing with lauraa and scott, they recommend 
to pass alignment as part of data based on 
commit message for ca279cf1065fb689abea1dc7d8c11787729bb185 which adds "data":

"As I can't predict all the possible requirements/needs for all allocation    
uses cases, I add a "free" field 'void *data' to pass any needed     
information to the allocation function.  For example 'data' could be used     
to handle a structure where you store the alignment, the expected memory     
bank, the requester device, or any information that could influence the     
allocation algorithm."

*v3:
changes:
title has been modified, original patch link: 
http://patchwork.ozlabs.org/patch/500317/

according to the comments, add gen_pool_alloc_data,
modify gen_pool_alloc as a wrapper, define struct data_align
for gen_pool_first_fit_align algorithm. add parameter 
pointer pool to algorithm.

 include/linux/genalloc.h | 23 +++++++++++++++---
 lib/genalloc.c           | 63 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 1ccaab4..760af91 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -34,6 +34,7 @@
 
 struct device;
 struct device_node;
+struct gen_pool;
 
 /**
  * Allocation callback function type definition
@@ -47,7 +48,7 @@ typedef unsigned long (*genpool_algo_t)(unsigned long *map,
 			unsigned long size,
 			unsigned long start,
 			unsigned int nr,
-			void *data);
+			void *data, struct gen_pool *pool);
 
 /*
  *  General purpose special memory pool descriptor.
@@ -73,6 +74,13 @@ struct gen_pool_chunk {
 	unsigned long bits[0];		/* bitmap for allocating memory chunk */
 };
 
+/*
+ *  General purpose special memory pool data descriptor.
+ */
+struct data_align {
+	int align;		/* alignment by bytes for starting address */
+};
+
 extern struct gen_pool *gen_pool_create(int, int);
 extern phys_addr_t gen_pool_virt_to_phys(struct gen_pool *pool, unsigned long);
 extern int gen_pool_add_virt(struct gen_pool *, unsigned long, phys_addr_t,
@@ -96,6 +104,7 @@ static inline int gen_pool_add(struct gen_pool *pool, unsigned long addr,
 }
 extern void gen_pool_destroy(struct gen_pool *);
 extern unsigned long gen_pool_alloc(struct gen_pool *, size_t);
+extern unsigned long gen_pool_alloc_data(struct gen_pool *, size_t, void *data);
 extern void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size,
 		dma_addr_t *dma);
 extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
@@ -108,14 +117,20 @@ extern void gen_pool_set_algo(struct gen_pool *pool, genpool_algo_t algo,
 		void *data);
 
 extern unsigned long gen_pool_first_fit(unsigned long *map, unsigned long size,
-		unsigned long start, unsigned int nr, void *data);
+		unsigned long start, unsigned int nr, void *data,
+		struct gen_pool *pool);
+
+extern unsigned long gen_pool_first_fit_align(unsigned long *map,
+		unsigned long size, unsigned long start, unsigned int nr,
+		void *data, struct gen_pool *pool);
 
 extern unsigned long gen_pool_first_fit_order_align(unsigned long *map,
 		unsigned long size, unsigned long start, unsigned int nr,
-		void *data);
+		void *data, struct gen_pool *pool);
 
 extern unsigned long gen_pool_best_fit(unsigned long *map, unsigned long size,
-		unsigned long start, unsigned int nr, void *data);
+		unsigned long start, unsigned int nr, void *data,
+		struct gen_pool *pool);
 
 extern struct gen_pool *devm_gen_pool_create(struct device *dev,
 		int min_alloc_order, int nid);
diff --git a/lib/genalloc.c b/lib/genalloc.c
index d214866..e0b737c 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -269,6 +269,24 @@ EXPORT_SYMBOL(gen_pool_destroy);
  */
 unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
 {
+	return gen_pool_alloc_data(pool, size, NULL);
+}
+EXPORT_SYMBOL(gen_pool_alloc);
+
+/**
+ * gen_pool_alloc_data - allocate special memory from the pool
+ * @pool: pool to allocate from
+ * @size: number of bytes to allocate from the pool
+ * @data: data passed to algorithm
+ *
+ * Allocate the requested number of bytes from the specified pool.
+ * Uses the pool allocation function (with first-fit algorithm by default).
+ * Can not be used in NMI handler on architectures without
+ * NMI-safe cmpxchg implementation.
+ */
+unsigned long gen_pool_alloc_data(struct gen_pool *pool, size_t size,
+		void *data)
+{
 	struct gen_pool_chunk *chunk;
 	unsigned long addr = 0;
 	int order = pool->min_alloc_order;
@@ -290,7 +308,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
 		end_bit = chunk_size(chunk) >> order;
 retry:
 		start_bit = pool->algo(chunk->bits, end_bit, start_bit, nbits,
-				pool->data);
+				pool->data, pool);
 		if (start_bit >= end_bit)
 			continue;
 		remain = bitmap_set_ll(chunk->bits, start_bit, nbits);
@@ -309,7 +327,7 @@ retry:
 	rcu_read_unlock();
 	return addr;
 }
-EXPORT_SYMBOL(gen_pool_alloc);
+EXPORT_SYMBOL(gen_pool_alloc_data);
 
 /**
  * gen_pool_dma_alloc - allocate special memory from the pool for DMA usage
@@ -500,15 +518,47 @@ EXPORT_SYMBOL(gen_pool_set_algo);
  * @start: The bitnumber to start searching at
  * @nr: The number of zeroed bits we're looking for
  * @data: additional data - unused
+ * @pool: unused, but the algorithm functions have same format.
  */
 unsigned long gen_pool_first_fit(unsigned long *map, unsigned long size,
-		unsigned long start, unsigned int nr, void *data)
+		unsigned long start, unsigned int nr, void *data,
+		struct gen_pool *pool)
 {
 	return bitmap_find_next_zero_area(map, size, start, nr, 0);
 }
 EXPORT_SYMBOL(gen_pool_first_fit);
 
 /**
+ * gen_pool_first_fit_align - find the first available region
+ * of memory matching the size requirement (alignment constraint)
+ * @map: The address to base the search on
+ * @size: The bitmap size in bits
+ * @start: The bitnumber to start searching at
+ * @nr: The number of zeroed bits we're looking for
+ * @data: data for alignment
+ * @pool: get order from pool
+ */
+unsigned long gen_pool_first_fit_align(unsigned long *map, unsigned long size,
+		unsigned long start, unsigned int nr, void *data,
+		struct gen_pool *pool)
+{
+	struct data_align *alignment;
+	unsigned long align_mask;
+	int order;
+
+	if (data == NULL)
+		return -EINVAL;
+
+	alignment = (struct data_align *)data;
+
+	order = pool->min_alloc_order;
+	align_mask = ((alignment->align + (1UL << order) - 1) >> order) - 1;
+
+	return bitmap_find_next_zero_area(map, size, start, nr, align_mask);
+}
+EXPORT_SYMBOL(gen_pool_first_fit_align);
+
+/**
  * gen_pool_first_fit_order_align - find the first available region
  * of memory matching the size requirement. The region will be aligned
  * to the order of the size specified.
@@ -517,10 +567,11 @@ EXPORT_SYMBOL(gen_pool_first_fit);
  * @start: The bitnumber to start searching at
  * @nr: The number of zeroed bits we're looking for
  * @data: additional data - unused
+ * @pool: unused, but the algorithm functions have same format.
  */
 unsigned long gen_pool_first_fit_order_align(unsigned long *map,
 		unsigned long size, unsigned long start,
-		unsigned int nr, void *data)
+		unsigned int nr, void *data, struct gen_pool *pool)
 {
 	unsigned long align_mask = roundup_pow_of_two(nr) - 1;
 
@@ -536,12 +587,14 @@ EXPORT_SYMBOL(gen_pool_first_fit_order_align);
  * @start: The bitnumber to start searching at
  * @nr: The number of zeroed bits we're looking for
  * @data: additional data - unused
+ * @pool: unused, but the algorithm functions have same format.
  *
  * Iterate over the bitmap to find the smallest free region
  * which we can allocate the memory.
  */
 unsigned long gen_pool_best_fit(unsigned long *map, unsigned long size,
-		unsigned long start, unsigned int nr, void *data)
+		unsigned long start, unsigned int nr, void *data,
+		struct gen_pool *pool)
 {
 	unsigned long start_bit = size;
 	unsigned long len = size + 1;
-- 
2.1.0.27.g96db324

Re: [RFC v3] genalloc:support memory-allocation with bytes-alignment to genalloc

[Scott Wood]

On Mon, 2015-08-03 at 16:35 +0800, Zhao Qiang wrote:
> 
> @@ -73,6 +74,13 @@ struct gen_pool_chunk {
>       unsigned long bits[0];          /* bitmap for allocating memory chunk */
>  };
>  
> +/*
> + *  General purpose special memory pool data descriptor.
> + */

It's not "general purpose".  It's for gen_pool_first_fit_align (and could be 
reused by any future algorithms that need alignment but nothing else).

> +struct data_align {
> +     int align;              /* alignment by bytes for starting address */
> +};

struct genpool_data_align


> diff --git a/lib/genalloc.c b/lib/genalloc.c
> index d214866..e0b737c 100644
> --- a/lib/genalloc.c
> +++ b/lib/genalloc.c
> @@ -269,6 +269,24 @@ EXPORT_SYMBOL(gen_pool_destroy);
>   */
>  unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
>  {
> +     return gen_pool_alloc_data(pool, size, NULL);
> +}
> +EXPORT_SYMBOL(gen_pool_alloc);

This should pass in pool->data, not NULL.

> +
> +/**
> + * gen_pool_alloc_data - allocate special memory from the pool
> + * @pool: pool to allocate from
> + * @size: number of bytes to allocate from the pool
> + * @data: data passed to algorithm
> + *
> + * Allocate the requested number of bytes from the specified pool.
> + * Uses the pool allocation function (with first-fit algorithm by default).
> + * Can not be used in NMI handler on architectures without
> + * NMI-safe cmpxchg implementation.
> + */
> +unsigned long gen_pool_alloc_data(struct gen_pool *pool, size_t size,
> +             void *data)
> +{
>       struct gen_pool_chunk *chunk;
>       unsigned long addr = 0;
>       int order = pool->min_alloc_order;
> @@ -290,7 +308,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool, 
> size_t size)
>               end_bit = chunk_size(chunk) >> order;
>  retry:
>               start_bit = pool->algo(chunk->bits, end_bit, start_bit, nbits,
> -                             pool->data);
> +                             pool->data, pool);


This should pass in data, not pool->data.  Currently you don't use the data 
argument at all.


> +unsigned long gen_pool_first_fit_align(unsigned long *map, unsigned long 
> size,
> +             unsigned long start, unsigned int nr, void *data,
> +             struct gen_pool *pool)
> +{
> +     struct data_align *alignment;
> +     unsigned long align_mask;
> +     int order;
> +
> +     if (data == NULL)
> +             return -EINVAL;

Unnecessary check.  If this happens it will happen quickly on the developer's 
machine, so let it oops and be obvious.

> +
> +     alignment = (struct data_align *)data;

Unnecessary cast.

-Scott