Re: [PATCH 4/4] zram: make deduplication feature optional

[Joonsoo Kim]

On Mon, Mar 27, 2017 at 05:11:05PM +0900, Sergey Senozhatsky wrote:
> On (03/23/17 12:05), Joonsoo Kim wrote:
> > On Wed, Mar 22, 2017 at 09:00:59AM +0900, Minchan Kim wrote:
> > > On Thu, Mar 16, 2017 at 11:46:38AM +0900, js1304@xxxxxxxxx wrote:
> > > > From: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> > > > 
> > > > Benefit of deduplication is dependent on the workload so it's not
> > > > preferable to always enable. Therefore, make it optional.
> > > 
> > > Please make it to Kconfig, too. And write down the description to impress
> > > "help a lot for users who uses zram to build output directory"
> > > And the feature should be disabled as default.
> > 
> > Okay.
> 
> 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.