Date: Tue, 28 Mar 2017 11:22:45 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
        Minchan Kim <minchan@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        linux-kernel@vger.kernel.org, kernel-team@lge.com,
        Seth Jennings <sjenning@redhat.com>, Dan Streetman <ddstreet@ieee.org>
Subject: Re: [PATCH 4/4] zram: make deduplication feature optional
Message-ID: <20170328022244.GB10573@jagdpanzerIV.localdomain>
References: <1489632398-31501-1-git-send-email-iamjoonsoo.kim@lge.com>
 <1489632398-31501-5-git-send-email-iamjoonsoo.kim@lge.com>
 <20170322000059.GB30149@bbox>
 <20170323030530.GC17486@js1304-P5Q-DELUXE>
 <20170327081105.GA390@jagdpanzerIV.localdomain>
 <20170328010217.GB8462@js1304-P5Q-DELUXE>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170328010217.GB8462@js1304-P5Q-DELUXE>
User-Agent: Mutt/1.8.0 (2017-02-23)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2192
Lines: 56

Cc Seth and Dan, just in case


Hello Joonsoo,

On (03/28/17 10:02), Joonsoo Kim wrote:
[..]
> > so I was thinking for a moment -- do we want to keep this
> > functionality in zram or may be it belongs to allocator (zsmalloc)?
> > what do you think? just a question.
> 
> I think that zram is more appropriate layer to implement this feature.
> I may be wrong so please let me know if I'm missing something.
> 
> First, I'd like to leave allocator to just allocator. If it awares the
> contents, further improvement would be restricted. For example, we
> should use map/unmap semantic to store contents, since, without them,
> we can't know when the content is changed and when deduplication check
> should be done. I know that zsmalloc is already implemented by that
> way but I guess that similar issue could happen in the future.
> 
> Second, we always need to compress the page to check duplication
> if it is implemented in zsmalloc since we store compressed page to
> zsmalloc. I guess that less compression would be better in performance
> wise.
> 
> Third, in case of zsmalloc dedup, we always need to allocate zs memory
> before checking duplication and need to free it if duplication is
> found. It's also undesirable.
> 
> If you are okay with above arguments, I will send v2 soon.

thanks.
I'm OK with your arguments.


to explain my point a bit further (zsmalloc was a bad call,
I guess I meant zpool):

the reason I asked was that both zram and zswap sort of trying to
have same optimizations - zero filled pages handling, for example.
zram is a bit ahead now (to the best of my knowledge), because of
the recent 'same element' filled pages. zswap, probably, will have
something like this as well some day. or may be it won't, up to Seth
and Dan. de-duplication definitely can improve both zram and zswap,
which, once again, suggests that at some point zswap will have its
own implementation. well, or it won't.

so I though that may be we could have zero filled pages handling/same
element pages handling/de-duplication somewhere in the "middle" layer.
like zpool for instance (zram does not support zpool as of now) so we
could unify things.

just an idea. no pressure.

	-ss