Date: Fri, 25 Aug 2017 14:19:25 +0900
From: Minchan Kim <minchan@kernel.org>
To: Nick Terrell <terrelln@fb.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        "sergey.senozhatsky.work@gmail.com" 
        <sergey.senozhatsky.work@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Yann Collet <cyan@fb.com>
Subject: Re: [PATCH] zram: add zstd to the supported algorithms list
Message-ID: <20170825051925.GB26819@blaptop>
References: <20170824014936.4738-1-sergey.senozhatsky@gmail.com>
 <AA6434F2-6932-436E-B063-B6C0417A553C@contoso.com>
 <20170825004947.GE29701@js1304-P5Q-DELUXE>
 <27EDD68A-61DC-42F1-8422-16B9AB9F0EB3@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <27EDD68A-61DC-42F1-8422-16B9AB9F0EB3@fb.com>
User-Agent: Mutt/1.8.3 (2017-05-23)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2639
Lines: 69

Hi Nick,

On Fri, Aug 25, 2017 at 01:35:35AM +0000, Nick Terrell wrote:
> On 8/24/17, 5:49 PM, "Joonsoo Kim" <iamjoonsoo.kim@lge.com> wrote:
> > On Thu, Aug 24, 2017 at 09:33:54PM +0000, Nick Terrell wrote:
> > > On Thu, Aug 24, 2017 at 10:49:36AM +0900, Sergey Senozhatsky wrote:
> > > > Add ZSTD to the list of supported compression algorithms.
> > > > 
> > > > Official benchmarks [1]:
> > > 
> > > Awesome! Let me know if you need anything from me.
> > > 
> > Hello, Nick.
> > 
> > Awesome work!!!
> > 
> > Let me ask a question.
> > Zram compress and decompress a small data (a page) and your github
> > site says that using predefined dictionary would be helpful in this
> > situation. However, it seems that compression crypto API for zstd
> > doesn't use ZSTD_compress_usingDict(). Is there any plan to support
> > it?
> 
> I think using dictionaries in zram could be very interesting. We could for
> example, take a random sample of the RAM and use that as the dictionary
> for compression. E.g. take 32 512B samples from RAM and build a 16 KB
> dictionary (sizes may vary).

For static option, could we create the dictionary with data in zram
and dump the dictionary into file. And then, rebuiling zram or kernel
includes the dictionary into images.

For it, we would need some knob like

        cat /sys/block/zram/zstd_dict > dict.data

        CONFIG_ZSTD_DICT_DIR=
        CONFIG_ZSTD_DICT_FILE= 

For dynamic option, could we make the dictionary with data
in zram dynamically? So, upcoming pages will use the newly
created dictionary but old compressed pages will use own dictionary.

I'm not sure it's possible, anyway, if predefined dict can help
comp ratio a lot in 4K data, I really love the feature and will support
to have it. ;)

> 
> I'm not sure how you would pass a dictionary into the crypto compression
> API, but I'm sure we can make something work if dictionary compression
> proves to be beneficial enough.

Yes, it would be better to integrate the feature crypto but Please, don't tie to
crypto API. If it's hard to support with current cypto API in short time,
I really want to support it with zcomp_zstd.c.

Please look at old zcomp model.
http://elixir.free-electrons.com/linux/v4.7/source/drivers/block/zram/zcomp_lz4.c

> 
> What data have you, or anyone, used for benchmarking compression ratio and 
> speed for RAM? Since it is such a specialized application, the standard
> compression benchmarks aren't very applicable.

I have used my image dumped from desktop swap device.
Of course, it doesn't cover all of cases in the world but it would be better
to use IO benchmark buffer, IMHO. :)

Thanks.