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

[Minchan Kim]

On Tue, Mar 28, 2017 at 02:12:04PM +0900, Sergey Senozhatsky wrote:
> Hello Minchan,
> 
> On (03/28/17 11:50), Minchan Kim wrote:
> [..]
> > > 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.
> > 
> > As I pointed out, at least, dedup was no benefit for the swap case.
> > I don't want to disrupt zsmalloc without any *proved* benefit.
> > Even though it *might* have benefit, it shouldn't be in allocator
> > layer unless it's really huge benefit like performance.
> 
> sure.
> 
> zpool, I meant zpool. I mistakenly used the word 'allocator'.
> 
> I meant some intermediate layer between zram and actual memory allocator,
> a common layer which both zram and zswap can use and which can have
> common functionality. just an idea. haven't really thought about it yet.
> 
> > It makes hard zram's allocator change in future.
> > And please consider zswap is born for the latency in server workload
> > while zram is memory efficiency in embedded world.
> 
> may be. I do suspect zswap is used in embedded as well [1]. there is even
> a brand new allocator that 'reportedly' uses less memory than zsmalloc
> and outperforms zsmalloc in embedded setups [1] (once again, reportedly.
> I haven't tried it).
> 
> if z3fold is actually this good (I'm not saying it is not, haven't
> tested it), then it makes sense to switch to zpool API in zram and let
> zram users to select the allocator that fits their setups better.
> 
> just saying.
> 
> 
> [1] http://events.linuxfoundation.org/sites/events/files/slides/zram1.pdf

I do not want to support multiple allocators in zram.
It's really maintainance headache as well as making zram's goal float.
If new allocator *saves* much memory compared to zsmalloc, it might be
good candidate for replacing zsmalloc.

If so, feel free to send patches with test workload without *any noise*.
Please, do not tell "it's good" with just simple test. What we need is
"why it's good" so that we can investigate what is current problem and
if it is caused by zsmalloc's design so it's hard to change, then
we might think of new allocator seriously.

Anyway, it's off-topic with Joonsoo's patch.