Re: [Linaro-mm-sig] [PATCH 02/10] lib: genalloc: Generic allocator improvements

2011-06-14 Thread Jordan Crouse

On 06/10/2011 06:52 AM, Alan Cox wrote:

I plan to replace it with lib/bitmap.c bitmap_* based allocator (similar like
it it is used by dma_declare_coherent_memory() and friends in
drivers/base/dma-coherent.c). We need something really simple for CMA area
management.

IMHO allocate_resource and friends a bit too heavy here, but good to know
that such allocator also exists.


Not sure I'd class allocate_resource as heavyweight but providing it's
using something that already exists rather than inventing yet another
allocator.

This wants dealing with before it goes upstream though so the chaneges in
lib/*c etc never have to reach mainline and then get changed back.


Even if CMA doesn't end up using genalloc, there are existing consumers of
the API and I think the _aligned function has value.

I agree that allocate_resource isn't overly heavy, but comparatively genalloc
is really simple and lightweight for a driver to manage a contiguous address
space without a lot of extra thought. I think both APIs serve slightly
different masters, but if somebody thought about it long enough there could
be some consolidation (same goes for the internal guts of
dma_declare_coherent_memory).

I agree with Michal - if genalloc goes deprecated, then so be it, but as
long as it lives, I think its useful to get these functions upstream.

Jordan
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/10] lib: genalloc: Generic allocator improvements

2011-06-10 Thread Michal Nazarewicz
On Fri, 10 Jun 2011 14:52:17 +0200, Alan Cox   
wrote:


I plan to replace it with lib/bitmap.c bitmap_* based allocator  
(similar like

it it is used by dma_declare_coherent_memory() and friends in
drivers/base/dma-coherent.c). We need something really simple for CMA  
area

management.

IMHO allocate_resource and friends a bit too heavy here, but good to  
know

that such allocator also exists.


Not sure I'd class allocate_resource as heavyweight but providing it's
using something that already exists rather than inventing yet another
allocator.


genalloc is already in the kernel and is used in a few places, so we
either let everyone use it as they see fit or we deprecate the library.
If we don't deprecate it I see no reason why CMA should not use it.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/10] lib: genalloc: Generic allocator improvements

2011-06-10 Thread Alan Cox
> I plan to replace it with lib/bitmap.c bitmap_* based allocator (similar like
> it it is used by dma_declare_coherent_memory() and friends in
> drivers/base/dma-coherent.c). We need something really simple for CMA area
> management. 
> 
> IMHO allocate_resource and friends a bit too heavy here, but good to know 
> that such allocator also exists.

Not sure I'd class allocate_resource as heavyweight but providing it's
using something that already exists rather than inventing yet another
allocator.

This wants dealing with before it goes upstream though so the chaneges in
lib/*c etc never have to reach mainline and then get changed back.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 02/10] lib: genalloc: Generic allocator improvements

2011-06-10 Thread Marek Szyprowski
Hello,

On Friday, June 10, 2011 1:25 PM Alan Cox wrote:

> I am curious about one thing
> 
> Why do we need this allocator. Why not use allocate_resource and friends.
> The kernel generic resource handler already handles object alignment and
> subranges. It just seems to be a surplus allocator that could perhaps be
> mostly removed by using the kernel resource allocator we already have ?

genalloc was used mainly for historical reasons (in the earlier version we
were looking for direct replacement for first fit allocator).

I plan to replace it with lib/bitmap.c bitmap_* based allocator (similar like
it it is used by dma_declare_coherent_memory() and friends in
drivers/base/dma-coherent.c). We need something really simple for CMA area
management. 

IMHO allocate_resource and friends a bit too heavy here, but good to know 
that such allocator also exists.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/10] lib: genalloc: Generic allocator improvements

2011-06-10 Thread Alan Cox
I am curious about one thing

Why do we need this allocator. Why not use allocate_resource and friends.
The kernel generic resource handler already handles object alignment and
subranges. It just seems to be a surplus allocator that could perhaps be
mostly removed by using the kernel resource allocator we already have ?

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/10] lib: genalloc: Generic allocator improvements

2011-06-10 Thread Marek Szyprowski
From: Michal Nazarewicz 

This commit adds a gen_pool_alloc_aligned() function to the
generic allocator API.  It allows specifying alignment for the
allocated block.  This feature uses
the bitmap_find_next_zero_area_off() function.

It also fixes possible issue with bitmap's last element being
not fully allocated (ie. space allocated for chunk->bits is
not a multiple of sizeof(long)).

It also makes some other smaller changes:
- moves structure definitions out of the header file,
- adds __must_check to functions returning value,
- makes gen_pool_add() return -ENOMEM rater than -1 on error,
- changes list_for_each to list_for_each_entry, and
- makes use of bitmap_clear().

Signed-off-by: Michal Nazarewicz 
Signed-off-by: Kyungmin Park 
[m.szyprowski: rebased and updated to Linux v3.0-rc1]
Signed-off-by: Marek Szyprowski 
CC: Michal Nazarewicz 
---
 include/linux/genalloc.h |   50 ++--
 lib/genalloc.c   |  190 +++---
 2 files changed, 138 insertions(+), 102 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 5bbebda..af44e88 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -11,28 +11,11 @@
 
 #ifndef __GENALLOC_H__
 #define __GENALLOC_H__
-/*
- *  General purpose special memory pool descriptor.
- */
-struct gen_pool {
-   rwlock_t lock;
-   struct list_head chunks;/* list of chunks in this pool */
-   int min_alloc_order;/* minimum allocation order */
-};
 
-/*
- *  General purpose special memory pool chunk descriptor.
- */
-struct gen_pool_chunk {
-   spinlock_t lock;
-   struct list_head next_chunk;/* next chunk in pool */
-   phys_addr_t phys_addr;  /* physical starting address of memory 
chunk */
-   unsigned long start_addr;   /* starting address of memory chunk */
-   unsigned long end_addr; /* ending address of memory chunk */
-   unsigned long bits[0];  /* bitmap for allocating memory chunk */
-};
-
-extern struct gen_pool *gen_pool_create(int, int);
+struct gen_pool;
+
+struct gen_pool *__must_check gen_pool_create(unsigned order, int nid);
+
 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,
 size_t, int);
@@ -53,7 +36,26 @@ static inline int gen_pool_add(struct gen_pool *pool, 
unsigned long addr,
 {
return gen_pool_add_virt(pool, addr, -1, size, nid);
 }
-extern void gen_pool_destroy(struct gen_pool *);
-extern unsigned long gen_pool_alloc(struct gen_pool *, size_t);
-extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
+
+void gen_pool_destroy(struct gen_pool *pool);
+
+unsigned long __must_check
+gen_pool_alloc_aligned(struct gen_pool *pool, size_t size,
+  unsigned alignment_order);
+
+/**
+ * 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 a first-fit algorithm.
+ */
+static inline unsigned long __must_check
+gen_pool_alloc(struct gen_pool *pool, size_t size)
+{
+   return gen_pool_alloc_aligned(pool, size, 0);
+}
+
+void gen_pool_free(struct gen_pool *pool, unsigned long addr, size_t size);
 #endif /* __GENALLOC_H__ */
diff --git a/lib/genalloc.c b/lib/genalloc.c
index 577ddf8..b41dd90 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -16,23 +16,46 @@
 #include 
 
 
+/* General purpose special memory pool descriptor. */
+struct gen_pool {
+   rwlock_t lock;  /* protects chunks list */
+   struct list_head chunks;/* list of chunks in this pool */
+   unsigned order; /* minimum allocation order */
+};
+
+/* General purpose special memory pool chunk descriptor. */
+struct gen_pool_chunk {
+   spinlock_t lock;/* protects bits */
+   struct list_head next_chunk;/* next chunk in pool */
+   phys_addr_t phys_addr;  /* physical starting address of memory 
chunk */
+   unsigned long start;/* start of memory chunk */
+   unsigned long size; /* number of bits */
+   unsigned long bits[0];  /* bitmap for allocating memory chunk */
+};
+
+
 /**
- * gen_pool_create - create a new special memory pool
- * @min_alloc_order: log base 2 of number of bytes each bitmap bit represents
- * @nid: node id of the node the pool structure should be allocated on, or -1
+ * gen_pool_create() - create a new special memory pool
+ * @order: Log base 2 of number of bytes each bitmap bit
+ * represents.
+ * @nid:   Node id of the node the pool structure should be allocated
+ * on, or -1.  This will be also used for other allocations.
  *
  * Create a new special memory pool that can be used to manag