[RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Zhao Qiang]

Bytes alignment is required to manage some special ram,
so add gen_pool_alloc_align func to genalloc.
rename gen_pool_alloc to gen_pool_alloc_align with a align parameter,
then provide gen_pool_alloc to call gen_pool_alloc_align with
align = 1 Byte.

Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
FSL's IP block QE require this function to manage muram.
QE supported only PowerPC, and its code was put under arch/powerpc directory,
using arch/powerpc/lib/rheap.c to manage muram.
Now it support both arm(ls1021,ls1043,ls2085 and such on) and powerpc,
the code need to move from arch/powerpc to public direcory,
Scott wood hopes to use genalloc to manage the muram, after discussing 
with scott, we decide to add gen_pool_alloc_align to meet the requirement
for bytes-alignment.

 include/linux/genalloc.h | 10 +++++++---
 lib/genalloc.c           | 38 ++++++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 1ccaab4..65fdf14 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -96,6 +96,8 @@ 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_align(struct gen_pool *, size_t,
+		unsigned long align);
 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 +110,16 @@ 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,
+		unsigned long align_mask);
 
 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, unsigned long align_mask);
 
 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,
+		unsigned long align_mask);
 
 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..dd63448 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -258,19 +258,22 @@ void gen_pool_destroy(struct gen_pool *pool)
 EXPORT_SYMBOL(gen_pool_destroy);
 
 /**
- * gen_pool_alloc - allocate special memory from the pool
+ * gen_pool_alloc_align - allocate special memory from the pool
  * @pool: pool to allocate from
  * @size: number of bytes to allocate from the pool
+ * @align: number of bytes to align
  *
  * 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(struct gen_pool *pool, size_t size)
+unsigned long gen_pool_alloc_align(struct gen_pool *pool, size_t size,
+		unsigned long align)
 {
 	struct gen_pool_chunk *chunk;
 	unsigned long addr = 0;
+	unsigned long align_mask;
 	int order = pool->min_alloc_order;
 	int nbits, start_bit = 0, end_bit, remain;
 
@@ -281,6 +284,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
 	if (size == 0)
 		return 0;
 
+	align_mask = ((align + (1UL << order) - 1) >> order) - 1;
 	nbits = (size + (1UL << order) - 1) >> order;
 	rcu_read_lock();
 	list_for_each_entry_rcu(chunk, &pool->chunks, next_chunk) {
@@ -290,7 +294,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, align_mask);
 		if (start_bit >= end_bit)
 			continue;
 		remain = bitmap_set_ll(chunk->bits, start_bit, nbits);
@@ -309,6 +313,22 @@ retry:
 	rcu_read_unlock();
 	return addr;
 }
+EXPORT_SYMBOL(gen_pool_alloc_align);
+
+/**
+ * gen_pool_alloc - allocate special memory from the pool
+ * @pool: pool to allocate from
+ * @size: number of bytes to allocate from the pool
+ *
+ * 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(struct gen_pool *pool, size_t size)
+{
+	return gen_pool_alloc_align(pool, size, 1);
+}
 EXPORT_SYMBOL(gen_pool_alloc);
 
 /**
@@ -502,9 +522,10 @@ EXPORT_SYMBOL(gen_pool_set_algo);
  * @data: additional data - unused
  */
 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,
+		unsigned long align_mask)
 {
-	return bitmap_find_next_zero_area(map, size, start, nr, 0);
+	return bitmap_find_next_zero_area(map, size, start, nr, align_mask);
 }
 EXPORT_SYMBOL(gen_pool_first_fit);
 
@@ -520,7 +541,7 @@ EXPORT_SYMBOL(gen_pool_first_fit);
  */
 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, unsigned long align_mask)
 {
 	unsigned long align_mask = roundup_pow_of_two(nr) - 1;
 
@@ -541,13 +562,14 @@ EXPORT_SYMBOL(gen_pool_first_fit_order_align);
  * 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, unsigned long align_mask)
 {
 	unsigned long start_bit = size;
 	unsigned long len = size + 1;
 	unsigned long index;
 
-	index = bitmap_find_next_zero_area(map, size, start, nr, 0);
+	index = bitmap_find_next_zero_area(map, size, start, nr, align_mask);
 
 	while (index < size) {
 		int next_bit = find_next_bit(map, size, index + nr);
-- 
2.1.0.27.g96db324

Re: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Laura Abbott]

On 07/09/2015 12:47 AM, Zhao Qiang wrote:
> Bytes alignment is required to manage some special ram,
> so add gen_pool_alloc_align func to genalloc.
> rename gen_pool_alloc to gen_pool_alloc_align with a align parameter,
> then provide gen_pool_alloc to call gen_pool_alloc_align with
> align = 1 Byte.
>
> Signed-off-by: Zhao Qiang <B45475@freescale.com>
> ---
> FSL's IP block QE require this function to manage muram.
> QE supported only PowerPC, and its code was put under arch/powerpc directory,
> using arch/powerpc/lib/rheap.c to manage muram.
> Now it support both arm(ls1021,ls1043,ls2085 and such on) and powerpc,
> the code need to move from arch/powerpc to public direcory,
> Scott wood hopes to use genalloc to manage the muram, after discussing
> with scott, we decide to add gen_pool_alloc_align to meet the requirement
> for bytes-alignment.

gen_pool supports custom allocation algorithms. I thought this was discussed
previously and the conclusion was that if you wanted alignment you should
use custom allocation algorithms. I'm failing at finding any thread discussing
it though.

Perhaps another option would be to add another runtime argument to gen_pool
where you could pass the alignment to your custom allocation function. This
way alignment isn't inherently coded into any of the algorithms.

>
>   include/linux/genalloc.h | 10 +++++++---
>   lib/genalloc.c           | 38 ++++++++++++++++++++++++++++++--------
>   2 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
> index 1ccaab4..65fdf14 100644
> --- a/include/linux/genalloc.h
> +++ b/include/linux/genalloc.h
> @@ -96,6 +96,8 @@ 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_align(struct gen_pool *, size_t,
> +		unsigned long align);
>   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 +110,16 @@ 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,
> +		unsigned long align_mask);
>
>   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, unsigned long align_mask);
>
>   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,
> +		unsigned long align_mask);
>
>   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..dd63448 100644
> --- a/lib/genalloc.c
> +++ b/lib/genalloc.c
> @@ -258,19 +258,22 @@ void gen_pool_destroy(struct gen_pool *pool)
>   EXPORT_SYMBOL(gen_pool_destroy);
>
>   /**
> - * gen_pool_alloc - allocate special memory from the pool
> + * gen_pool_alloc_align - allocate special memory from the pool
>    * @pool: pool to allocate from
>    * @size: number of bytes to allocate from the pool
> + * @align: number of bytes to align
>    *
>    * 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(struct gen_pool *pool, size_t size)
> +unsigned long gen_pool_alloc_align(struct gen_pool *pool, size_t size,
> +		unsigned long align)
>   {
>   	struct gen_pool_chunk *chunk;
>   	unsigned long addr = 0;
> +	unsigned long align_mask;
>   	int order = pool->min_alloc_order;
>   	int nbits, start_bit = 0, end_bit, remain;
>
> @@ -281,6 +284,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
>   	if (size == 0)
>   		return 0;
>
> +	align_mask = ((align + (1UL << order) - 1) >> order) - 1;
>   	nbits = (size + (1UL << order) - 1) >> order;
>   	rcu_read_lock();
>   	list_for_each_entry_rcu(chunk, &pool->chunks, next_chunk) {
> @@ -290,7 +294,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, align_mask);
>   		if (start_bit >= end_bit)
>   			continue;
>   		remain = bitmap_set_ll(chunk->bits, start_bit, nbits);
> @@ -309,6 +313,22 @@ retry:
>   	rcu_read_unlock();
>   	return addr;
>   }
> +EXPORT_SYMBOL(gen_pool_alloc_align);
> +
> +/**
> + * gen_pool_alloc - allocate special memory from the pool
> + * @pool: pool to allocate from
> + * @size: number of bytes to allocate from the pool
> + *
> + * 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(struct gen_pool *pool, size_t size)
> +{
> +	return gen_pool_alloc_align(pool, size, 1);

Passing 1 here would change the behavior of the existing algorithms which
were passing 0 for the align mask

> +}
>   EXPORT_SYMBOL(gen_pool_alloc);
>
>   /**
> @@ -502,9 +522,10 @@ EXPORT_SYMBOL(gen_pool_set_algo);
>    * @data: additional data - unused
>    */
>   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,
> +		unsigned long align_mask)
>   {
> -	return bitmap_find_next_zero_area(map, size, start, nr, 0);
> +	return bitmap_find_next_zero_area(map, size, start, nr, align_mask);
>   }
>   EXPORT_SYMBOL(gen_pool_first_fit);
>
> @@ -520,7 +541,7 @@ EXPORT_SYMBOL(gen_pool_first_fit);
>    */
>   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, unsigned long align_mask)
>   {
>   	unsigned long align_mask = roundup_pow_of_two(nr) - 1;
>
> @@ -541,13 +562,14 @@ EXPORT_SYMBOL(gen_pool_first_fit_order_align);
>    * 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, unsigned long align_mask)
>   {
>   	unsigned long start_bit = size;
>   	unsigned long len = size + 1;
>   	unsigned long index;
>
> -	index = bitmap_find_next_zero_area(map, size, start, nr, 0);
> +	index = bitmap_find_next_zero_area(map, size, start, nr, align_mask);
>
>   	while (index < size) {
>   		int next_bit = find_next_bit(map, size, index + nr);
>

Thanks,
Laura

Re: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Scott Wood]

On Thu, 2015-07-09 at 14:51 -0700, Laura Abbott wrote:
> On 07/09/2015 12:47 AM, Zhao Qiang wrote:
> > Bytes alignment is required to manage some special ram,
> > so add gen_pool_alloc_align func to genalloc.
> > rename gen_pool_alloc to gen_pool_alloc_align with a align parameter,
> > then provide gen_pool_alloc to call gen_pool_alloc_align with
> > align = 1 Byte.
> > 
> > Signed-off-by: Zhao Qiang <B45475@freescale.com>
> > ---
> > FSL's IP block QE require this function to manage muram.
> > QE supported only PowerPC, and its code was put under arch/powerpc 
> > directory,
> > using arch/powerpc/lib/rheap.c to manage muram.
> > Now it support both arm(ls1021,ls1043,ls2085 and such on) and powerpc,
> > the code need to move from arch/powerpc to public direcory,
> > Scott wood hopes to use genalloc to manage the muram, after discussing
> > with scott, we decide to add gen_pool_alloc_align to meet the requirement
> > for bytes-alignment.
> 
> gen_pool supports custom allocation algorithms. I thought this was discussed
> previously and the conclusion was that if you wanted alignment you should
> use custom allocation algorithms. I'm failing at finding any thread 
> discussing it though.

I hope that by "custom algorithm" you don't mean something implemented 
outside lib/genalloc.c, as this does not seem like such a specialized 
requirement that everyone must reimplement it separately.

> Perhaps another option would be to add another runtime argument to gen_pool
> where you could pass the alignment to your custom allocation function. This
> way alignment isn't inherently coded into any of the algorithms.

That wouldn't let the alignment change for each allocation (and could already 
be done with pool->data).  I suppose one could call get_pool_set_algo() with 
different data (or modify the memory that pool->data is already pointing to) 
before each allocation, but that's a bit clunky...  If making alignment part 
of the mainstream flow is undesired for some reason, how about a 
gen_pool_alloc_data() that lets it be passed in per-allocation (with 
gen_pool_alloc() being a wrapper that passes in pool->data)?

Yes, I know, we could do it in a wrapper (like cpm_muram_alloc() 
unnecessarily does), but why not make the interface better match the way it's 
used?

-Scott

Re: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Scott Wood]

On Thu, 2015-07-09 at 15:47 +0800, Zhao Qiang wrote:
> @@ -541,13 +562,14 @@ EXPORT_SYMBOL(gen_pool_first_fit_order_align);
>   * 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, unsigned long align_mask)
>  {
>       unsigned long start_bit = size;
>       unsigned long len = size + 1;
>       unsigned long index;
>  
> -     index = bitmap_find_next_zero_area(map, size, start, nr, 0);
> +     index = bitmap_find_next_zero_area(map, size, start, nr, align_mask);
>  
>       while (index < size) {
>               int next_bit = find_next_bit(map, size, index + nr);

What about the other call to bitmap_find_next_zero_area()?

-Scott

Re: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Laura Abbott]

On 07/09/2015 03:17 PM, Scott Wood wrote:
> On Thu, 2015-07-09 at 14:51 -0700, Laura Abbott wrote:
>> On 07/09/2015 12:47 AM, Zhao Qiang wrote:
>>> Bytes alignment is required to manage some special ram,
>>> so add gen_pool_alloc_align func to genalloc.
>>> rename gen_pool_alloc to gen_pool_alloc_align with a align parameter,
>>> then provide gen_pool_alloc to call gen_pool_alloc_align with
>>> align = 1 Byte.
>>>
>>> Signed-off-by: Zhao Qiang <B45475@freescale.com>
>>> ---
>>> FSL's IP block QE require this function to manage muram.
>>> QE supported only PowerPC, and its code was put under arch/powerpc
>>> directory,
>>> using arch/powerpc/lib/rheap.c to manage muram.
>>> Now it support both arm(ls1021,ls1043,ls2085 and such on) and powerpc,
>>> the code need to move from arch/powerpc to public direcory,
>>> Scott wood hopes to use genalloc to manage the muram, after discussing
>>> with scott, we decide to add gen_pool_alloc_align to meet the requirement
>>> for bytes-alignment.
>>
>> gen_pool supports custom allocation algorithms. I thought this was discussed
>> previously and the conclusion was that if you wanted alignment you should
>> use custom allocation algorithms. I'm failing at finding any thread
>> discussing it though.
>
> I hope that by "custom algorithm" you don't mean something implemented
> outside lib/genalloc.c, as this does not seem like such a specialized
> requirement that everyone must reimplement it separately.
>

If the functions are generic enough (which I think they are) they could stay
in genalloc.c as another option for people to use.

>> Perhaps another option would be to add another runtime argument to gen_pool
>> where you could pass the alignment to your custom allocation function. This
>> way alignment isn't inherently coded into any of the algorithms.
>
> That wouldn't let the alignment change for each allocation (and could already
> be done with pool->data).  I suppose one could call get_pool_set_algo() with
> different data (or modify the memory that pool->data is already pointing to)
> before each allocation, but that's a bit clunky...  If making alignment part
> of the mainstream flow is undesired for some reason, how about a
> gen_pool_alloc_data() that lets it be passed in per-allocation (with
> gen_pool_alloc() being a wrapper that passes in pool->data)?
>

Yes, that's what I was thinking. I dropped the alloc from my 'runtime argument
to gen_pool_alloc' so it wasn't clear I was talking about allocation time and
not pool creation time.

> Yes, I know, we could do it in a wrapper (like cpm_muram_alloc()
> unnecessarily does), but why not make the interface better match the way it's
> used?

Agreed.

>
> -Scott
>

Thanks,
Laura

RE: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Zhao Qiang]

DQoNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29kIFNjb3R0LUIw
NzQyMQ0KPiBTZW50OiBGcmlkYXksIEp1bHkgMTAsIDIwMTUgNjoyMCBBTQ0KPiBUbzogWmhhbyBR
aWFuZy1CNDU0NzUNCj4gQ2M6IGxhdXJhYUBjb2RlYXVyb3JhLm9yZzsgbGludXgta2VybmVsQHZn
ZXIua2VybmVsLm9yZzsgbGludXhwcGMtDQo+IGRldkBsaXN0cy5vemxhYnMub3JnOyBha3BtQGxp
bnV4LWZvdW5kYXRpb24ub3JnOyBvbG9mQGxpeG9tLm5ldDsNCj4gY2F0YWxpbi5tYXJpbmFzQGFy
bS5jb207IFhpZSBYaWFvYm8tUjYzMDYxDQo+IFN1YmplY3Q6IFJlOiBbUkZDXSBnZW5hbGxvYzph
ZGQgYW4gZ2VuX3Bvb2xfYWxsb2NfYWxpZ24gZnVuYyB0byBnZW5hbGxvYw0KPiANCj4gT24gVGh1
LCAyMDE1LTA3LTA5IGF0IDE1OjQ3ICswODAwLCBaaGFvIFFpYW5nIHdyb3RlOg0KPiA+IEBAIC01
NDEsMTMgKzU2MiwxNCBAQCBFWFBPUlRfU1lNQk9MKGdlbl9wb29sX2ZpcnN0X2ZpdF9vcmRlcl9h
bGlnbik7DQo+ID4gICAqIHdoaWNoIHdlIGNhbiBhbGxvY2F0ZSB0aGUgbWVtb3J5Lg0KPiA+ICAg
Ki8NCj4gPiAgdW5zaWduZWQgbG9uZyBnZW5fcG9vbF9iZXN0X2ZpdCh1bnNpZ25lZCBsb25nICpt
YXAsIHVuc2lnbmVkIGxvbmcgc2l6ZSwNCj4gPiAtICAgICAgICAgICAgIHVuc2lnbmVkIGxvbmcg
c3RhcnQsIHVuc2lnbmVkIGludCBuciwgdm9pZCAqZGF0YSkNCj4gPiArICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICB1bnNpZ25lZCBsb25nIHN0YXJ0LCB1bnNpZ25lZCBpbnQgbnIsDQo+ID4g
KyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdm9pZCAqZGF0YSwgdW5zaWduZWQgbG9uZyBh
bGlnbl9tYXNrKQ0KPiA+ICB7DQo+ID4gICAgICAgdW5zaWduZWQgbG9uZyBzdGFydF9iaXQgPSBz
aXplOw0KPiA+ICAgICAgIHVuc2lnbmVkIGxvbmcgbGVuID0gc2l6ZSArIDE7DQo+ID4gICAgICAg
dW5zaWduZWQgbG9uZyBpbmRleDsNCj4gPg0KPiA+IC0gICAgIGluZGV4ID0gYml0bWFwX2ZpbmRf
bmV4dF96ZXJvX2FyZWEobWFwLCBzaXplLCBzdGFydCwgbnIsIDApOw0KPiA+ICsgICAgIGluZGV4
ID0gYml0bWFwX2ZpbmRfbmV4dF96ZXJvX2FyZWEobWFwLCBzaXplLCBzdGFydCwgbnIsDQo+IGFs
aWduX21hc2spOw0KPiA+DQo+ID4gICAgICAgd2hpbGUgKGluZGV4IDwgc2l6ZSkgew0KPiA+ICAg
ICAgICAgICAgICAgaW50IG5leHRfYml0ID0gZmluZF9uZXh0X2JpdChtYXAsIHNpemUsIGluZGV4
ICsgbnIpOw0KPiANCj4gV2hhdCBhYm91dCB0aGUgb3RoZXIgY2FsbCB0byBiaXRtYXBfZmluZF9u
ZXh0X3plcm9fYXJlYSgpPw0KDQpBbGwgb3RoZXJzIHdpbGwgcGFzcyB0aGUgYWxpZ25fbWFzayB0
byBiaXRtYXBfZmluZF9uZXh0X3plcm9fYXJlYS4NCg0KPiANCj4gLVNjb3R0DQoNCg==

RE: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Zhao Qiang]

> -----Original Message-----
> From: Laura Abbott [mailto:labbott@redhat.com]
> Sent: Friday, July 10, 2015 5:51 AM
> To: Zhao Qiang-B45475; lauraa@codeaurora.org
> Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> akpm@linux-foundation.org; olof@lixom.net; catalin.marinas@arm.com; Wood
> Scott-B07421; Xie Xiaobo-R63061
> Subject: Re: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc
>=20
> On 07/09/2015 12:47 AM, Zhao Qiang wrote:
> > Bytes alignment is required to manage some special ram, so add
> > gen_pool_alloc_align func to genalloc.
> > rename gen_pool_alloc to gen_pool_alloc_align with a align parameter,
> > then provide gen_pool_alloc to call gen_pool_alloc_align with align =3D
> > 1 Byte.
> >
> > Signed-off-by: Zhao Qiang <B45475@freescale.com>
> > ---
> > FSL's IP block QE require this function to manage muram.
> > QE supported only PowerPC, and its code was put under arch/powerpc
> > directory, using arch/powerpc/lib/rheap.c to manage muram.
> > Now it support both arm(ls1021,ls1043,ls2085 and such on) and powerpc,
> > the code need to move from arch/powerpc to public direcory, Scott wood
> > hopes to use genalloc to manage the muram, after discussing with
> > scott, we decide to add gen_pool_alloc_align to meet the requirement
> > for bytes-alignment.
>=20
> gen_pool supports custom allocation algorithms. I thought this was
> discussed previously and the conclusion was that if you wanted alignment
> you should use custom allocation algorithms. I'm failing at finding any
> thread discussing it though.
>=20
> Perhaps another option would be to add another runtime argument to
> gen_pool where you could pass the alignment to your custom allocation
> function. This way alignment isn't inherently coded into any of the
> algorithms.
>=20
> >
> >   include/linux/genalloc.h | 10 +++++++---
> >   lib/genalloc.c           | 38 ++++++++++++++++++++++++++++++--------
> >   2 files changed, 37 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h index
> > 1ccaab4..65fdf14 100644
> > --- a/include/linux/genalloc.h
> > +++ b/include/linux/genalloc.h
> > @@ -96,6 +96,8 @@ 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_align(struct gen_pool *, size_t,
> > +		unsigned long align);
> >   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 +110,16 @@ 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,
> > +		unsigned long align_mask);
> >
> >   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, unsigned long align_mask);
> >
> >   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,
> > +		unsigned long align_mask);
> >
> >   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..dd63448
> > 100644
> > --- a/lib/genalloc.c
> > +++ b/lib/genalloc.c
> > @@ -258,19 +258,22 @@ void gen_pool_destroy(struct gen_pool *pool)
> >   EXPORT_SYMBOL(gen_pool_destroy);
> >
> >   /**
> > - * gen_pool_alloc - allocate special memory from the pool
> > + * gen_pool_alloc_align - allocate special memory from the pool
> >    * @pool: pool to allocate from
> >    * @size: number of bytes to allocate from the pool
> > + * @align: number of bytes to align
> >    *
> >    * 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(struct gen_pool *pool, size_t size)
> > +unsigned long gen_pool_alloc_align(struct gen_pool *pool, size_t size,
> > +		unsigned long align)
> >   {
> >   	struct gen_pool_chunk *chunk;
> >   	unsigned long addr =3D 0;
> > +	unsigned long align_mask;
> >   	int order =3D pool->min_alloc_order;
> >   	int nbits, start_bit =3D 0, end_bit, remain;
> >
> > @@ -281,6 +284,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool,
> size_t size)
> >   	if (size =3D=3D 0)
> >   		return 0;
> >
> > +	align_mask =3D ((align + (1UL << order) - 1) >> order) - 1;
> >   	nbits =3D (size + (1UL << order) - 1) >> order;
> >   	rcu_read_lock();
> >   	list_for_each_entry_rcu(chunk, &pool->chunks, next_chunk) { @@
> > -290,7 +294,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool,
> size_t size)
> >   		end_bit =3D chunk_size(chunk) >> order;
> >   retry:
> >   		start_bit =3D pool->algo(chunk->bits, end_bit, start_bit, nbits,
> > -				pool->data);
> > +				pool->data, align_mask);
> >   		if (start_bit >=3D end_bit)
> >   			continue;
> >   		remain =3D bitmap_set_ll(chunk->bits, start_bit, nbits); @@ -
> 309,6
> > +313,22 @@ retry:
> >   	rcu_read_unlock();
> >   	return addr;
> >   }
> > +EXPORT_SYMBOL(gen_pool_alloc_align);
> > +
> > +/**
> > + * gen_pool_alloc - allocate special memory from the pool
> > + * @pool: pool to allocate from
> > + * @size: number of bytes to allocate from the pool
> > + *
> > + * 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(struct gen_pool *pool, size_t size) {
> > +	return gen_pool_alloc_align(pool, size, 1);
>=20
> Passing 1 here would change the behavior of the existing algorithms which
> were passing 0 for the align mask

When passing 1 here(align_mask =3D ((align + (1UL << order) - 1) >> order) =
- 1), align_mask will be 0.
It will not change the behavior of the existing algorithms.

>=20
> > +}
> >   EXPORT_SYMBOL(gen_pool_alloc);
> >
> >   /**
> > @@ -502,9 +522,10 @@ EXPORT_SYMBOL(gen_pool_set_algo);
> >    * @data: additional data - unused
> >    */
> >   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,
> > +		unsigned long align_mask)
> >   {
> > -	return bitmap_find_next_zero_area(map, size, start, nr, 0);
> > +	return bitmap_find_next_zero_area(map, size, start, nr, align_mask);
> >   }
> >   EXPORT_SYMBOL(gen_pool_first_fit);
> >
> > @@ -520,7 +541,7 @@ EXPORT_SYMBOL(gen_pool_first_fit);
> >    */
> >   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, unsigned long align_mask)
> >   {
> >   	unsigned long align_mask =3D roundup_pow_of_two(nr) - 1;
> >
> > @@ -541,13 +562,14 @@ EXPORT_SYMBOL(gen_pool_first_fit_order_align);
> >    * 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, unsigned long align_mask)
> >   {
> >   	unsigned long start_bit =3D size;
> >   	unsigned long len =3D size + 1;
> >   	unsigned long index;
> >
> > -	index =3D bitmap_find_next_zero_area(map, size, start, nr, 0);
> > +	index =3D bitmap_find_next_zero_area(map, size, start, nr,
> > +align_mask);
> >
> >   	while (index < size) {
> >   		int next_bit =3D find_next_bit(map, size, index + nr);
> >
>=20
> Thanks,
> Laura

Re: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc

[Laura Abbott]

On 07/12/2015 07:22 PM, Zhao Qiang wrote:
>
>
>> -----Original Message-----
>> From: Laura Abbott [mailto:labbott@redhat.com]
>> Sent: Friday, July 10, 2015 5:51 AM
>> To: Zhao Qiang-B45475; lauraa@codeaurora.org
>> Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
>> akpm@linux-foundation.org; olof@lixom.net; catalin.marinas@arm.com; Wood
>> Scott-B07421; Xie Xiaobo-R63061
>> Subject: Re: [RFC] genalloc:add an gen_pool_alloc_align func to genalloc
>>
>> On 07/09/2015 12:47 AM, Zhao Qiang wrote:
>>> Bytes alignment is required to manage some special ram, so add
>>> gen_pool_alloc_align func to genalloc.
>>> rename gen_pool_alloc to gen_pool_alloc_align with a align parameter,
>>> then provide gen_pool_alloc to call gen_pool_alloc_align with align =
>>> 1 Byte.
>>>
>>> Signed-off-by: Zhao Qiang <B45475@freescale.com>
>>> ---
>>> FSL's IP block QE require this function to manage muram.
>>> QE supported only PowerPC, and its code was put under arch/powerpc
>>> directory, using arch/powerpc/lib/rheap.c to manage muram.
>>> Now it support both arm(ls1021,ls1043,ls2085 and such on) and powerpc,
>>> the code need to move from arch/powerpc to public direcory, Scott wood
>>> hopes to use genalloc to manage the muram, after discussing with
>>> scott, we decide to add gen_pool_alloc_align to meet the requirement
>>> for bytes-alignment.
>>
>> gen_pool supports custom allocation algorithms. I thought this was
>> discussed previously and the conclusion was that if you wanted alignment
>> you should use custom allocation algorithms. I'm failing at finding any
>> thread discussing it though.
>>
>> Perhaps another option would be to add another runtime argument to
>> gen_pool where you could pass the alignment to your custom allocation
>> function. This way alignment isn't inherently coded into any of the
>> algorithms.
>>
>>>
>>>    include/linux/genalloc.h | 10 +++++++---
>>>    lib/genalloc.c           | 38 ++++++++++++++++++++++++++++++--------
>>>    2 files changed, 37 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h index
>>> 1ccaab4..65fdf14 100644
>>> --- a/include/linux/genalloc.h
>>> +++ b/include/linux/genalloc.h
>>> @@ -96,6 +96,8 @@ 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_align(struct gen_pool *, size_t,
>>> +		unsigned long align);
>>>    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 +110,16 @@ 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,
>>> +		unsigned long align_mask);
>>>
>>>    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, unsigned long align_mask);
>>>
>>>    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,
>>> +		unsigned long align_mask);
>>>
>>>    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..dd63448
>>> 100644
>>> --- a/lib/genalloc.c
>>> +++ b/lib/genalloc.c
>>> @@ -258,19 +258,22 @@ void gen_pool_destroy(struct gen_pool *pool)
>>>    EXPORT_SYMBOL(gen_pool_destroy);
>>>
>>>    /**
>>> - * gen_pool_alloc - allocate special memory from the pool
>>> + * gen_pool_alloc_align - allocate special memory from the pool
>>>     * @pool: pool to allocate from
>>>     * @size: number of bytes to allocate from the pool
>>> + * @align: number of bytes to align
>>>     *
>>>     * 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(struct gen_pool *pool, size_t size)
>>> +unsigned long gen_pool_alloc_align(struct gen_pool *pool, size_t size,
>>> +		unsigned long align)
>>>    {
>>>    	struct gen_pool_chunk *chunk;
>>>    	unsigned long addr = 0;
>>> +	unsigned long align_mask;
>>>    	int order = pool->min_alloc_order;
>>>    	int nbits, start_bit = 0, end_bit, remain;
>>>
>>> @@ -281,6 +284,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool,
>> size_t size)
>>>    	if (size == 0)
>>>    		return 0;
>>>
>>> +	align_mask = ((align + (1UL << order) - 1) >> order) - 1;
>>>    	nbits = (size + (1UL << order) - 1) >> order;
>>>    	rcu_read_lock();
>>>    	list_for_each_entry_rcu(chunk, &pool->chunks, next_chunk) { @@
>>> -290,7 +294,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, align_mask);
>>>    		if (start_bit >= end_bit)
>>>    			continue;
>>>    		remain = bitmap_set_ll(chunk->bits, start_bit, nbits); @@ -
>> 309,6
>>> +313,22 @@ retry:
>>>    	rcu_read_unlock();
>>>    	return addr;
>>>    }
>>> +EXPORT_SYMBOL(gen_pool_alloc_align);
>>> +
>>> +/**
>>> + * gen_pool_alloc - allocate special memory from the pool
>>> + * @pool: pool to allocate from
>>> + * @size: number of bytes to allocate from the pool
>>> + *
>>> + * 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(struct gen_pool *pool, size_t size) {
>>> +	return gen_pool_alloc_align(pool, size, 1);
>>
>> Passing 1 here would change the behavior of the existing algorithms which
>> were passing 0 for the align mask
>
> When passing 1 here(align_mask = ((align + (1UL << order) - 1) >> order) - 1), align_mask will be 0.
> It will not change the behavior of the existing algorithms.
>

Yes, you are right, I did the math wrong when looking at the align function.

Laura