All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Kyeongdon Kim <kyeongdon.kim@lge.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [PATCH 2/3] zram: try vmalloc() after kmalloc()
Date: Tue, 24 Nov 2015 15:12:58 +0900	[thread overview]
Message-ID: <20151124061258.GJ705@swordfish> (raw)
In-Reply-To: <1448337696-10689-1-git-send-email-minchan@kernel.org>

[..]
>  static void *zcomp_lz4_create(void)
>  {
> -       return kzalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
> +       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_NORETRY|__GFP_NOWARN|__GFP_NOMEMALLOC);
> +       if (!ret)
> +               ret = __vmalloc(LZ4_MEM_COMPRESS,
> +                                       __GFP_NORETRY|__GFP_NOWARN|
> +                                       __GFP_NOMEMALLOC|__GFP_ZERO,
> +                                       PAGE_KERNEL);
> +       return ret;
>  }
[..]

so this change is still questionable. is there a real value in having
a vmalloc() fallback in the middle of allocations sequence:

	zstrm = kmalloc(sizeof(*zstrm), GFP_NOIO);
		^^^ ok, can fail here

	zstrm->zstrm->private = comp->backend->create();
				^^^ kzalloc() and vmalloc() fallback ??

	zstrm->buffer = (void *)__get_free_pages(GFP_NOIO | __GFP_ZERO, 1);
		^^^ can fail here again.

can you please comment on this?


and I'd prefer it to be a bit different -- use likely path first and
avoid an assignment in unlikely path.
... and add GFP_NOIO to both kzalloc() and __vmalloc().

and there is no __GFP_HIGHMEM in __vmalloc() call?

something like this:

---


	ret = kzalloc(LZ4_MEM_COMPRESS, GFP_NOIO | __GFP_NORETRY |
					__GFP_NOWARN | __GFP_NOMEMALLOC);
	if (ret)
		return ret;

	return __vmalloc(LZ4_MEM_COMPRESS,
			GFP_NOIO | __GFP_NOWARN | __GFP_HIGHMEM | __GFP_ZERO,
			PAGE_KERNEL);


	-ss

  parent reply	other threads:[~2015-11-24  6:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-24  4:01 [PATCH 1/3] zram/zcomp: use GFP_NOIO to allocate streams Minchan Kim
2015-11-24  4:01 ` [PATCH 2/3] zram: try vmalloc() after kmalloc() Minchan Kim
2015-11-24  4:01 ` [RFC 3/3] zram: pass gfp from zcomp frontend to backend Minchan Kim
2015-11-24  6:00 ` [PATCH 1/3] zram/zcomp: use GFP_NOIO to allocate streams Sergey Senozhatsky
2015-11-24  7:10   ` Minchan Kim
2015-11-24  6:12 ` Sergey Senozhatsky [this message]
2015-11-24  6:42   ` [PATCH 2/3] zram: try vmalloc() after kmalloc() Sergey Senozhatsky
2015-11-24  7:08     ` Minchan Kim
2015-11-24  7:03   ` Minchan Kim
2015-11-24  7:36     ` Sergey Senozhatsky
2015-11-24  7:57       ` Minchan Kim
2015-11-24  8:07         ` Sergey Senozhatsky
2015-11-24  8:14           ` Minchan Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151124061258.GJ705@swordfish \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=kyeongdon.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minchan@kernel.org \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.