[PATCH] staging: zsmalloc: add user-definable alloc/free funcs

[PATCH] staging: zsmalloc: add user-definable alloc/free funcs

[Seth Jennings]

This patch allows a zsmalloc user to define the page
allocation and free functions to be used when growing
or releasing parts of the memory pool.

The functions are passed in the struct zs_pool_ops parameter
of zs_create_pool() at pool creation time.  If this parameter
is NULL, zsmalloc uses alloc_page and __free_page() by default.

While there is no current user of this functionality, zcache
development plans to make use of it in the near future.

Patch applies to Greg's staging-next branch.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/staging/zcache/zcache-main.c     |    2 +-
 drivers/staging/zram/zram_drv.c          |    3 +-
 drivers/staging/zsmalloc/zsmalloc-main.c |   39 +++++++++++++++++++++++-------
 drivers/staging/zsmalloc/zsmalloc.h      |    8 +++++-
 drivers/staging/zsmalloc/zsmalloc_int.h  |    2 +
 5 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index b698464..7ef5313 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -984,7 +984,7 @@ int zcache_new_client(uint16_t cli_id)
 		goto out;
 	cli->allocated = 1;
 #ifdef CONFIG_FRONTSWAP
-	cli->zspool = zs_create_pool("zcache", ZCACHE_GFP_MASK);
+	cli->zspool = zs_create_pool("zcache", ZCACHE_GFP_MASK, NULL);
 	if (cli->zspool == NULL)
 		goto out;
 #endif
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index 7f13819..278eb4d 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -663,7 +663,8 @@ int zram_init_device(struct zram *zram)
 	/* zram devices sort of resembles non-rotational disks */
 	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, zram->disk->queue);
 
-	zram->mem_pool = zs_create_pool("zram", GFP_NOIO | __GFP_HIGHMEM);
+	zram->mem_pool = zs_create_pool("zram", GFP_NOIO | __GFP_HIGHMEM,
+					NULL);
 	if (!zram->mem_pool) {
 		pr_err("Error creating memory pool\n");
 		ret = -ENOMEM;
diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
index 09caa4f..c8bfb77 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -267,7 +267,7 @@ static unsigned long obj_idx_to_offset(struct page *page,
 	return off + obj_idx * class_size;
 }
 
-static void free_zspage(struct page *first_page)
+static void free_zspage(struct zs_pool *pool, struct page *first_page)
 {
 	struct page *nextp, *tmp;
 
@@ -282,7 +282,7 @@ static void free_zspage(struct page *first_page)
 	first_page->mapping = NULL;
 	first_page->freelist = NULL;
 	reset_page_mapcount(first_page);
-	__free_page(first_page);
+	(*pool->ops->free_page)(first_page);
 
 	/* zspage with only 1 system page */
 	if (!nextp)
@@ -345,7 +345,7 @@ static void init_zspage(struct page *first_page, struct size_class *class)
 /*
  * Allocate a zspage for the given size class
  */
-static struct page *alloc_zspage(struct size_class *class, gfp_t flags)
+static struct page *alloc_zspage(struct zs_pool *pool, struct size_class *class)
 {
 	int i, error;
 	struct page *first_page = NULL;
@@ -365,7 +365,7 @@ static struct page *alloc_zspage(struct size_class *class, gfp_t flags)
 	for (i = 0; i < class->zspage_order; i++) {
 		struct page *page, *prev_page;
 
-		page = alloc_page(flags);
+		page = (*pool->ops->alloc_page)(pool->flags);
 		if (!page)
 			goto cleanup;
 
@@ -398,7 +398,7 @@ static struct page *alloc_zspage(struct size_class *class, gfp_t flags)
 
 cleanup:
 	if (unlikely(error) && first_page) {
-		free_zspage(first_page);
+		free_zspage(pool, first_page);
 		first_page = NULL;
 	}
 
@@ -482,7 +482,24 @@ fail:
 	return notifier_to_errno(ret);
 }
 
-struct zs_pool *zs_create_pool(const char *name, gfp_t flags)
+
+static inline struct page *zs_alloc_page(gfp_t flags)
+{
+	return alloc_page(flags);
+}
+
+static inline void zs_free_page(struct page *page)
+{
+	__free_page(page);
+}
+
+static struct zs_pool_ops default_ops = {
+	.alloc_page = zs_alloc_page,
+	.free_page = zs_free_page
+};
+
+struct zs_pool *zs_create_pool(const char *name, gfp_t flags,
+			struct zs_pool_ops *ops)
 {
 	int i, error, ovhd_size;
 	struct zs_pool *pool;
@@ -492,7 +509,7 @@ struct zs_pool *zs_create_pool(const char *name, gfp_t flags)
 
 	ovhd_size = roundup(sizeof(*pool), PAGE_SIZE);
 	pool = kzalloc(ovhd_size, GFP_KERNEL);
-	if (!pool)
+	if (!pool || (ops && (!ops->alloc_page || !ops->free_page)))
 		return NULL;
 
 	for (i = 0; i < ZS_SIZE_CLASSES; i++) {
@@ -524,6 +541,10 @@ struct zs_pool *zs_create_pool(const char *name, gfp_t flags)
 
 	pool->flags = flags;
 	pool->name = name;
+	if (ops)
+		pool->ops = ops;
+	else
+		pool->ops = &default_ops;
 
 	error = 0; /* Success */
 
@@ -592,7 +613,7 @@ void *zs_malloc(struct zs_pool *pool, size_t size)
 
 	if (!first_page) {
 		spin_unlock(&class->lock);
-		first_page = alloc_zspage(class, pool->flags);
+		first_page = alloc_zspage(pool, class);
 		if (unlikely(!first_page))
 			return NULL;
 
@@ -658,7 +679,7 @@ void zs_free(struct zs_pool *pool, void *obj)
 	spin_unlock(&class->lock);
 
 	if (fullness == ZS_EMPTY)
-		free_zspage(first_page);
+		free_zspage(pool, first_page);
 }
 EXPORT_SYMBOL_GPL(zs_free);
 
diff --git a/drivers/staging/zsmalloc/zsmalloc.h b/drivers/staging/zsmalloc/zsmalloc.h
index 949384e..51fb32e 100644
--- a/drivers/staging/zsmalloc/zsmalloc.h
+++ b/drivers/staging/zsmalloc/zsmalloc.h
@@ -17,7 +17,13 @@
 
 struct zs_pool;
 
-struct zs_pool *zs_create_pool(const char *name, gfp_t flags);
+struct zs_pool_ops {
+	struct page * (*alloc_page)(gfp_t);
+	void (*free_page)(struct page *);
+};
+
+struct zs_pool *zs_create_pool(const char *name, gfp_t flags,
+			struct zs_pool_ops *ops);
 void zs_destroy_pool(struct zs_pool *pool);
 
 void *zs_malloc(struct zs_pool *pool, size_t size);
diff --git a/drivers/staging/zsmalloc/zsmalloc_int.h b/drivers/staging/zsmalloc/zsmalloc_int.h
index 92eefc6..ade09c1 100644
--- a/drivers/staging/zsmalloc/zsmalloc_int.h
+++ b/drivers/staging/zsmalloc/zsmalloc_int.h
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include "zsmalloc.h"
 
 /*
  * This must be power of 2 and greater than of equal to sizeof(link_free).
@@ -146,6 +147,7 @@ struct link_free {
 };
 
 struct zs_pool {
+	struct zs_pool_ops *ops;
 	struct size_class size_class[ZS_SIZE_CLASSES];
 
 	gfp_t flags;	/* allocation flags used when growing pool */
-- 
1.7.5.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] staging: zsmalloc: add user-definable alloc/free funcs

[Greg Kroah-Hartman]

On Fri, Mar 16, 2012 at 04:04:48PM -0500, Seth Jennings wrote:
> This patch allows a zsmalloc user to define the page
> allocation and free functions to be used when growing
> or releasing parts of the memory pool.
> 
> The functions are passed in the struct zs_pool_ops parameter
> of zs_create_pool() at pool creation time.  If this parameter
> is NULL, zsmalloc uses alloc_page and __free_page() by default.
> 
> While there is no current user of this functionality, zcache
> development plans to make use of it in the near future.

I'm starting to get tired of seeing new features be added to this chunk
of code, and the other related bits, without any noticable movement
toward getting it merged into the mainline tree.

So, I'm going to take a stance here and say, no more new features until
it gets merged into the "real" part of the kernel tree, as you all
should not be spinning your wheels on new stuff, when there's no
guarantee that the whole thing could just be rejected outright tomorrow.

I'm sorry, I know this isn't fair for your specific patch, but we have
to stop this sometime, and as this patch adds code isn't even used by
anyone, its a good of a time as any.

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] staging: zsmalloc: add user-definable alloc/free funcs

[Seth Jennings]

On 03/16/2012 04:32 PM, Greg Kroah-Hartman wrote:
> On Fri, Mar 16, 2012 at 04:04:48PM -0500, Seth Jennings wrote:
>> This patch allows a zsmalloc user to define the page
>> allocation and free functions to be used when growing
>> or releasing parts of the memory pool.
>>
>> The functions are passed in the struct zs_pool_ops parameter
>> of zs_create_pool() at pool creation time.  If this parameter
>> is NULL, zsmalloc uses alloc_page and __free_page() by default.
>>
>> While there is no current user of this functionality, zcache
>> development plans to make use of it in the near future.
> 
> I'm starting to get tired of seeing new features be added to this chunk
> of code, and the other related bits, without any noticable movement
> toward getting it merged into the mainline tree.

Fair enough

> 
> So, I'm going to take a stance here and say, no more new features until
> it gets merged into the "real" part of the kernel tree, as you all
> should not be spinning your wheels on new stuff, when there's no
> guarantee that the whole thing could just be rejected outright tomorrow.
> 
> I'm sorry, I know this isn't fair for your specific patch, but we have
> to stop this sometime, and as this patch adds code isn't even used by
> anyone, its a good of a time as any.

So, this the my first "promotion from staging" rodeo.  I would love to
see this code mainlined ASAP.  How would I/we go about doing that?

I guess another way to ask is, what needs to be done in the way of
code quality and acks in the community to promote zcache to
/drivers/misc for example?

Also, the tmem part of zcache will (probably, Dan?) be broken
out an placed in /lib.

--
Seth

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] staging: zsmalloc: add user-definable alloc/free funcs

[Greg Kroah-Hartman]

On Mon, Mar 19, 2012 at 01:54:56PM -0500, Seth Jennings wrote:
> > I'm sorry, I know this isn't fair for your specific patch, but we have
> > to stop this sometime, and as this patch adds code isn't even used by
> > anyone, its a good of a time as any.
> 
> So, this the my first "promotion from staging" rodeo.  I would love to
> see this code mainlined ASAP.  How would I/we go about doing that?

What subsystem should this code live in?  The -mm code, I'm guessing,
right?  If so, submit it to the linux-mm mailing list for inclusion, you
can point them at what is in drivers/staging right now, or probably it's
easier if you just make a new patch that adds the code that is in
drivers/staging/ to the correct location in the kernel.  That way it's
easier to review and change.  When it finally gets accepted, we can then
delete the drivers/staging code.

hope this helps,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] staging: zsmalloc: add user-definable alloc/free funcs

[Konrad Rzeszutek Wilk]

On Mon, Mar 19, 2012 at 04:34:09PM -0700, Greg Kroah-Hartman wrote:
> On Mon, Mar 19, 2012 at 01:54:56PM -0500, Seth Jennings wrote:
> > > I'm sorry, I know this isn't fair for your specific patch, but we have
> > > to stop this sometime, and as this patch adds code isn't even used by
> > > anyone, its a good of a time as any.
> > 
> > So, this the my first "promotion from staging" rodeo.  I would love to
> > see this code mainlined ASAP.  How would I/we go about doing that?
> 
> What subsystem should this code live in?  The -mm code, I'm guessing,
> right?  If so, submit it to the linux-mm mailing list for inclusion, you
> can point them at what is in drivers/staging right now, or probably it's
> easier if you just make a new patch that adds the code that is in
> drivers/staging/ to the correct location in the kernel.  That way it's
> easier to review and change.  When it finally gets accepted, we can then
> delete the drivers/staging code.


Hey Greg,

Little background - for zcache to kick-butts (both Dan and Seth posted some
pretty awesome benchmark numbers) it depends on the frontswap - which is in
the #linux-next. Dan made an attempt to post it for a GIT PULL and an interesting
conversation ensued where folks decided it needs more additions before they were
comfortable with it. zcache isn't using those additions, but I don't see why
it couldn't use them.

The things that bouncing around in my head are:
 - get a punch-out list (ie todo) of what MM needs for the zcache to get out.
   I think posting it as a new driver would be right way to do it (And then
   based on feedback work out the issues in drivers/staging). But what
   about authorship - there are mulitple authors ?

 - zcache is a bit different that the normal drivers type - and it is unclear
   yet what will be required to get it out - and both Seth and Nitin have this
   hungry look in their eyes of wanting to make it super-duper. So doing
   the work to do it - is not going to be a problem at all - just some form
   of clear goals of what we need "now" vs "would love to have".

 - folks are using it, which means continued -stable kernel back-porting.

So with that in mind I was wondering whether you would be up for:
 - me sending to you before a merge window some updates to the zcache
   as a git pull - that way you won't have to deal with a bunch of
   small patches and when there is something you don't like we can fix
   it up to your liking. The goal would be for us - Dan, Nitin, Seth and me
   working on promoting the driver out of staging and you won't have to
   be bugged every time we have a new change that might be perceived
   as feature, but is in fact a step towards mainstreaming it. I figured
   that is what you are most annoyed at - handling those uncoordinated
   requests and not seeing a clear target.

 - alongside of that, I work on making those frontswap changes folks
   have asked for. Since those changes can affect zcache, that means
   adding them in zcache alongside.

Hopefully, by the time those two items are done, both pieces can go in
the kernel at the same time-ish.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] staging: zsmalloc: add user-definable alloc/free funcs

[Greg Kroah-Hartman]

On Mon, Mar 26, 2012 at 11:50:19AM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Mar 19, 2012 at 04:34:09PM -0700, Greg Kroah-Hartman wrote:
> > On Mon, Mar 19, 2012 at 01:54:56PM -0500, Seth Jennings wrote:
> > > > I'm sorry, I know this isn't fair for your specific patch, but we have
> > > > to stop this sometime, and as this patch adds code isn't even used by
> > > > anyone, its a good of a time as any.
> > > 
> > > So, this the my first "promotion from staging" rodeo.  I would love to
> > > see this code mainlined ASAP.  How would I/we go about doing that?
> > 
> > What subsystem should this code live in?  The -mm code, I'm guessing,
> > right?  If so, submit it to the linux-mm mailing list for inclusion, you
> > can point them at what is in drivers/staging right now, or probably it's
> > easier if you just make a new patch that adds the code that is in
> > drivers/staging/ to the correct location in the kernel.  That way it's
> > easier to review and change.  When it finally gets accepted, we can then
> > delete the drivers/staging code.
> 
> 
> Hey Greg,
> 
> Little background - for zcache to kick-butts (both Dan and Seth posted some
> pretty awesome benchmark numbers) it depends on the frontswap - which is in
> the #linux-next. Dan made an attempt to post it for a GIT PULL and an interesting
> conversation ensued where folks decided it needs more additions before they were
> comfortable with it. zcache isn't using those additions, but I don't see why
> it couldn't use them.
> 
> The things that bouncing around in my head are:
>  - get a punch-out list (ie todo) of what MM needs for the zcache to get out.
>    I think posting it as a new driver would be right way to do it (And then
>    based on feedback work out the issues in drivers/staging). But what
>    about authorship - there are mulitple authors ?

What does authorship matter here?  To move files out of staging, just
send a patch that does that, all authorship history is preserved.

And as a new driver, that's up to the mm developers, not me.

>  - zcache is a bit different that the normal drivers type - and it is unclear
>    yet what will be required to get it out - and both Seth and Nitin have this
>    hungry look in their eyes of wanting to make it super-duper. So doing
>    the work to do it - is not going to be a problem at all - just some form
>    of clear goals of what we need "now" vs "would love to have".

Again, work with the -mm developers.

>  - folks are using it, which means continued -stable kernel back-porting.

What do you mean by this?

> So with that in mind I was wondering whether you would be up for:
>  - me sending to you before a merge window some updates to the zcache
>    as a git pull - that way you won't have to deal with a bunch of
>    small patches and when there is something you don't like we can fix
>    it up to your liking. The goal would be for us - Dan, Nitin, Seth and me
>    working on promoting the driver out of staging and you won't have to
>    be bugged every time we have a new change that might be perceived
>    as feature, but is in fact a step towards mainstreaming it. I figured
>    that is what you are most annoyed at - handling those uncoordinated
>    requests and not seeing a clear target.

Lots of small patches are fine, as long as they are obviously working
toward getting the code out of staging.  This specific patch was just
adding a new feature, one that no one could even use, so that was not
something that would help it get out of the staging tree.

So no, I don't need patches batched up, and a git pull, I just need to
see that every patch I am sent is working toward getting it out of here.

>  - alongside of that, I work on making those frontswap changes folks
>    have asked for. Since those changes can affect zcache, that means
>    adding them in zcache alongside.

Ok.

> Hopefully, by the time those two items are done, both pieces can go in
> the kernel at the same time-ish.

That would be good to see have happen.

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>