public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-25  5:51 [PATCH v2 1/3] zram/zcomp: use GFP_NOIO to allocate streams Minchan Kim
@ 2015-11-25  5:51 ` Minchan Kim
  2015-11-25 12:46   ` Sergey Senozhatsky
  2015-11-27  2:05   ` Sergey Senozhatsky
  0 siblings, 2 replies; 11+ messages in thread
From: Minchan Kim @ 2015-11-25  5:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Sergey Senozhatsky, Kyeongdon Kim, Minchan Kim

Each zcomp backend uses own gfp flag but it's pointless
because the context they could be called is driven by upper
layer(ie, zcomp frontend). As well, zcomp frondend could
call them in different context. One context(ie, zram init part)
is it should be better to make sure successful allocation
other context(ie, further stream allocation part for accelarating
I/O speed) is just optional so let's pass gfp down from driver
(ie, zcomp frontend) like normal MM convention.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 drivers/block/zram/zcomp.c     | 24 ++++++++++++++++--------
 drivers/block/zram/zcomp.h     |  2 +-
 drivers/block/zram/zcomp_lz4.c | 18 +++---------------
 drivers/block/zram/zcomp_lzo.c | 19 ++++---------------
 4 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index c53617752b93..e7c197ede919 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -74,18 +74,18 @@ static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
  * allocate new zcomp_strm structure with ->private initialized by
  * backend, return NULL on error
  */
-static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
+static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)
 {
-	struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), GFP_NOIO);
+	struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), flags);
 	if (!zstrm)
 		return NULL;
 
-	zstrm->private = comp->backend->create();
+	zstrm->private = comp->backend->create(flags);
 	/*
 	 * allocate 2 pages. 1 for compressed data, plus 1 extra for the
 	 * case when compressed size is larger than the original one
 	 */
-	zstrm->buffer = (void *)__get_free_pages(GFP_NOIO | __GFP_ZERO, 1);
+	zstrm->buffer = (void *)__get_free_pages(flags | __GFP_ZERO, 1);
 	if (!zstrm->private || !zstrm->buffer) {
 		zcomp_strm_free(comp, zstrm);
 		zstrm = NULL;
@@ -120,8 +120,16 @@ static struct zcomp_strm *zcomp_strm_multi_find(struct zcomp *comp)
 		/* allocate new zstrm stream */
 		zs->avail_strm++;
 		spin_unlock(&zs->strm_lock);
-
-		zstrm = zcomp_strm_alloc(comp);
+		/*
+		 * This function could be called in swapout/fs write path
+		 * so we couldn't use GFP_FS|IO. And it assumes we already
+		 * have at least one stream in zram initialization so we
+		 * don't do best effort to allocate more stream in here.
+		 * A default stream will work well without further multiple
+		 * stream. That's why we use __GFP_NORETRY|NOWARN|NOMEMALLOC.
+		 */
+		zstrm = zcomp_strm_alloc(comp, GFP_NOIO|__GFP_NORETRY|
+					__GFP_NOWARN|__GFP_NOMEMALLOC);
 		if (!zstrm) {
 			spin_lock(&zs->strm_lock);
 			zs->avail_strm--;
@@ -209,7 +217,7 @@ static int zcomp_strm_multi_create(struct zcomp *comp, int max_strm)
 	zs->max_strm = max_strm;
 	zs->avail_strm = 1;
 
-	zstrm = zcomp_strm_alloc(comp);
+	zstrm = zcomp_strm_alloc(comp, GFP_KERNEL);
 	if (!zstrm) {
 		kfree(zs);
 		return -ENOMEM;
@@ -259,7 +267,7 @@ static int zcomp_strm_single_create(struct zcomp *comp)
 
 	comp->stream = zs;
 	mutex_init(&zs->strm_lock);
-	zs->zstrm = zcomp_strm_alloc(comp);
+	zs->zstrm = zcomp_strm_alloc(comp, GFP_KERNEL);
 	if (!zs->zstrm) {
 		kfree(zs);
 		return -ENOMEM;
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 46e2b9f8f1f0..b7d2a4bcae54 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -33,7 +33,7 @@ struct zcomp_backend {
 	int (*decompress)(const unsigned char *src, size_t src_len,
 			unsigned char *dst);
 
-	void *(*create)(void);
+	void *(*create)(gfp_t flags);
 	void (*destroy)(void *private);
 
 	const char *name;
diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
index 715df0e48c13..4e0cb4a4acf7 100644
--- a/drivers/block/zram/zcomp_lz4.c
+++ b/drivers/block/zram/zcomp_lz4.c
@@ -15,25 +15,13 @@
 
 #include "zcomp_lz4.h"
 
-static void *zcomp_lz4_create(void)
+static void *zcomp_lz4_create(gfp_t flags)
 {
 	void *ret;
 
-	/*
-	 * This function could be called in swapout/fs write path
-	 * so we couldn't use GFP_FS|IO. And it assumes we already
-	 * have at least one stream in zram initialization so we
-	 * don't do best effort to allocate more stream in here.
-	 * A default stream will work well without further multiple
-	 * stream. That's why we use  __GFP_NORETRY|NOWARN|NOMEMALLOC.
-	 */
-	ret = kzalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
-					__GFP_NOWARN|__GFP_NOMEMALLOC);
+	ret = kmalloc(LZ4_MEM_COMPRESS, flags);
 	if (!ret)
-		ret = __vmalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
-						__GFP_NOWARN|__GFP_NOMEMALLOC|
-						__GFP_ZERO,
-						PAGE_KERNEL);
+		ret = __vmalloc(LZ4_MEM_COMPRESS, flags, PAGE_KERNEL);
 	return ret;
 }
 
diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
index 639b94affbfd..ac4597506314 100644
--- a/drivers/block/zram/zcomp_lzo.c
+++ b/drivers/block/zram/zcomp_lzo.c
@@ -15,24 +15,13 @@
 
 #include "zcomp_lzo.h"
 
-static void *lzo_create(void)
+static void *lzo_create(gfp_t flags)
 {
 	void *ret;
-	/*
-	 * This function could be called in swapout/fs write path
-	 * so we couldn't use GFP_FS|IO. And it assumes we already
-	 * have at least one stream in zram initialization so we
-	 * don't do best effort to allocate more stream in here.
-	 * A default stream will work well without further multiple
-	 * stream. That's why we use  __GFP_NORETRY|NOWARN|NOMEMALLOC.
-	 */
-	ret = kzalloc(LZO1X_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
-					__GFP_NOWARN|__GFP_NOMEMALLOC);
+
+	ret = kmalloc(LZO1X_MEM_COMPRESS, flags);
 	if (!ret)
-		ret = __vmalloc(LZO1X_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
-						__GFP_NOWARN|__GFP_NOMEMALLOC|
-						__GFP_ZERO,
-						PAGE_KERNEL);
+		ret = __vmalloc(LZO1X_MEM_COMPRESS, flags, PAGE_KERNEL);
 	return ret;
 }
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-25  5:51 ` [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend Minchan Kim
@ 2015-11-25 12:46   ` Sergey Senozhatsky
  2015-11-25 13:50     ` Minchan Kim
  2015-11-27  2:05   ` Sergey Senozhatsky
  1 sibling, 1 reply; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-25 12:46 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-kernel, Sergey Senozhatsky, Kyeongdon Kim

On (11/25/15 14:51), Minchan Kim wrote:
[..]
> +		/*
> +		 * This function could be called in swapout/fs write path
> +		 * so we couldn't use GFP_FS|IO. And it assumes we already
> +		 * have at least one stream in zram initialization so we
> +		 * don't do best effort to allocate more stream in here.
> +		 * A default stream will work well without further multiple
> +		 * stream. That's why we use __GFP_NORETRY|NOWARN|NOMEMALLOC.
> +		 */
> +		zstrm = zcomp_strm_alloc(comp, GFP_NOIO|__GFP_NORETRY|
> +					__GFP_NOWARN|__GFP_NOMEMALLOC);
[..]

I think that applying 3/3 before 2/3 will be a simpler (and probably a better)
thing to do. We fitst extend zcomp interface and pass flags (without any
functional change) and then extend the flags and introduce vmalloc fallback.

So we don't have to add comments to lz4/lzo backend that are getting (re-)moved
in the very next commit.

I will send swapped 2 and 3 patches shortly (I didn't change commit
messages and SoBs). Please take a look.

> -	/*
> -	 * This function could be called in swapout/fs write path
> -	 * so we couldn't use GFP_FS|IO. And it assumes we already
> -	 * have at least one stream in zram initialization so we
> -	 * don't do best effort to allocate more stream in here.
> -	 * A default stream will work well without further multiple
> -	 * stream. That's why we use  __GFP_NORETRY|NOWARN|NOMEMALLOC.
> -	 */
> -	ret = kzalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -					__GFP_NOWARN|__GFP_NOMEMALLOC);
> +	ret = kmalloc(LZ4_MEM_COMPRESS, flags);
>  	if (!ret)
> -		ret = __vmalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -						__GFP_NOWARN|__GFP_NOMEMALLOC|
> -						__GFP_ZERO,
> -						PAGE_KERNEL);
> +		ret = __vmalloc(LZ4_MEM_COMPRESS, flags, PAGE_KERNEL);
[..]

__vmalloc() is still missing __GFP_HIGHMEM

> -	/*
> -	 * This function could be called in swapout/fs write path
> -	 * so we couldn't use GFP_FS|IO. And it assumes we already
> -	 * have at least one stream in zram initialization so we
> -	 * don't do best effort to allocate more stream in here.
> -	 * A default stream will work well without further multiple
> -	 * stream. That's why we use  __GFP_NORETRY|NOWARN|NOMEMALLOC.
> -	 */
> -	ret = kzalloc(LZO1X_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -					__GFP_NOWARN|__GFP_NOMEMALLOC);
> +
> +	ret = kmalloc(LZO1X_MEM_COMPRESS, flags);
>  	if (!ret)
> -		ret = __vmalloc(LZO1X_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -						__GFP_NOWARN|__GFP_NOMEMALLOC|
> -						__GFP_ZERO,
> -						PAGE_KERNEL);
> +		ret = __vmalloc(LZO1X_MEM_COMPRESS, flags, PAGE_KERNEL);

ditto.

	-ss

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-25 12:46   ` Sergey Senozhatsky
@ 2015-11-25 13:50     ` Minchan Kim
  2015-11-25 15:04       ` Sergey Senozhatsky
  0 siblings, 1 reply; 11+ messages in thread
From: Minchan Kim @ 2015-11-25 13:50 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: Andrew Morton, linux-kernel, Kyeongdon Kim

Hi Sergey,

On Wed, Nov 25, 2015 at 09:46:47PM +0900, Sergey Senozhatsky wrote:
> On (11/25/15 14:51), Minchan Kim wrote:
> [..]
> > +		/*
> > +		 * This function could be called in swapout/fs write path
> > +		 * so we couldn't use GFP_FS|IO. And it assumes we already
> > +		 * have at least one stream in zram initialization so we
> > +		 * don't do best effort to allocate more stream in here.
> > +		 * A default stream will work well without further multiple
> > +		 * stream. That's why we use __GFP_NORETRY|NOWARN|NOMEMALLOC.
> > +		 */
> > +		zstrm = zcomp_strm_alloc(comp, GFP_NOIO|__GFP_NORETRY|
> > +					__GFP_NOWARN|__GFP_NOMEMALLOC);
> [..]
> 
> I think that applying 3/3 before 2/3 will be a simpler (and probably a better)
> thing to do. We fitst extend zcomp interface and pass flags (without any
> functional change) and then extend the flags and introduce vmalloc fallback.

The reason I ordered such way is that I wanted to discuss [2/3] as
stable material after I get your ACK. It solves real problem in android platform
which is real fact and I think it's enough small to send stable tree.
What do you think?

> 
> So we don't have to add comments to lz4/lzo backend that are getting (re-)moved
> in the very next commit.

Fair enough if you don't agree sending [2/3] to stable.

> 
> I will send swapped 2 and 3 patches shortly (I didn't change commit
> messages and SoBs). Please take a look.

Thanks!

> 
> > -	/*
> > -	 * This function could be called in swapout/fs write path
> > -	 * so we couldn't use GFP_FS|IO. And it assumes we already
> > -	 * have at least one stream in zram initialization so we
> > -	 * don't do best effort to allocate more stream in here.
> > -	 * A default stream will work well without further multiple
> > -	 * stream. That's why we use  __GFP_NORETRY|NOWARN|NOMEMALLOC.
> > -	 */
> > -	ret = kzalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> > -					__GFP_NOWARN|__GFP_NOMEMALLOC);
> > +	ret = kmalloc(LZ4_MEM_COMPRESS, flags);
> >  	if (!ret)
> > -		ret = __vmalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> > -						__GFP_NOWARN|__GFP_NOMEMALLOC|
> > -						__GFP_ZERO,
> > -						PAGE_KERNEL);
> > +		ret = __vmalloc(LZ4_MEM_COMPRESS, flags, PAGE_KERNEL);
> [..]
> 
> __vmalloc() is still missing __GFP_HIGHMEM

Argh, Sorry.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-25 13:50     ` Minchan Kim
@ 2015-11-25 15:04       ` Sergey Senozhatsky
  2015-11-25 15:20         ` Minchan Kim
  0 siblings, 1 reply; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-25 15:04 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, Kyeongdon Kim

Hello,

On (11/25/15 22:50), Minchan Kim wrote:
[..]
> > I think that applying 3/3 before 2/3 will be a simpler (and probably a better)
> > thing to do. We fitst extend zcomp interface and pass flags (without any
> > functional change) and then extend the flags and introduce vmalloc fallback.
> 
> The reason I ordered such way is that I wanted to discuss [2/3] as
> stable material after I get your ACK. It solves real problem in android platform
> which is real fact and I think it's enough small to send stable tree.
> What do you think?
> 
> > 
> > So we don't have to add comments to lz4/lzo backend that are getting (re-)moved
> > in the very next commit.
> 
> Fair enough if you don't agree sending [2/3] to stable.

Aha, I see. I don't mind to send it to -stable (with __GFP_HIGHMEM fix
up).

Do -stable guys take this type of patches?

	-ss

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-25 15:04       ` Sergey Senozhatsky
@ 2015-11-25 15:20         ` Minchan Kim
  2015-11-26  7:39           ` Sergey Senozhatsky
  0 siblings, 1 reply; 11+ messages in thread
From: Minchan Kim @ 2015-11-25 15:20 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: Andrew Morton, linux-kernel, Kyeongdon Kim

On Thu, Nov 26, 2015 at 12:04:49AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (11/25/15 22:50), Minchan Kim wrote:
> [..]
> > > I think that applying 3/3 before 2/3 will be a simpler (and probably a better)
> > > thing to do. We fitst extend zcomp interface and pass flags (without any
> > > functional change) and then extend the flags and introduce vmalloc fallback.
> > 
> > The reason I ordered such way is that I wanted to discuss [2/3] as
> > stable material after I get your ACK. It solves real problem in android platform
> > which is real fact and I think it's enough small to send stable tree.
> > What do you think?
> > 
> > > 
> > > So we don't have to add comments to lz4/lzo backend that are getting (re-)moved
> > > in the very next commit.
> > 
> > Fair enough if you don't agree sending [2/3] to stable.
> 
> Aha, I see. I don't mind to send it to -stable (with __GFP_HIGHMEM fix
> up).

Sure.
Can I add your acked-by for [2/3] and [3/3]?

And I will keep order and add stable mark in [2/3].

> 
> Do -stable guys take this type of patches?

I think so because it's real problem fix and enough small to backport.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
@ 2015-11-26  7:24 Sergey Senozhatsky
  0 siblings, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-26  7:24 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Kyeongdon Kim, linux-kernel, Sergey Senozhatsky,
	Sergey Senozhatsky

Minchan Kim <minchan@kernel.org> wrote:
[..]
> > Aha, I see. I don't mind to send it to -stable (with __GFP_HIGHMEM fix
> > up).

Hello Minchan,

Sorry for not replying sooner.

> Sure.
> Can I add your acked-by for [2/3] and [3/3]?
> 
> And I will keep order and add stable mark in [2/3].

yes.


a) + __GFP_HIGHMEM in 2/3 and 3/3

b) can I add two small nitpicks from my side?

#1 s/could/can/ ?
-  * This function could be called in swapout/fs write path
-  * so we couldn't use GFP_FS|IO. And it assumes we already

+  "This function can be called in swapout/fs write path so we can't use"

#2 can you please add spaces around GFP flags? it's just a bit easier to read.

   GFP_NOIO | __GFP_HIGHMEM | __GFP_NORETRY | __GFP_NOWARN | __GFP_NOMEMALLOC
vs
   GFP_NOIO|__GFP_HIGHMEM|__GFP_NORETRY|__GFP_NOWARN|__GFP_NOMEMALLOC


c) Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Thank you.

	-ss

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-25 15:20         ` Minchan Kim
@ 2015-11-26  7:39           ` Sergey Senozhatsky
  0 siblings, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-26  7:39 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Kyeongdon Kim, linux-kernel, Sergey Senozhatsky,
	Sergey Senozhatsky

[sorry, ended up screwing up Message-Id in In-Reply-To! resending]
------------------------------------------------------------------


Minchan Kim <minchan@kernel.org> wrote:
[..]
> > Aha, I see. I don't mind to send it to -stable (with __GFP_HIGHMEM fix
> > up).

Hello Minchan,

Sorry for not replying sooner.

> Sure.
> Can I add your acked-by for [2/3] and [3/3]?
> 
> And I will keep order and add stable mark in [2/3].

yes.


a) + __GFP_HIGHMEM in 2/3 and 3/3

b) can I add two small nitpicks from my side?

#1 s/could/can/ ?
-  * This function could be called in swapout/fs write path
-  * so we couldn't use GFP_FS|IO. And it assumes we already

+  "This function can be called in swapout/fs write path so we can't use"

#2 can you please add spaces around GFP flags? it's just a bit easier to read.

   GFP_NOIO | __GFP_HIGHMEM | __GFP_NORETRY | __GFP_NOWARN | __GFP_NOMEMALLOC
vs
   GFP_NOIO|__GFP_HIGHMEM|__GFP_NORETRY|__GFP_NOWARN|__GFP_NOMEMALLOC


c) Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Thank you.

	-ss

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-25  5:51 ` [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend Minchan Kim
  2015-11-25 12:46   ` Sergey Senozhatsky
@ 2015-11-27  2:05   ` Sergey Senozhatsky
  2015-11-27  2:19     ` Sergey Senozhatsky
       [not found]     ` <CAEwNFnC5edvy2a+-gXP0xU1QPGKrEhQhr_cR6KGL2teskwZPpg@mail.gmail.com>
  1 sibling, 2 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-27  2:05 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Kyeongdon Kim, linux-kernel, Sergey Senozhatsky,
	Sergey Senozhatsky

Hello,

Minchan Kim wrote:
[..]
> +static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)
>  {
> +               zstrm = zcomp_strm_alloc(comp, GFP_NOIO|__GFP_NORETRY|
> +                                       __GFP_NOWARN|__GFP_NOMEMALLOC);


and it seems that after 3/3 (v2) we also lost GFP_ZERO for private
allocation. kzalloc->kmalloc, and no explicit __GFP_ZERO for __vmalloc().


> -       ret = kzalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -                                       __GFP_NOWARN|__GFP_NOMEMALLOC);
> +       ret = kmalloc(LZ4_MEM_COMPRESS, flags);
		^^^ here
>         if (!ret)
> -               ret = __vmalloc(LZ4_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -                                               __GFP_NOWARN|__GFP_NOMEMALLOC|
> -                                               __GFP_ZERO,
> -                                               PAGE_KERNEL);
> +               ret = __vmalloc(LZ4_MEM_COMPRESS, flags, PAGE_KERNEL);
			^^^ here


> -       ret = kzalloc(LZO1X_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -                                       __GFP_NOWARN|__GFP_NOMEMALLOC);
> +
> +       ret = kmalloc(LZO1X_MEM_COMPRESS, flags);
		^^^ ditto
>         if (!ret)
> -               ret = __vmalloc(LZO1X_MEM_COMPRESS, GFP_NOIO|__GFP_NORETRY|
> -                                               __GFP_NOWARN|__GFP_NOMEMALLOC|
> -                                               __GFP_ZERO,
> -                                               PAGE_KERNEL);
> +               ret = __vmalloc(LZO1X_MEM_COMPRESS, flags, PAGE_KERNEL);
			^^^ ditto


so lz*_create() should look like

---

static void *lzo_create(gfp_t flags)
{
        void *ret;

        ret = kzalloc(LZO1X_MEM_COMPRESS, flags);
        if (!ret)
                ret = __vmalloc(LZO1X_MEM_COMPRESS,
                                flags | __GFP_ZERO | __GFP_HIGHMEM,
                                PAGE_KERNEL);
        return ret;
}

---


I have a patchset with my nitpicks addressed and fix-ups for missing gfps.
Do you mind me to send it out?

	-ss

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-27  2:05   ` Sergey Senozhatsky
@ 2015-11-27  2:19     ` Sergey Senozhatsky
  2015-11-27  2:30       ` Sergey Senozhatsky
       [not found]     ` <CAEwNFnC5edvy2a+-gXP0xU1QPGKrEhQhr_cR6KGL2teskwZPpg@mail.gmail.com>
  1 sibling, 1 reply; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-27  2:19 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Kyeongdon Kim, linux-kernel, Sergey Senozhatsky,
	Sergey Senozhatsky

On (11/27/15 11:05), Sergey Senozhatsky wrote:
> Minchan Kim wrote:
> [..]
> > +static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)
> >  {
> > +               zstrm = zcomp_strm_alloc(comp, GFP_NOIO|__GFP_NORETRY|
> > +                                       __GFP_NOWARN|__GFP_NOMEMALLOC);
> 
> 
> and it seems that after 3/3 (v2) we also lost GFP_ZERO for private
> allocation. kzalloc->kmalloc, and no explicit __GFP_ZERO for __vmalloc().
> 

well, we probably don't really need __GFP_ZERO for ->private, but
let's address it in a separate patch, not as an undocumented change.

	-ss

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
  2015-11-27  2:19     ` Sergey Senozhatsky
@ 2015-11-27  2:30       ` Sergey Senozhatsky
  0 siblings, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-27  2:30 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Kyeongdon Kim, linux-kernel, Sergey Senozhatsky,
	Sergey Senozhatsky

On (11/27/15 11:19), Sergey Senozhatsky wrote:
[..]
> > > +static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)
> > >  {
> > > +               zstrm = zcomp_strm_alloc(comp, GFP_NOIO|__GFP_NORETRY|
> > > +                                       __GFP_NOWARN|__GFP_NOMEMALLOC);
> > 
> > 
> > and it seems that after 3/3 (v2) we also lost GFP_ZERO for private
> > allocation. kzalloc->kmalloc, and no explicit __GFP_ZERO for __vmalloc().
> > 
> 
> well, we probably don't really need __GFP_ZERO for ->private, but
> let's address it in a separate patch, not as an undocumented change.

hm... something like this perhaps. I don't think there any security
issues with removing __GFP_ZERO, we have 'garbage' in ->private anyway.


    zram/zcomp: do not zero out zcomp private pages
    
    Do not __GFP_ZERO allocated zcomp ->private pages. We keep
    allocated streams around and use them for read/write requests,
    so we supply a zeroed out ->private to compression algorithm
    as a scratch buffer only once -- the first time we use that
    stream. For the rest of IO requests served by this stream
    ->private usually contains some temporarily data from the
    previous requests.

---

diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
index dc2338d..0110086 100644
--- a/drivers/block/zram/zcomp_lz4.c
+++ b/drivers/block/zram/zcomp_lz4.c
@@ -19,10 +19,10 @@ static void *zcomp_lz4_create(gfp_t flags)
 {
        void *ret;
 
-       ret = kzalloc(LZ4_MEM_COMPRESS, flags);
+       ret = kmalloc(LZ4_MEM_COMPRESS, flags);
        if (!ret)
                ret = __vmalloc(LZ4_MEM_COMPRESS,
-                               flags | __GFP_ZERO | __GFP_HIGHMEM,
+                               flags | __GFP_HIGHMEM,
                                PAGE_KERNEL);
        return ret;
 }
diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
index 0ab6fce..ed7a1f0 100644
--- a/drivers/block/zram/zcomp_lzo.c
+++ b/drivers/block/zram/zcomp_lzo.c
@@ -19,10 +19,10 @@ static void *lzo_create(gfp_t flags)
 {
        void *ret;
 
-       ret = kzalloc(LZO1X_MEM_COMPRESS, flags);
+       ret = kmalloc(LZO1X_MEM_COMPRESS, flags);
        if (!ret)
                ret = __vmalloc(LZO1X_MEM_COMPRESS,
-                               flags | __GFP_ZERO | __GFP_HIGHMEM,
+                               flags | __GFP_HIGHMEM,
                                PAGE_KERNEL);
        return ret;
 }

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend
       [not found]     ` <CAEwNFnC5edvy2a+-gXP0xU1QPGKrEhQhr_cR6KGL2teskwZPpg@mail.gmail.com>
@ 2015-11-27  3:31       ` Sergey Senozhatsky
  0 siblings, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2015-11-27  3:31 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Sergey Senozhatsky, Andrew Morton, Kyeongdon Kim,
	linux-kernel@vger.kernel.org, Sergey Senozhatsky

On (11/27/15 11:22), Minchan Kim wrote:
[..]
>>      I have a patchset with my nitpicks addressed and fix-ups for missing
>>      gfps.
>>      Do you mind me to send it out?

>    Hey Sergey
>    I am on vacation and outside now. so if you resend with things you pointed

Oh, lovely, have a good one!

>    out I am very happy.
>    Pz ask Kyeongdon to test new patch set again because we changed his patch
>    a bit and suppose to mark as stable.

ok, will do. thanks.

	-ss

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-11-27  3:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-26  7:24 [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend Sergey Senozhatsky
  -- strict thread matches above, loose matches on Subject: below --
2015-11-25  5:51 [PATCH v2 1/3] zram/zcomp: use GFP_NOIO to allocate streams Minchan Kim
2015-11-25  5:51 ` [PATCH v2 3/3] zram: pass gfp from zcomp frontend to backend Minchan Kim
2015-11-25 12:46   ` Sergey Senozhatsky
2015-11-25 13:50     ` Minchan Kim
2015-11-25 15:04       ` Sergey Senozhatsky
2015-11-25 15:20         ` Minchan Kim
2015-11-26  7:39           ` Sergey Senozhatsky
2015-11-27  2:05   ` Sergey Senozhatsky
2015-11-27  2:19     ` Sergey Senozhatsky
2015-11-27  2:30       ` Sergey Senozhatsky
     [not found]     ` <CAEwNFnC5edvy2a+-gXP0xU1QPGKrEhQhr_cR6KGL2teskwZPpg@mail.gmail.com>
2015-11-27  3:31       ` Sergey Senozhatsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox