linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Geliang Tang <geliangtang@gmail.com>
To: Minchan Kim <minchan@kernel.org>, Nitin Gupta <ngupta@vflare.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Dan Streetman <ddstreet@ieee.org>,
	Vitaly Wool <vitalywool@gmail.com>
Cc: Geliang Tang <geliangtang@gmail.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: [PATCH] zram: add zpool support v2
Date: Wed, 15 Jun 2016 22:42:06 +0800	[thread overview]
Message-ID: <cover.1466000844.git.geliangtang@gmail.com> (raw)
In-Reply-To: <CAMJBoFPA_7G4nEeaPzL6uAvewpvgAYMmJ-A2FwfDSYVyOBfShA@mail.gmail.com>

On Mon, Jun 13, 2016 at 11:11:00AM +0200, Vitaly Wool wrote:
> Den 8 juni 2016 6:33 em skrev "Dan Streetman" <ddstreet@ieee.org>:
> >
> > On Wed, Jun 8, 2016 at 5:39 AM, Geliang Tang <geliangtang@gmail.com>
> wrote:
> > > This patch adds zpool support for zram, it will allow us to use both
> > > the zpool api and directly zsmalloc api in zram.
> >
> > besides the problems below, this was discussed a while ago and I
> > believe Minchan is still against it, as nobody has so far shown what
> > the benefit to zram would be; zram doesn't need the predictability, or
> > evictability, of zbud or z3fold.
> 
> > Right.
> >
> > Geliang, I cannot ack without any *detail* that what's the problem of
> > zram/zsmalloc, why we can't fix it in zsmalloc itself.
> > The zbud and zsmalloc is otally different design to aim different goal
> > determinism vs efficiency so you can choose what you want between
> > zswap
> > and zram rather than mixing the features.
>
> I'd also probably Cc Vitaly Wool on this
>
> Well, I believe I have something to say here. z3fold is generally faster
> than zsmalloc which makes it a better choice for zram sometimes, e.g. when
> zram device is used for swap. Also,  z3fold and zbud do not require MMU so
> zram over these can be used on small Linux powered MMU-less IoT devices, as
> opposed to the traditional zram over zsmalloc. Otherwise I do agree with
> Dan.
> 
> >
> > It doesn't make sense for zram to conditionally use zpool; either it
> > uses it and thus has 'select ZPOOL' in its Kconfig entry, or it
> > doesn't use it at all.
> >
> > > +#endif
> >
> > first, no.  this obviously makes using zpool in zram completely pointless.
> >
> > second, did you test this?  the pool you're passing is the zpool, not
> > the zs_pool.  quite bad things will happen when this code runs.  There
> > is no way to get the zs_pool from the zpool object (that's the point
> > of abstraction, of course).
> >
> > The fact zpool doesn't have these apis (currently) is one of the
> > reasons against changing zram to use zpool.
> >

Thank you all for your reply. I updated the patch and I hope this is better.

Geliang Tang (1):
  zram: update zram to use zpool

 drivers/block/zram/Kconfig    |  3 ++-
 drivers/block/zram/zram_drv.c | 59 ++++++++++++++++++++++---------------------
 drivers/block/zram/zram_drv.h |  4 +--
 mm/zsmalloc.c                 | 12 +++++----
 4 files changed, 41 insertions(+), 37 deletions(-)

-- 
2.5.5

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2016-06-15 14:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-08  9:39 [PATCH] zram: add zpool support Geliang Tang
2016-06-08 14:51 ` Dan Streetman
2016-06-09  1:34   ` Minchan Kim
2016-06-09  1:43     ` Sergey Senozhatsky
2016-06-13  9:11   ` Vitaly Wool
2016-06-13 20:04     ` Fwd: " Vitaly Wool
2016-06-15 14:42     ` Geliang Tang [this message]
2016-06-15 14:42       ` [PATCH] zram: update zram to use zpool Geliang Tang
2016-06-15 14:54         ` Dan Streetman
2016-06-15 15:33         ` kbuild test robot
2016-06-15 23:17         ` Minchan Kim
2016-06-17  8:30           ` Vitaly Wool
2016-06-17 12:28             ` Austin S. Hemmelgarn
2016-06-20  8:00             ` 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=cover.1466000844.git.geliangtang@gmail.com \
    --to=geliangtang@gmail.com \
    --cc=ddstreet@ieee.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=ngupta@vflare.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=vitalywool@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).