Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190515210202.21169-1-richard@nod.at>
In-Reply-To: <20190515210202.21169-1-richard@nod.at>
From:   Emil Lenngren <emil.lenngren@gmail.com>
Date:   Fri, 7 Jun 2019 17:34:16 +0200
Message-ID: <CAO1O6sdU=kAYS2sTKwiagxrbg+fMer9nvbwA9C4LoFMgH7e1dQ@mail.gmail.com>
Subject: Re: [PATCH] ubifs: Add support for zstd compression.
To:     Richard Weinberger <richard@nod.at>
Cc:     linux-mtd@lists.infradead.org,
        Sebastian Andrzej Siewior <sebastian@breakpoint.cc>,
        LKML <linux-kernel@vger.kernel.org>,
        Michele Dionisio <michele.dionisio@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello,

Den ons 15 maj 2019 kl 23:03 skrev Richard Weinberger <richard@nod.at>:
>
> From: Michele Dionisio <michele.dionisio@gmail.com>
>
> zstd shows a good compression rate and is faster than lzo,
> also on slow ARM cores.
>
> Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> Signed-off-by: Michele Dionisio <michele.dionisio@gmail.com>
> [rw: rewrote commit message]
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  fs/ubifs/Kconfig       | 10 ++++++++++
>  fs/ubifs/compress.c    | 27 ++++++++++++++++++++++++++-
>  fs/ubifs/super.c       |  2 ++
>  fs/ubifs/ubifs-media.h |  2 ++
>  4 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig
> index 9da2f135121b..8d84d2ed096d 100644
> --- a/fs/ubifs/Kconfig
> +++ b/fs/ubifs/Kconfig
> @@ -5,8 +5,10 @@ config UBIFS_FS
>         select CRYPTO if UBIFS_FS_ADVANCED_COMPR
>         select CRYPTO if UBIFS_FS_LZO
>         select CRYPTO if UBIFS_FS_ZLIB
> +       select CRYPTO if UBIFS_FS_ZSTD
>         select CRYPTO_LZO if UBIFS_FS_LZO
>         select CRYPTO_DEFLATE if UBIFS_FS_ZLIB
> +       select CRYPTO_ZSTD if UBIFS_FS_ZSTD
>         select CRYPTO_HASH_INFO
>         select UBIFS_FS_XATTR if FS_ENCRYPTION
>         depends on MTD_UBI
> @@ -37,6 +39,14 @@ config UBIFS_FS_ZLIB
>         help
>           Zlib compresses better than LZO but it is slower. Say 'Y' if unsure.
>
> +config UBIFS_FS_ZSTD
> +       bool "ZSTD compression support" if UBIFS_FS_ADVANCED_COMPR
> +       depends on UBIFS_FS
> +       default y
> +       help
> +         ZSTD compresses is a big win in speed over Zlib and
> +         in compression ratio over LZO. Say 'Y' if unsure.
> +
>  config UBIFS_ATIME_SUPPORT
>         bool "Access time support"
>         default n
> diff --git a/fs/ubifs/compress.c b/fs/ubifs/compress.c
> index 565cb56d7225..89183aeeeb7a 100644
> --- a/fs/ubifs/compress.c
> +++ b/fs/ubifs/compress.c
> @@ -71,6 +71,24 @@ static struct ubifs_compressor zlib_compr = {
>  };
>  #endif
>
> +#ifdef CONFIG_UBIFS_FS_ZSTD
> +static DEFINE_MUTEX(zstd_enc_mutex);
> +static DEFINE_MUTEX(zstd_dec_mutex);
> +
> +static struct ubifs_compressor zstd_compr = {
> +       .compr_type = UBIFS_COMPR_ZSTD,
> +       .comp_mutex = &zstd_enc_mutex,
> +       .decomp_mutex = &zstd_dec_mutex,
> +       .name = "zstd",
> +       .capi_name = "zstd",
> +};
> +#else
> +static struct ubifs_compressor zstd_compr = {
> +       .compr_type = UBIFS_COMPR_ZSTD,
> +       .name = "zstd",
> +};
> +#endif
> +
>  /* All UBIFS compressors */
>  struct ubifs_compressor *ubifs_compressors[UBIFS_COMPR_TYPES_CNT];
>
> @@ -228,13 +246,19 @@ int __init ubifs_compressors_init(void)
>         if (err)
>                 return err;
>
> -       err = compr_init(&zlib_compr);
> +       err = compr_init(&zstd_compr);
>         if (err)
>                 goto out_lzo;
>
> +       err = compr_init(&zlib_compr);
> +       if (err)
> +               goto out_zstd;
> +
>         ubifs_compressors[UBIFS_COMPR_NONE] = &none_compr;
>         return 0;
>
> +out_zstd:
> +       compr_exit(&zstd_compr);
>  out_lzo:
>         compr_exit(&lzo_compr);
>         return err;
> @@ -247,4 +271,5 @@ void ubifs_compressors_exit(void)
>  {
>         compr_exit(&lzo_compr);
>         compr_exit(&zlib_compr);
> +       compr_exit(&zstd_compr);
>  }
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 04b8ecfd3470..ea8615261936 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1055,6 +1055,8 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
>                                 c->mount_opts.compr_type = UBIFS_COMPR_LZO;
>                         else if (!strcmp(name, "zlib"))
>                                 c->mount_opts.compr_type = UBIFS_COMPR_ZLIB;
> +                       else if (!strcmp(name, "zstd"))
> +                               c->mount_opts.compr_type = UBIFS_COMPR_ZSTD;
>                         else {
>                                 ubifs_err(c, "unknown compressor \"%s\"", name); //FIXME: is c ready?
>                                 kfree(name);
> diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
> index 8b7c1844014f..697b1b89066a 100644
> --- a/fs/ubifs/ubifs-media.h
> +++ b/fs/ubifs/ubifs-media.h
> @@ -348,12 +348,14 @@ enum {
>   * UBIFS_COMPR_NONE: no compression
>   * UBIFS_COMPR_LZO: LZO compression
>   * UBIFS_COMPR_ZLIB: ZLIB compression
> + * UBIFS_COMPR_ZSTD: ZSTD compression
>   * UBIFS_COMPR_TYPES_CNT: count of supported compression types
>   */
>  enum {
>         UBIFS_COMPR_NONE,
>         UBIFS_COMPR_LZO,
>         UBIFS_COMPR_ZLIB,
> +       UBIFS_COMPR_ZSTD,
>         UBIFS_COMPR_TYPES_CNT,
>  };
>
> --
> 2.16.4

In fs/ubifs/sb.c we have

static int get_default_compressor(struct ubifs_info *c)
{
    if (ubifs_compr_present(c, UBIFS_COMPR_LZO))
        return UBIFS_COMPR_LZO;

    if (ubifs_compr_present(c, UBIFS_COMPR_ZLIB))
        return UBIFS_COMPR_ZLIB;

    return UBIFS_COMPR_NONE;
}

Maybe add an entry for zstd here as well?

/Emil