[PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Steve Wise]

Modules using the genpool allocator need to be able to destroy the data
structure when unloading.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---

 include/linux/genalloc.h |    1 +
 lib/genalloc.c           |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 690c428..ca2b119 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -30,6 +30,7 @@ struct gen_pool_chunk {
 };
 
 extern struct gen_pool *gen_pool_create(int, int);
+extern void gen_pool_destroy(struct gen_pool *);
 extern int gen_pool_add(struct gen_pool *, unsigned long, size_t, int);
 extern unsigned long gen_pool_alloc(struct gen_pool *, size_t);
 extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
diff --git a/lib/genalloc.c b/lib/genalloc.c
index 71338b4..c8afa10 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -36,6 +36,28 @@ EXPORT_SYMBOL(gen_pool_create);
 
 
 /*
+ * Destroy a memory pool.  Assumes the user deals with releasing the
+ * actual memory managed by the pool.  
+ *
+ * @pool: pool to destroy.
+ *
+ */
+void gen_pool_destroy(struct gen_pool *pool)
+{
+	struct list_head *_chunk, *next;
+	struct gen_pool_chunk *chunk;
+
+	list_for_each_safe(_chunk, next, &pool->chunks) {
+		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
+		kfree(chunk);
+	}
+	kfree(pool);
+	return;
+}
+EXPORT_SYMBOL(gen_pool_destroy);
+
+
+/*
  * Add a new chunk of memory to the specified pool.
  *
  * @pool: pool to add new memory chunk to

Re: [PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Randy Dunlap]

On Wed, 27 Sep 2006 10:35:45 -0500 Steve Wise wrote:

> Modules using the genpool allocator need to be able to destroy the data
> structure when unloading.
> 
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
> 
>  include/linux/genalloc.h |    1 +
>  lib/genalloc.c           |   22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
> index 690c428..ca2b119 100644
> --- a/include/linux/genalloc.h
> +++ b/include/linux/genalloc.h
> @@ -30,6 +30,7 @@ struct gen_pool_chunk {
>  };
>  
>  extern struct gen_pool *gen_pool_create(int, int);
> +extern void gen_pool_destroy(struct gen_pool *);
>  extern int gen_pool_add(struct gen_pool *, unsigned long, size_t, int);
>  extern unsigned long gen_pool_alloc(struct gen_pool *, size_t);
>  extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
> diff --git a/lib/genalloc.c b/lib/genalloc.c
> index 71338b4..c8afa10 100644
> --- a/lib/genalloc.c
> +++ b/lib/genalloc.c
> @@ -36,6 +36,28 @@ EXPORT_SYMBOL(gen_pool_create);
>  
>  
>  /*
> + * Destroy a memory pool.  Assumes the user deals with releasing the
> + * actual memory managed by the pool.  
> + *
> + * @pool: pool to destroy.
> + *
> + */

Please use kernel-doc for exported kernel interfaces.
See Documentation/kernel-doc-nano-HOWTO.txt for info,
and/or see some file like kernel/printk.c for examples.

> +void gen_pool_destroy(struct gen_pool *pool)
> +{
> +	struct list_head *_chunk, *next;
> +	struct gen_pool_chunk *chunk;
> +
> +	list_for_each_safe(_chunk, next, &pool->chunks) {
> +		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
> +		kfree(chunk);
> +	}
> +	kfree(pool);
> +	return;
> +}
> +EXPORT_SYMBOL(gen_pool_destroy);
> +
> +
> +/*
>   * Add a new chunk of memory to the specified pool.
>   *
>   * @pool: pool to add new memory chunk to
> -

argh.  more.  (not part of your patch)

---
~Randy

Re: [PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Steve Wise]

> > index 71338b4..c8afa10 100644
> > --- a/lib/genalloc.c
> > +++ b/lib/genalloc.c
> > @@ -36,6 +36,28 @@ EXPORT_SYMBOL(gen_pool_create);
> >  
> >  
> >  /*
> > + * Destroy a memory pool.  Assumes the user deals with releasing the
> > + * actual memory managed by the pool.  
> > + *
> > + * @pool: pool to destroy.
> > + *
> > + */
> 
> Please use kernel-doc for exported kernel interfaces.
> See Documentation/kernel-doc-nano-HOWTO.txt for info,
> and/or see some file like kernel/printk.c for examples.
> 
> > +void gen_pool_destroy(struct gen_pool *pool)
> > +{
> > +	struct list_head *_chunk, *next;
> > +	struct gen_pool_chunk *chunk;
> > +
> > +	list_for_each_safe(_chunk, next, &pool->chunks) {
> > +		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
> > +		kfree(chunk);
> > +	}
> > +	kfree(pool);
> > +	return;
> > +}
> > +EXPORT_SYMBOL(gen_pool_destroy);
> > +
> > +
> > +/*
> >   * Add a new chunk of memory to the specified pool.
> >   *
> >   * @pool: pool to add new memory chunk to
> > -
> 
> argh.  more.  (not part of your patch)


I formatted it to align with the rest of the file...

Steve.

Re: [PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Randy Dunlap]

On Wed, 27 Sep 2006 10:53:25 -0500 Steve Wise wrote:

> > > index 71338b4..c8afa10 100644
> > > --- a/lib/genalloc.c
> > > +++ b/lib/genalloc.c
> > > @@ -36,6 +36,28 @@ EXPORT_SYMBOL(gen_pool_create);
> > >  
> > >  
> > >  /*
> > > + * Destroy a memory pool.  Assumes the user deals with releasing the
> > > + * actual memory managed by the pool.  
> > > + *
> > > + * @pool: pool to destroy.
> > > + *
> > > + */
> > 
> > Please use kernel-doc for exported kernel interfaces.
> > See Documentation/kernel-doc-nano-HOWTO.txt for info,
> > and/or see some file like kernel/printk.c for examples.
> > 
> > > +void gen_pool_destroy(struct gen_pool *pool)
> > > +{
> > > +	struct list_head *_chunk, *next;
> > > +	struct gen_pool_chunk *chunk;
> > > +
> > > +	list_for_each_safe(_chunk, next, &pool->chunks) {
> > > +		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
> > > +		kfree(chunk);
> > > +	}
> > > +	kfree(pool);
> > > +	return;
> > > +}
> > > +EXPORT_SYMBOL(gen_pool_destroy);
> > > +
> > > +
> > > +/*
> > >   * Add a new chunk of memory to the specified pool.
> > >   *
> > >   * @pool: pool to add new memory chunk to
> > > -
> > 
> > argh.  more.  (not part of your patch)
> 
> 
> I formatted it to align with the rest of the file...

Yes, so I noticed (when I got to the end of the patch).

I suppose I'll queue this up for kernel-doc work.

---
~Randy

Re: [PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Dean Nelson]

Modules using the genpool allocator need to be able to destroy the data
structure when unloading.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
Signed-off-by: Dean Nelson <dcn@sgi.com>

---

Hi Steve,

I agree that the ability to destroy the allocated structures is
necessary. Thanks for doing the work. I do think it appropriate
to ensure that there are no outstanding allocations (to avoid use
after free issues) and have added that check in this new patch.
This patch has not been tested, though it does compile. I don't
have the time today. I hope you don't mind testing it? :-)

It also looks like I need to straighten out the kernel-doc
aspects of this file. I'll tackle that as a separate patch.

Thanks,
Dean


 include/linux/genalloc.h |    1 +
 lib/genalloc.c           |   29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)


Index: linux-2.6/lib/genalloc.c
===================================================================
--- linux-2.6.orig/lib/genalloc.c	2006-09-27 13:42:35.000000000 -0500
+++ linux-2.6/lib/genalloc.c	2006-09-27 14:31:56.816523882 -0500
@@ -71,6 +71,35 @@
 
 
 /*
+ * Destroy a memory pool.
+ *
+ * @pool: pool to destroy
+ */
+void gen_pool_destroy(struct gen_pool *pool)
+{
+	struct list_head *_chunk, *_next_chunk;
+	struct gen_pool_chunk *chunk;
+	int order = pool->min_alloc_order;
+	int bit, end_bit;
+
+
+	write_lock(&pool->lock);
+	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
+		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
+
+		end_bit = (chunk->end_addr - chunk->start_addr) >> order;
+		bit = find_next_bit(chunk->bits, end_bit, 0);
+		BUG_ON(bit < end_bit);
+
+		kfree(chunk);
+	}
+	kfree(pool);
+	return;
+}
+EXPORT_SYMBOL(gen_pool_destroy);
+
+
+/*
  * Allocate the requested number of bytes from the specified pool.
  * Uses a first-fit algorithm.
  *
Index: linux-2.6/include/linux/genalloc.h
===================================================================
--- linux-2.6.orig/include/linux/genalloc.h	2006-09-27 13:42:34.000000000 -0500
+++ linux-2.6/include/linux/genalloc.h	2006-09-27 14:18:31.807816652 -0500
@@ -31,5 +31,6 @@
 
 extern struct gen_pool *gen_pool_create(int, int);
 extern int gen_pool_add(struct gen_pool *, unsigned long, size_t, int);
+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);

Re: [PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Dean Nelson]

On Wed, Sep 27, 2006 at 08:56:08AM -0700, Randy Dunlap wrote:
> On Wed, 27 Sep 2006 10:53:25 -0500 Steve Wise wrote:
> 
> > > > index 71338b4..c8afa10 100644
> > > > --- a/lib/genalloc.c
> > > > +++ b/lib/genalloc.c
> > > > @@ -36,6 +36,28 @@ EXPORT_SYMBOL(gen_pool_create);
> > > >  
> > > >  
> > > >  /*
> > > > + * Destroy a memory pool.  Assumes the user deals with releasing the
> > > > + * actual memory managed by the pool.  
> > > > + *
> > > > + * @pool: pool to destroy.
> > > > + *
> > > > + */
> > > 
> > > Please use kernel-doc for exported kernel interfaces.
> > > See Documentation/kernel-doc-nano-HOWTO.txt for info,
> > > and/or see some file like kernel/printk.c for examples.
> > > 
> > > > +void gen_pool_destroy(struct gen_pool *pool)
> > > > +{
> > > > +	struct list_head *_chunk, *next;
> > > > +	struct gen_pool_chunk *chunk;
> > > > +
> > > > +	list_for_each_safe(_chunk, next, &pool->chunks) {
> > > > +		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
> > > > +		kfree(chunk);
> > > > +	}
> > > > +	kfree(pool);
> > > > +	return;
> > > > +}
> > > > +EXPORT_SYMBOL(gen_pool_destroy);
> > > > +
> > > > +
> > > > +/*
> > > >   * Add a new chunk of memory to the specified pool.
> > > >   *
> > > >   * @pool: pool to add new memory chunk to
> > > > -
> > > 
> > > argh.  more.  (not part of your patch)
> > 
> > 
> > I formatted it to align with the rest of the file...
> 
> Yes, so I noticed (when I got to the end of the patch).
> 
> I suppose I'll queue this up for kernel-doc work.

Sorry Randy, this was my mistake. I didn't know about kernel-doc.
I'll put together a patch tommorrow, if that would be alright?
I don't have the time today.

Thanks,
Dean

Re: [PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Randy Dunlap]

On Wed, 27 Sep 2006 14:59:29 -0500 Dean Nelson wrote:

> On Wed, Sep 27, 2006 at 08:56:08AM -0700, Randy Dunlap wrote:
> > On Wed, 27 Sep 2006 10:53:25 -0500 Steve Wise wrote:
> > 
> > > > > index 71338b4..c8afa10 100644
> > > > > --- a/lib/genalloc.c
> > > > > +++ b/lib/genalloc.c
> > > > > @@ -36,6 +36,28 @@ EXPORT_SYMBOL(gen_pool_create);
> > > > >  
> > > > >  
> > > > >  /*
> > > > > + * Destroy a memory pool.  Assumes the user deals with releasing the
> > > > > + * actual memory managed by the pool.  
> > > > > + *
> > > > > + * @pool: pool to destroy.
> > > > > + *
> > > > > + */
> > > > 
> > > > Please use kernel-doc for exported kernel interfaces.
> > > > See Documentation/kernel-doc-nano-HOWTO.txt for info,
> > > > and/or see some file like kernel/printk.c for examples.
> > > > 
> > > > > +void gen_pool_destroy(struct gen_pool *pool)
> > > > > +{
> > > > > +	struct list_head *_chunk, *next;
> > > > > +	struct gen_pool_chunk *chunk;
> > > > > +
> > > > > +	list_for_each_safe(_chunk, next, &pool->chunks) {
> > > > > +		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
> > > > > +		kfree(chunk);
> > > > > +	}
> > > > > +	kfree(pool);
> > > > > +	return;
> > > > > +}
> > > > > +EXPORT_SYMBOL(gen_pool_destroy);
> > > > > +
> > > > > +
> > > > > +/*
> > > > >   * Add a new chunk of memory to the specified pool.
> > > > >   *
> > > > >   * @pool: pool to add new memory chunk to
> > > > > -
> > > > 
> > > > argh.  more.  (not part of your patch)
> > > 
> > > 
> > > I formatted it to align with the rest of the file...
> > 
> > Yes, so I noticed (when I got to the end of the patch).
> > 
> > I suppose I'll queue this up for kernel-doc work.
> 
> Sorry Randy, this was my mistake. I didn't know about kernel-doc.
> I'll put together a patch tommorrow, if that would be alright?
> I don't have the time today.

Sure, no problem.  Thanks for doing that.
I'll review it when you post it.

---
~Randy

Re: [PATCH 2.6.18 ] LIB Add gen_pool_destroy().

[Steve Wise]

Dean,

Looks like it works fine.

Any chance of getting this into 2.6.18 I wonder?


Steve.



On Wed, 2006-09-27 at 14:51 -0500, Dean Nelson wrote:
> Modules using the genpool allocator need to be able to destroy the data
> structure when unloading.
> 
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> Signed-off-by: Dean Nelson <dcn@sgi.com>
> 
> ---
> 
> Hi Steve,
> 
> I agree that the ability to destroy the allocated structures is
> necessary. Thanks for doing the work. I do think it appropriate
> to ensure that there are no outstanding allocations (to avoid use
> after free issues) and have added that check in this new patch.
> This patch has not been tested, though it does compile. I don't
> have the time today. I hope you don't mind testing it? :-)
> 
> It also looks like I need to straighten out the kernel-doc
> aspects of this file. I'll tackle that as a separate patch.
> 
> Thanks,
> Dean
> 
> 
>  include/linux/genalloc.h |    1 +
>  lib/genalloc.c           |   29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> 
> Index: linux-2.6/lib/genalloc.c
> ===================================================================
> --- linux-2.6.orig/lib/genalloc.c	2006-09-27 13:42:35.000000000 -0500
> +++ linux-2.6/lib/genalloc.c	2006-09-27 14:31:56.816523882 -0500
> @@ -71,6 +71,35 @@
>  
> 
>  /*
> + * Destroy a memory pool.
> + *
> + * @pool: pool to destroy
> + */
> +void gen_pool_destroy(struct gen_pool *pool)
> +{
> +	struct list_head *_chunk, *_next_chunk;
> +	struct gen_pool_chunk *chunk;
> +	int order = pool->min_alloc_order;
> +	int bit, end_bit;
> +
> +
> +	write_lock(&pool->lock);
> +	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
> +		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
> +
> +		end_bit = (chunk->end_addr - chunk->start_addr) >> order;
> +		bit = find_next_bit(chunk->bits, end_bit, 0);
> +		BUG_ON(bit < end_bit);
> +
> +		kfree(chunk);
> +	}
> +	kfree(pool);
> +	return;
> +}
> +EXPORT_SYMBOL(gen_pool_destroy);
> +
> +
> +/*
>   * Allocate the requested number of bytes from the specified pool.
>   * Uses a first-fit algorithm.
>   *
> Index: linux-2.6/include/linux/genalloc.h
> ===================================================================
> --- linux-2.6.orig/include/linux/genalloc.h	2006-09-27 13:42:34.000000000 -0500
> +++ linux-2.6/include/linux/genalloc.h	2006-09-27 14:18:31.807816652 -0500
> @@ -31,5 +31,6 @@
>  
>  extern struct gen_pool *gen_pool_create(int, int);
>  extern int gen_pool_add(struct gen_pool *, unsigned long, size_t, int);
> +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);
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] add gen_pool_destroy()

[Dean Nelson]

Modules using the genpool allocator need to be able to kfree() the memory
used for the genpool data structures when unloading.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
Signed-off-by: Dean Nelson <dcn@sgi.com>

---

Hi Linus,

Would you please accept this patch against the genpool allocator?

There is still an issue with the block comments not being up to
kernel-doc standards that plagues the entire file, which I will
remedy in a followup patch.

Thanks,
Dean

 include/linux/genalloc.h |    1 +
 lib/genalloc.c           |   37 +++++++++++++++++++++++++++++++++----
 2 files changed, 34 insertions(+), 4 deletions(-)


Index: linux-2.6/lib/genalloc.c
===================================================================
--- linux-2.6.orig/lib/genalloc.c	2006-09-27 13:42:35.000000000 -0500
+++ linux-2.6/lib/genalloc.c	2006-09-28 07:49:05.948568928 -0500
@@ -71,6 +71,35 @@
 
 
 /*
+ * Destroy a memory pool. Verifies that there are no outstanding allocations.
+ *
+ * @pool: pool to destroy
+ */
+void gen_pool_destroy(struct gen_pool *pool)
+{
+	struct list_head *_chunk, *_next_chunk;
+	struct gen_pool_chunk *chunk;
+	int order = pool->min_alloc_order;
+	int bit, end_bit;
+
+
+	write_lock(&pool->lock);
+	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
+		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
+
+		end_bit = (chunk->end_addr - chunk->start_addr) >> order;
+		bit = find_next_bit(chunk->bits, end_bit, 0);
+		BUG_ON(bit < end_bit);
+
+		kfree(chunk);
+	}
+	kfree(pool);
+	return;
+}
+EXPORT_SYMBOL(gen_pool_destroy);
+
+
+/*
  * Allocate the requested number of bytes from the specified pool.
  * Uses a first-fit algorithm.
  *
@@ -79,7 +108,7 @@
  */
 unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
 {
-	struct list_head *_chunk;
+	struct list_head *_chunk, *_next_chunk;
 	struct gen_pool_chunk *chunk;
 	unsigned long addr, flags;
 	int order = pool->min_alloc_order;
@@ -91,7 +120,7 @@
 	nbits = (size + (1UL << order) - 1) >> order;
 
 	read_lock(&pool->lock);
-	list_for_each(_chunk, &pool->chunks) {
+	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
 		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
 
 		end_bit = (chunk->end_addr - chunk->start_addr) >> order;
@@ -137,7 +166,7 @@
  */
 void gen_pool_free(struct gen_pool *pool, unsigned long addr, size_t size)
 {
-	struct list_head *_chunk;
+	struct list_head *_chunk, *_next_chunk;
 	struct gen_pool_chunk *chunk;
 	unsigned long flags;
 	int order = pool->min_alloc_order;
@@ -146,7 +175,7 @@
 	nbits = (size + (1UL << order) - 1) >> order;
 
 	read_lock(&pool->lock);
-	list_for_each(_chunk, &pool->chunks) {
+	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
 		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
 
 		if (addr >= chunk->start_addr && addr < chunk->end_addr) {
Index: linux-2.6/include/linux/genalloc.h
===================================================================
--- linux-2.6.orig/include/linux/genalloc.h	2006-09-27 13:42:34.000000000 -0500
+++ linux-2.6/include/linux/genalloc.h	2006-09-27 14:18:31.807816652 -0500
@@ -31,5 +31,6 @@
 
 extern struct gen_pool *gen_pool_create(int, int);
 extern int gen_pool_add(struct gen_pool *, unsigned long, size_t, int);
+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);

Re: [PATCH] add gen_pool_destroy()

[Robin Holt]

Nack this.

Dean, you changed the list_for_each to list_for_each_safe where it was
not needed.  I think you only needed to change that in the remove path.
IIRC, list_for_each_safe is intended for walking a list where you unlink
the entry as part of your operation.  Keep it for the gen_pool_destroy
function since you are using kfree on those objects and somebody else
could reuse them (sort of a list delete), but do not change them for
the other case.  The list_for_each does an explicit prefetch which will
improve performance in many cases.

Thanks,
Robin


On Thu, Sep 28, 2006 at 08:16:14AM -0500, Dean Nelson wrote:
> Modules using the genpool allocator need to be able to kfree() the memory
> used for the genpool data structures when unloading.
> 
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> Signed-off-by: Dean Nelson <dcn@sgi.com>
> 
> ---
> 
> Hi Linus,
> 
> Would you please accept this patch against the genpool allocator?
> 
> There is still an issue with the block comments not being up to
> kernel-doc standards that plagues the entire file, which I will
> remedy in a followup patch.
> 
> Thanks,
> Dean
> 
>  include/linux/genalloc.h |    1 +
>  lib/genalloc.c           |   37 +++++++++++++++++++++++++++++++++----
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> 
> Index: linux-2.6/lib/genalloc.c
> ===================================================================
> --- linux-2.6.orig/lib/genalloc.c	2006-09-27 13:42:35.000000000 -0500
> +++ linux-2.6/lib/genalloc.c	2006-09-28 07:49:05.948568928 -0500
> @@ -71,6 +71,35 @@
>  
>  
>  /*
> + * Destroy a memory pool. Verifies that there are no outstanding allocations.
> + *
> + * @pool: pool to destroy
> + */
> +void gen_pool_destroy(struct gen_pool *pool)
> +{
> +	struct list_head *_chunk, *_next_chunk;
> +	struct gen_pool_chunk *chunk;
> +	int order = pool->min_alloc_order;
> +	int bit, end_bit;
> +
> +
> +	write_lock(&pool->lock);
> +	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
> +		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
> +
> +		end_bit = (chunk->end_addr - chunk->start_addr) >> order;
> +		bit = find_next_bit(chunk->bits, end_bit, 0);
> +		BUG_ON(bit < end_bit);
> +
> +		kfree(chunk);
> +	}
> +	kfree(pool);
> +	return;
> +}
> +EXPORT_SYMBOL(gen_pool_destroy);
> +
> +
> +/*
>   * Allocate the requested number of bytes from the specified pool.
>   * Uses a first-fit algorithm.
>   *
> @@ -79,7 +108,7 @@
>   */
>  unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
>  {
> -	struct list_head *_chunk;
> +	struct list_head *_chunk, *_next_chunk;
>  	struct gen_pool_chunk *chunk;
>  	unsigned long addr, flags;
>  	int order = pool->min_alloc_order;
> @@ -91,7 +120,7 @@
>  	nbits = (size + (1UL << order) - 1) >> order;
>  
>  	read_lock(&pool->lock);
> -	list_for_each(_chunk, &pool->chunks) {
> +	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
>  		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
>  
>  		end_bit = (chunk->end_addr - chunk->start_addr) >> order;
> @@ -137,7 +166,7 @@
>   */
>  void gen_pool_free(struct gen_pool *pool, unsigned long addr, size_t size)
>  {
> -	struct list_head *_chunk;
> +	struct list_head *_chunk, *_next_chunk;
>  	struct gen_pool_chunk *chunk;
>  	unsigned long flags;
>  	int order = pool->min_alloc_order;
> @@ -146,7 +175,7 @@
>  	nbits = (size + (1UL << order) - 1) >> order;
>  
>  	read_lock(&pool->lock);
> -	list_for_each(_chunk, &pool->chunks) {
> +	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
>  		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
>  
>  		if (addr >= chunk->start_addr && addr < chunk->end_addr) {
> Index: linux-2.6/include/linux/genalloc.h
> ===================================================================
> --- linux-2.6.orig/include/linux/genalloc.h	2006-09-27 13:42:34.000000000 -0500
> +++ linux-2.6/include/linux/genalloc.h	2006-09-27 14:18:31.807816652 -0500
> @@ -31,5 +31,6 @@
>  
>  extern struct gen_pool *gen_pool_create(int, int);
>  extern int gen_pool_add(struct gen_pool *, unsigned long, size_t, int);
> +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);
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] add gen_pool_destroy()

[Dean Nelson]

Modules using the genpool allocator need to be able to kfree() the memory
used for the genpool data structures when unloading.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
Signed-off-by: Dean Nelson <dcn@sgi.com>

---

On Thu, Sep 28, 2006 at 09:51:42AM -0500, Robin Holt wrote:
> Nack this.
> 
> Dean, you changed the list_for_each to list_for_each_safe where it was
> not needed.  I think you only needed to change that in the remove path.
> IIRC, list_for_each_safe is intended for walking a list where you unlink
> the entry as part of your operation.  Keep it for the gen_pool_destroy
> function since you are using kfree on those objects and somebody else
> could reuse them (sort of a list delete), but do not change them for
> the other case.  The list_for_each does an explicit prefetch which will
> improve performance in many cases.

Thanks Robin.

Linus, here's a new version of the patch which corrects the
issue that Robin Holt pointed out.

Again, there is an issue with the block comments not being up to
kernel-doc standards that plagues the entire file, which I will
remedy in a followup patch.

Thanks,
Dean


 include/linux/genalloc.h |    1 +
 lib/genalloc.c           |   30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)


Index: linux-2.6/lib/genalloc.c
===================================================================
--- linux-2.6.orig/lib/genalloc.c	2006-09-28 09:53:53.372104302 -0500
+++ linux-2.6/lib/genalloc.c	2006-09-28 10:42:47.627258805 -0500
@@ -71,6 +71,36 @@
 
 
 /*
+ * Destroy a memory pool. Verifies that there are no outstanding allocations.
+ *
+ * @pool: pool to destroy
+ */
+void gen_pool_destroy(struct gen_pool *pool)
+{
+	struct list_head *_chunk, *_next_chunk;
+	struct gen_pool_chunk *chunk;
+	int order = pool->min_alloc_order;
+	int bit, end_bit;
+
+
+	write_lock(&pool->lock);
+	list_for_each_safe(_chunk, _next_chunk, &pool->chunks) {
+		chunk = list_entry(_chunk, struct gen_pool_chunk, next_chunk);
+		list_del(&chunk->next_chunk);
+
+		end_bit = (chunk->end_addr - chunk->start_addr) >> order;
+		bit = find_next_bit(chunk->bits, end_bit, 0);
+		BUG_ON(bit < end_bit);
+
+		kfree(chunk);
+	}
+	kfree(pool);
+	return;
+}
+EXPORT_SYMBOL(gen_pool_destroy);
+
+
+/*
  * Allocate the requested number of bytes from the specified pool.
  * Uses a first-fit algorithm.
  *
Index: linux-2.6/include/linux/genalloc.h
===================================================================
--- linux-2.6.orig/include/linux/genalloc.h	2006-09-28 09:53:53.372104302 -0500
+++ linux-2.6/include/linux/genalloc.h	2006-09-28 09:53:55.200331504 -0500
@@ -31,5 +31,6 @@
 
 extern struct gen_pool *gen_pool_create(int, int);
 extern int gen_pool_add(struct gen_pool *, unsigned long, size_t, int);
+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);

[PATCH] make genpool allocator adhere to kernel-doc standards

[Dean Nelson]

The exported kernel interfaces of genpool allocator need to adhere to
the requirements of kernel-doc.

Signed-off-by: Dean Nelson <dcn@sgi.com>

---

On Wed, Sep 27, 2006 at 01:27:28PM -0700, Randy Dunlap wrote:
> On Wed, 27 Sep 2006 14:59:29 -0500 Dean Nelson wrote:
> > 
> > Sorry Randy, this was my mistake. I didn't know about kernel-doc.
> > I'll put together a patch tommorrow, if that would be alright?
> > I don't have the time today.
> 
> Sure, no problem.  Thanks for doing that.
> I'll review it when you post it.

Thanks for reviewing this. If you do not have any issues with
this patch, would you mind sending it on to Linus?

This patch is dependent on another patch to be applied first, the
patch with a subject line of: '[PATCH] add gen_pool_destroy()'.
(Its most recent version.)

Thanks,
Dean


 genalloc.c |   39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)


Index: linux-2.6/lib/genalloc.c
===================================================================
--- linux-2.6.orig/lib/genalloc.c	2006-09-28 10:54:41.330794389 -0500
+++ linux-2.6/lib/genalloc.c	2006-09-28 11:02:54.327472348 -0500
@@ -14,11 +14,13 @@
 #include <linux/genalloc.h>
 
 
-/*
- * Create a new special memory pool.
- *
+/**
+ * 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
+ *
+ * Create a new special memory pool that can be used to manage special purpose
+ * memory not managed by the regular kmalloc/kfree interface.
  */
 struct gen_pool *gen_pool_create(int min_alloc_order, int nid)
 {
@@ -35,14 +37,15 @@
 EXPORT_SYMBOL(gen_pool_create);
 
 
-/*
- * Add a new chunk of memory to the specified pool.
- *
+/**
+ * gen_pool_add - add a new chunk of special memory to the pool
  * @pool: pool to add new memory chunk to
  * @addr: starting address of memory chunk to add to pool
  * @size: size in bytes of the memory chunk to add to pool
  * @nid: node id of the node the chunk structure and bitmap should be
  *       allocated on, or -1
+ *
+ * Add a new chunk of special memory to the specified pool.
  */
 int gen_pool_add(struct gen_pool *pool, unsigned long addr, size_t size,
 		 int nid)
@@ -70,10 +73,12 @@
 EXPORT_SYMBOL(gen_pool_add);
 
 
-/*
- * Destroy a memory pool. Verifies that there are no outstanding allocations.
- *
+/**
+ * gen_pool_destroy - destroy a special memory pool
  * @pool: pool to destroy
+ *
+ * Destroy the specified special memory pool. Verifies that there are no
+ * outstanding allocations.
  */
 void gen_pool_destroy(struct gen_pool *pool)
 {
@@ -100,12 +105,13 @@
 EXPORT_SYMBOL(gen_pool_destroy);
 
 
-/*
- * Allocate the requested number of bytes from the specified pool.
- * Uses a first-fit algorithm.
- *
+/**
+ * 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.
  */
 unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
 {
@@ -158,12 +164,13 @@
 EXPORT_SYMBOL(gen_pool_alloc);
 
 
-/*
- * Free the specified memory back to the specified pool.
- *
+/**
+ * gen_pool_free - free allocated special memory back to the pool
  * @pool: pool to free to
  * @addr: starting address of memory to free back to pool
  * @size: size in bytes of memory to free
+ *
+ * Free previously allocated special memory back to the specified pool.
  */
 void gen_pool_free(struct gen_pool *pool, unsigned long addr, size_t size)
 {

Re: [PATCH] make genpool allocator adhere to kernel-doc standards

[Randy Dunlap]

On Thu, 28 Sep 2006 12:24:07 -0500 Dean Nelson wrote:

> The exported kernel interfaces of genpool allocator need to adhere to
> the requirements of kernel-doc.
> 
> Signed-off-by: Dean Nelson <dcn@sgi.com>

Acked-by: Randy Dunlap <rdunlap@xenotime.net>

> ---
> Thanks for reviewing this. If you do not have any issues with
> this patch, would you mind sending it on to Linus?
> 
> This patch is dependent on another patch to be applied first, the
> patch with a subject line of: '[PATCH] add gen_pool_destroy()'.
> (Its most recent version.)
> 
> Thanks,
> Dean
> 
> 
>  genalloc.c |   39 +++++++++++++++++++++++----------------
>  1 file changed, 23 insertions(+), 16 deletions(-)
> 
> 
> Index: linux-2.6/lib/genalloc.c
> ===================================================================
> --- linux-2.6.orig/lib/genalloc.c	2006-09-28 10:54:41.330794389 -0500
> +++ linux-2.6/lib/genalloc.c	2006-09-28 11:02:54.327472348 -0500
> @@ -14,11 +14,13 @@
>  #include <linux/genalloc.h>
>  
>  
> -/*
> - * Create a new special memory pool.
> - *
> +/**
> + * 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
> + *
> + * Create a new special memory pool that can be used to manage special purpose
> + * memory not managed by the regular kmalloc/kfree interface.
>   */
>  struct gen_pool *gen_pool_create(int min_alloc_order, int nid)
>  {
> @@ -35,14 +37,15 @@
>  EXPORT_SYMBOL(gen_pool_create);
>  
>  
> -/*
> - * Add a new chunk of memory to the specified pool.
> - *
> +/**
> + * gen_pool_add - add a new chunk of special memory to the pool
>   * @pool: pool to add new memory chunk to
>   * @addr: starting address of memory chunk to add to pool
>   * @size: size in bytes of the memory chunk to add to pool
>   * @nid: node id of the node the chunk structure and bitmap should be
>   *       allocated on, or -1
> + *
> + * Add a new chunk of special memory to the specified pool.
>   */
>  int gen_pool_add(struct gen_pool *pool, unsigned long addr, size_t size,
>  		 int nid)
> @@ -70,10 +73,12 @@
>  EXPORT_SYMBOL(gen_pool_add);
>  
>  
> -/*
> - * Destroy a memory pool. Verifies that there are no outstanding allocations.
> - *
> +/**
> + * gen_pool_destroy - destroy a special memory pool
>   * @pool: pool to destroy
> + *
> + * Destroy the specified special memory pool. Verifies that there are no
> + * outstanding allocations.
>   */
>  void gen_pool_destroy(struct gen_pool *pool)
>  {
> @@ -100,12 +105,13 @@
>  EXPORT_SYMBOL(gen_pool_destroy);
>  
>  
> -/*
> - * Allocate the requested number of bytes from the specified pool.
> - * Uses a first-fit algorithm.
> - *
> +/**
> + * 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.
>   */
>  unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
>  {
> @@ -158,12 +164,13 @@
>  EXPORT_SYMBOL(gen_pool_alloc);
>  
>  
> -/*
> - * Free the specified memory back to the specified pool.
> - *
> +/**
> + * gen_pool_free - free allocated special memory back to the pool
>   * @pool: pool to free to
>   * @addr: starting address of memory to free back to pool
>   * @size: size in bytes of memory to free
> + *
> + * Free previously allocated special memory back to the specified pool.
>   */
>  void gen_pool_free(struct gen_pool *pool, unsigned long addr, size_t size)
>  {
> -

---
~Randy

Re: [PATCH] make genpool allocator adhere to kernel-doc standards

[Dean Nelson]

The exported kernel interfaces of genpool allocator need to adhere to
the requirements of kernel-doc.

Signed-off-by: Dean Nelson <dcn@sgi.com>
Acked-by: Randy Dunlap <rdunlap@xenotime.net>

---

Linus,

This patch is dependent on another patch lib/genalloc.c to be applied first,
the patch with  Subject: Re: [PATCH] add gen_pool_destroy()

Thanks,
Dean


 genalloc.c |   39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)


Index: linux-2.6/lib/genalloc.c
===================================================================
--- linux-2.6.orig/lib/genalloc.c	2006-09-28 10:54:41.330794389 -0500
+++ linux-2.6/lib/genalloc.c	2006-09-28 11:02:54.327472348 -0500
@@ -14,11 +14,13 @@
 #include <linux/genalloc.h>
 
 
-/*
- * Create a new special memory pool.
- *
+/**
+ * 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
+ *
+ * Create a new special memory pool that can be used to manage special purpose
+ * memory not managed by the regular kmalloc/kfree interface.
  */
 struct gen_pool *gen_pool_create(int min_alloc_order, int nid)
 {
@@ -35,14 +37,15 @@
 EXPORT_SYMBOL(gen_pool_create);
 
 
-/*
- * Add a new chunk of memory to the specified pool.
- *
+/**
+ * gen_pool_add - add a new chunk of special memory to the pool
  * @pool: pool to add new memory chunk to
  * @addr: starting address of memory chunk to add to pool
  * @size: size in bytes of the memory chunk to add to pool
  * @nid: node id of the node the chunk structure and bitmap should be
  *       allocated on, or -1
+ *
+ * Add a new chunk of special memory to the specified pool.
  */
 int gen_pool_add(struct gen_pool *pool, unsigned long addr, size_t size,
 		 int nid)
@@ -70,10 +73,12 @@
 EXPORT_SYMBOL(gen_pool_add);
 
 
-/*
- * Destroy a memory pool. Verifies that there are no outstanding allocations.
- *
+/**
+ * gen_pool_destroy - destroy a special memory pool
  * @pool: pool to destroy
+ *
+ * Destroy the specified special memory pool. Verifies that there are no
+ * outstanding allocations.
  */
 void gen_pool_destroy(struct gen_pool *pool)
 {
@@ -100,12 +105,13 @@
 EXPORT_SYMBOL(gen_pool_destroy);
 
 
-/*
- * Allocate the requested number of bytes from the specified pool.
- * Uses a first-fit algorithm.
- *
+/**
+ * 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.
  */
 unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
 {
@@ -158,12 +164,13 @@
 EXPORT_SYMBOL(gen_pool_alloc);
 
 
-/*
- * Free the specified memory back to the specified pool.
- *
+/**
+ * gen_pool_free - free allocated special memory back to the pool
  * @pool: pool to free to
  * @addr: starting address of memory to free back to pool
  * @size: size in bytes of memory to free
+ *
+ * Free previously allocated special memory back to the specified pool.
  */
 void gen_pool_free(struct gen_pool *pool, unsigned long addr, size_t size)
 {