diff mbox series

[v2] mkfs.ubifs: Add ZSTD compression

Message ID 20190601104322.57inoggnek3crg55@flow
State Accepted
Delegated to: David Oberhollenzer
Headers show
Series [v2] mkfs.ubifs: Add ZSTD compression | expand

Commit Message

Sebastian Andrzej Siewior June 1, 2019, 10:43 a.m. UTC
I added ZSTD support to mkfs.ubifs and compared the ZSTD results with
zlib/lzo and the available ZSTD compression levels. The results are in
the following table:

Comp    image MiB        time  image2 MiB        time
none          271   0m 0,723s         223   0m 0,589s
lzo           164   0m13,705s         116   0m11,636s
zlib          150   0m 7,654s         103   0m 6,347s
favor-lzo     158   0m21,137s         110   0m17,764s
zstd-01       154   0m 1,607s         106   0m 1,429s
zstd-02       153   0m 1,704s         105   0m 1,479s
zstd-03*      152   0m 1,888s         104   0m 1,668s
zstd-04       151   0m 2,741s         103   0m 2,391s
zstd-05       150   0m 3,257s         102   0m 2,916s
zstd-06       150   0m 3,735s         102   0m 3,356s
zstd-07       150   0m 4,066s         102   0m 3,705s
zstd-08       152   0m 1,857s         104   0m 1,644s
zstd-09       152   0m 1,855s         104   0m 1,639s
zstd-10       150   0m 6,654s         102   0m 6,195s
zstd-11       150   0m10,027s         102   0m 9,130s
zstd-12       149   0m14,724s         101   0m13,415s
zstd-13       148   0m18,232s         100   0m16,719s
zstd-14       148   0m20,859s         100   0m19,554s
zstd-15       148   0m25,033s         100   0m23,186s
zstd-16       148   0m38,837s         100   0m36,543s
zstd-17       148   0m46,051s         100   0m43,120s
zstd-18       148   0m49,157s         100   0m45,807s
zstd-19       148   0m49,421s         100   0m45,951s
zstd-20       148   0m51,271s         100   0m48,030s
zstd-21       148   0m51,015s         100   0m48,676s
zstd-22       148   0m52,575s         100   0m50,013s

The UBIFS image was created via
  mkfs.ubifs -x $Comp -m 512 -e 128KiB -c 2200 -r $image $out

I used "debootstrap sid" to create a basic RFS and the results are in
the `image' column. The image2 column denotes the results for the same
image but with .deb files removed.
The time column contains the output of the run time of the command.

ZSTD's compression level three is currently default. Based on the
compression results (for the default level) it outperforms LZO in
run time and compression and is almost as good as ZLIB in terms of
compression but quicker.
The higher compression levels make almost no difference in compression
but take a lot of time.

The compression level used is the default offered by ZSTD. It does not
make sense the higher levels.

Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
v1…v2:	Drop the compression level, there is no need for it.

 Makefile.am                         |  4 +++
 configure.ac                        | 23 +++++++++++++++
 include/mtd/ubifs-media.h           |  2 ++
 ubifs-utils/Makemodule.am           |  4 +--
 ubifs-utils/mkfs.ubifs/compr.c      | 46 ++++++++++++++++++++++++++---
 ubifs-utils/mkfs.ubifs/compr.h      |  1 +
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 12 ++++++--
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.h |  3 ++
 8 files changed, 87 insertions(+), 8 deletions(-)

Comments

David Oberhollenzer June 2, 2019, 9:37 p.m. UTC | #1
Applied to mtd-utils.git master.

Thanks,

David
Emil Lenngren June 7, 2019, 2:20 p.m. UTC | #2
Hi,

Den lör 1 juni 2019 kl 12:43 skrev Sebastian Andrzej Siewior
<sebastian@breakpoint.cc>:
>
> I added ZSTD support to mkfs.ubifs and compared the ZSTD results with
> zlib/lzo and the available ZSTD compression levels. The results are in
> the following table:
>
> Comp    image MiB        time  image2 MiB        time
> none          271   0m 0,723s         223   0m 0,589s
> lzo           164   0m13,705s         116   0m11,636s
> zlib          150   0m 7,654s         103   0m 6,347s
> favor-lzo     158   0m21,137s         110   0m17,764s
> zstd-01       154   0m 1,607s         106   0m 1,429s
> zstd-02       153   0m 1,704s         105   0m 1,479s
> zstd-03*      152   0m 1,888s         104   0m 1,668s
> zstd-04       151   0m 2,741s         103   0m 2,391s
> zstd-05       150   0m 3,257s         102   0m 2,916s
> zstd-06       150   0m 3,735s         102   0m 3,356s
> zstd-07       150   0m 4,066s         102   0m 3,705s
> zstd-08       152   0m 1,857s         104   0m 1,644s
> zstd-09       152   0m 1,855s         104   0m 1,639s
> zstd-10       150   0m 6,654s         102   0m 6,195s
> zstd-11       150   0m10,027s         102   0m 9,130s
> zstd-12       149   0m14,724s         101   0m13,415s
> zstd-13       148   0m18,232s         100   0m16,719s
> zstd-14       148   0m20,859s         100   0m19,554s
> zstd-15       148   0m25,033s         100   0m23,186s
> zstd-16       148   0m38,837s         100   0m36,543s
> zstd-17       148   0m46,051s         100   0m43,120s
> zstd-18       148   0m49,157s         100   0m45,807s
> zstd-19       148   0m49,421s         100   0m45,951s
> zstd-20       148   0m51,271s         100   0m48,030s
> zstd-21       148   0m51,015s         100   0m48,676s
> zstd-22       148   0m52,575s         100   0m50,013s
>
> The UBIFS image was created via
>   mkfs.ubifs -x $Comp -m 512 -e 128KiB -c 2200 -r $image $out
>
> I used "debootstrap sid" to create a basic RFS and the results are in
> the `image' column. The image2 column denotes the results for the same
> image but with .deb files removed.
> The time column contains the output of the run time of the command.
>
> ZSTD's compression level three is currently default. Based on the
> compression results (for the default level) it outperforms LZO in
> run time and compression and is almost as good as ZLIB in terms of
> compression but quicker.
> The higher compression levels make almost no difference in compression
> but take a lot of time.
>
> The compression level used is the default offered by ZSTD. It does not
> make sense the higher levels.
>
> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> ---
> v1…v2:  Drop the compression level, there is no need for it.
>
>  Makefile.am                         |  4 +++
>  configure.ac                        | 23 +++++++++++++++
>  include/mtd/ubifs-media.h           |  2 ++
>  ubifs-utils/Makemodule.am           |  4 +--
>  ubifs-utils/mkfs.ubifs/compr.c      | 46 ++++++++++++++++++++++++++---
>  ubifs-utils/mkfs.ubifs/compr.h      |  1 +
>  ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 12 ++++++--
>  ubifs-utils/mkfs.ubifs/mkfs.ubifs.h |  3 ++
>  8 files changed, 87 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 1bc4684b191df..aacf589771389 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -10,6 +10,10 @@ if WITHOUT_LZO
>  AM_CPPFLAGS += -DWITHOUT_LZO
>  endif
>
> +if WITHOUT_ZSTD
> +AM_CPPFLAGS += -DWITHOUT_ZSTD
> +endif
> +
>  if WITH_SELINUX
>  AM_CPPFLAGS += -DWITH_SELINUX
>  endif
> diff --git a/configure.ac b/configure.ac
> index 1f862ec1d2d74..b64841c2b42e8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -66,6 +66,7 @@ need_pthread="no"
>  need_uuid="no"
>  need_zlib="no"
>  need_lzo="no"
> +need_zstd="no"
>  need_xattr="no"
>  need_cmocka="no"
>  need_selinux="no"
> @@ -138,6 +139,7 @@ AM_COND_IF([BUILD_UBIFS], [
>         need_xattr="yes"
>         need_zlib="yes"
>         need_lzo="yes"
> +       need_zstd="yes"
>         need_openssl="yes"
>  ])
>
> @@ -164,6 +166,14 @@ AC_ARG_WITH([lzo],
>         *) AC_MSG_ERROR([bad value ${withval} for --without-lzo]) ;;
>         esac])
>
> +AC_ARG_WITH([zstd],
> +       [AS_HELP_STRING([--without-zstd], [Disable support for ZSTD compression])],
> +       [case "${withval}" in
> +       yes) ;;
> +       no) need_zstd="no" ;;
> +       *) AC_MSG_ERROR([bad value ${withval} for --without-zstd]) ;;
> +       esac])
> +
>  AC_ARG_WITH([selinux],
>         [AS_HELP_STRING([--with-selinux],
>                 [Enable support for selinux extended attributes])],
> @@ -189,6 +199,7 @@ pthread_missing="no"
>  uuid_missing="no"
>  zlib_missing="no"
>  lzo_missing="no"
> +zstd_missing="no"
>  xattr_missing="no"
>  cmocka_missing="no"
>  selinux_missing="no"
> @@ -225,6 +236,10 @@ if test "x$need_lzo" = "xyes"; then
>         )
>  fi
>
> +if test "x$need_zstd" = "xyes"; then
> +       PKG_CHECK_MODULES([ZSTD], [libzstd],, zstd_missing="yes")
> +fi
> +
>  if test "x$need_xattr" = "xyes"; then
>         AC_CHECK_HEADERS([sys/xattr.h], [], [xattr_missing="yes"])
>         AC_CHECK_HEADERS([sys/acl.h], [], [xattr_missing="yes"])
> @@ -283,6 +298,13 @@ if test "x$lzo_missing" = "xyes"; then
>         dep_missing="yes"
>  fi
>
> +if test "x$zstd_missing" = "xyes"; then
> +       AC_MSG_WARN([cannot find ZSTD library required for mkfs program])
> +       AC_MSG_NOTICE([mtd-utils can optionally be built without mkfs.ubifs])
> +       AC_MSG_NOTICE([mtd-utils can optionally be built without ZSTD support])
> +       dep_missing="yes"
> +fi
> +
>  if test "x$xattr_missing" = "xyes"; then
>         AC_MSG_WARN([cannot find headers for extended attributes])
>         AC_MSG_WARN([disabling XATTR support])
> @@ -314,6 +336,7 @@ fi
>  ##### generate output #####
>
>  AM_CONDITIONAL([WITHOUT_LZO], [test "x$need_lzo" != "xyes"])
> +AM_CONDITIONAL([WITHOUT_ZSTD], [test "x$need_zstd" != "xyes"])
>  AM_CONDITIONAL([WITHOUT_XATTR], [test "x$need_xattr" != "xyes"])
>  AM_CONDITIONAL([WITH_SELINUX], [test "x$need_selinux" == "xyes"])
>  AM_CONDITIONAL([WITH_CRYPTO], [test "x$need_openssl" == "xyes"])
> diff --git a/include/mtd/ubifs-media.h b/include/mtd/ubifs-media.h
> index e69ba1687134e..7751ac72c2288 100644
> --- a/include/mtd/ubifs-media.h
> +++ b/include/mtd/ubifs-media.h
> @@ -343,12 +343,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,
>  };
>
> diff --git a/ubifs-utils/Makemodule.am b/ubifs-utils/Makemodule.am
> index b8e4075c9d2ae..164ce09cef586 100644
> --- a/ubifs-utils/Makemodule.am
> +++ b/ubifs-utils/Makemodule.am
> @@ -22,8 +22,8 @@ mkfs_ubifs_SOURCES += ubifs-utils/mkfs.ubifs/crypto.c \
>                 ubifs-utils/mkfs.ubifs/fscrypt.c
>  endif
>
> -mkfs_ubifs_LDADD = libmtd.a libubi.a $(ZLIB_LIBS) $(LZO_LIBS) $(UUID_LIBS) $(LIBSELINUX_LIBS) $(OPENSSL_LIBS) -lm
> -mkfs_ubifs_CPPFLAGS = $(AM_CPPFLAGS) $(ZLIB_CFLAGS) $(LZO_CFLAGS) $(UUID_CFLAGS) $(LIBSELINUX_CFLAGS)\
> +mkfs_ubifs_LDADD = libmtd.a libubi.a $(ZLIB_LIBS) $(LZO_LIBS) $(ZSTD_LIBS) $(UUID_LIBS) $(LIBSELINUX_LIBS) $(OPENSSL_LIBS) -lm
> +mkfs_ubifs_CPPFLAGS = $(AM_CPPFLAGS) $(ZLIB_CFLAGS) $(LZO_CFLAGS) $(ZSTD_CFLAGS) $(UUID_CFLAGS) $(LIBSELINUX_CFLAGS)\
>         -I$(top_srcdir)/ubi-utils/include -I$(top_srcdir)/ubifs-utils/mkfs.ubifs/
>
>  UBIFS_BINS = \
> diff --git a/ubifs-utils/mkfs.ubifs/compr.c b/ubifs-utils/mkfs.ubifs/compr.c
> index 8eff1865d96ef..62398357c5351 100644
> --- a/ubifs-utils/mkfs.ubifs/compr.c
> +++ b/ubifs-utils/mkfs.ubifs/compr.c
> @@ -28,6 +28,9 @@
>  #include <lzo/lzo1x.h>
>  #endif
>  #include <linux/types.h>
> +#ifndef WITHOUT_ZSTSD
> +#include <zstd.h>
> +#endif
>
>  #define crc32 __zlib_crc32
>  #include <zlib.h>
> @@ -109,6 +112,25 @@ static int lzo_compress(void *in_buf, size_t in_len, void *out_buf,
>  }
>  #endif
>
> +#ifndef WITHOUT_ZSTD
> +static ZSTD_CCtx *zctx;
> +
> +static int zstd_compress(void *in_buf, size_t in_len, void *out_buf,
> +                        size_t *out_len)
> +{
> +       size_t ret;
> +
> +       ret = ZSTD_compressCCtx(zctx, out_buf, *out_len, in_buf, in_len,
> +                               ZSTD_CLEVEL_DEFAULT);
> +       if (ZSTD_isError(ret)) {
> +               errcnt += 1;
> +               return -1;
> +       }
> +       *out_len = ret;
> +       return 0;
> +}
> +#endif
> +
>  static int no_compress(void *in_buf, size_t in_len, void *out_buf,
>                        size_t *out_len)
>  {
> @@ -192,6 +214,11 @@ int compress_data(void *in_buf, size_t in_len, void *out_buf, size_t *out_len,
>                 case MKFS_UBIFS_COMPR_ZLIB:
>                         ret = zlib_deflate(in_buf, in_len, out_buf, out_len);
>                         break;
> +#ifndef WITHOUT_ZSTD
> +               case MKFS_UBIFS_COMPR_ZSTD:
> +                       ret = zstd_compress(in_buf, in_len, out_buf, out_len);
> +                       break;
> +#endif
>                 case MKFS_UBIFS_COMPR_NONE:
>                         ret = 1;
>                         break;
> @@ -219,18 +246,29 @@ int init_compression(void)
>  #endif
>
>         zlib_buf = malloc(UBIFS_BLOCK_SIZE * WORST_COMPR_FACTOR);
> -       if (!zlib_buf) {
> -               free(lzo_mem);
> -               return -1;
> -       }
> +       if (!zlib_buf)
> +               goto err;
> +
> +#ifndef WITHOUT_ZSTD
> +       zctx = ZSTD_createCCtx();
> +       if (!zctx)
> +               goto err;
> +#endif
>
>         return 0;
> +err:
> +       free(zlib_buf);
> +       free(lzo_mem);
> +       return -1;
>  }
>
>  void destroy_compression(void)
>  {
>         free(zlib_buf);
>         free(lzo_mem);
> +#ifndef WITHOUT_ZSTD
> +       ZSTD_freeCCtx(zctx);
> +#endif
>         if (errcnt)
>                 fprintf(stderr, "%llu compression errors occurred\n", errcnt);
>  }
> diff --git a/ubifs-utils/mkfs.ubifs/compr.h b/ubifs-utils/mkfs.ubifs/compr.h
> index e3dd95ce1487b..d58c7c7bd313f 100644
> --- a/ubifs-utils/mkfs.ubifs/compr.h
> +++ b/ubifs-utils/mkfs.ubifs/compr.h
> @@ -36,6 +36,7 @@ enum compression_type
>         MKFS_UBIFS_COMPR_NONE,
>         MKFS_UBIFS_COMPR_LZO,
>         MKFS_UBIFS_COMPR_ZLIB,
> +       MKFS_UBIFS_COMPR_ZSTD,
>  };
>
>  int compress_data(void *in_buf, size_t in_len, void *out_buf, size_t *out_len,
> diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> index 4b31979d9d2c2..6d8e55dc32c07 100644
> --- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> +++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> @@ -35,6 +35,10 @@
>  #include <selinux/label.h>
>  #endif
>
> +#ifndef WITHOUT_ZSTD
> +#include <zstd.h>
> +#endif
> +
>  #include "crypto.h"
>  #include "fscrypt.h"
>
> @@ -209,8 +213,8 @@ static const char *helptext =
>  "-o, --output=FILE        output to FILE\n"
>  "-j, --jrn-size=SIZE      journal size\n"
>  "-R, --reserved=SIZE      how much space should be reserved for the super-user\n"
> -"-x, --compr=TYPE         compression type - \"lzo\", \"favor_lzo\", \"zlib\" or\n"
> -"                         \"none\" (default: \"lzo\")\n"
> +"-x, --compr=TYPE         compression type - \"lzo\", \"favor_lzo\", \"zlib\"\n"
> +"                         \"zstd\" or \"none\" (default: \"lzo\")\n"
>  "-X, --favor-percent      may only be used with favor LZO compression and defines\n"
>  "                         how many percent better zlib should compress to make\n"
>  "                         mkfs.ubifs use zlib instead of LZO (default 20%)\n"
> @@ -654,6 +658,10 @@ static int get_options(int argc, char**argv)
>                                 c->default_compr = UBIFS_COMPR_NONE;
>                         else if (strcmp(optarg, "zlib") == 0)
>                                 c->default_compr = UBIFS_COMPR_ZLIB;
> +#ifndef WITHOUT_ZSTD
> +                       else if (strcmp(optarg, "zstd") == 0)
> +                               c->default_compr = UBIFS_COMPR_ZSTD;
> +#endif
>  #ifndef WITHOUT_LZO
>                         else if (strcmp(optarg, "favor_lzo") == 0) {
>                                 c->default_compr = UBIFS_COMPR_LZO;
> diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h
> index 8f0186043079c..f1425c5af31a8 100644
> --- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h
> +++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h
> @@ -77,6 +77,9 @@
>  #if MKFS_UBIFS_COMPR_ZLIB != UBIFS_COMPR_ZLIB
>  #error MKFS_UBIFS_COMPR_ZLIB != UBIFS_COMPR_ZLIB
>  #endif
> +#if MKFS_UBIFS_COMPR_ZSTD != UBIFS_COMPR_ZSTD
> +#error MKFS_UBIFS_COMPR_ZSTD != UBIFS_COMPR_ZSTD
> +#endif
>
>  extern int verbose;
>  extern int debug_level;
> --
> 2.20.1
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

The new mtd-utils with this patch doesn't compile on Ubuntu 18.04 LTS
since it uses a slightly older version of zstd (1.3.3) that didn't
have the macro ZSTD_CLEVEL_DEFAULT defined, which was introduced in
version 1.3.5. Could you maybe consider adding the following lines in
compr.c:

#ifndef ZSTD_CLEVEL_DEFAULT
#define ZSTD_CLEVEL_DEFAULT 3
#endif

or similar, which make it compile on slightly older distributions?

/Emil
Richard Weinberger June 7, 2019, 8:24 p.m. UTC | #3
Emil,

----- Ursprüngliche Mail -----
> The new mtd-utils with this patch doesn't compile on Ubuntu 18.04 LTS
> since it uses a slightly older version of zstd (1.3.3) that didn't
> have the macro ZSTD_CLEVEL_DEFAULT defined, which was introduced in
> version 1.3.5. Could you maybe consider adding the following lines in
> compr.c:
> 
> #ifndef ZSTD_CLEVEL_DEFAULT
> #define ZSTD_CLEVEL_DEFAULT 3
> #endif
> 
> or similar, which make it compile on slightly older distributions?

If I read the docs correctly, we can just pass 0 as compression level,
which will default to ZSTD_CLEVEL_DEFAULT.
https://github.com/facebook/zstd/pull/1174/commits/e34c000e44444b9f8bd62e5af0a355ee186eb21f

This should work on all zstd versions.

Thanks,
//richard
Emil Lenngren June 10, 2019, 5:34 p.m. UTC | #4
Hi all,

Den fre 7 juni 2019 kl 22:24 skrev Richard Weinberger <richard@sigma-star.at>:
>
> Emil,
>
> ----- Ursprüngliche Mail -----
> > The new mtd-utils with this patch doesn't compile on Ubuntu 18.04 LTS
> > since it uses a slightly older version of zstd (1.3.3) that didn't
> > have the macro ZSTD_CLEVEL_DEFAULT defined, which was introduced in
> > version 1.3.5. Could you maybe consider adding the following lines in
> > compr.c:
> >
> > #ifndef ZSTD_CLEVEL_DEFAULT
> > #define ZSTD_CLEVEL_DEFAULT 3
> > #endif
> >
> > or similar, which make it compile on slightly older distributions?
>
> If I read the docs correctly, we can just pass 0 as compression level,
> which will default to ZSTD_CLEVEL_DEFAULT.
> https://github.com/facebook/zstd/pull/1174/commits/e34c000e44444b9f8bd62e5af0a355ee186eb21f
>
> This should work on all zstd versions.

After some more investigations, although increasing compression level
certainly increases compression time, decompression time does not seem
to be increased by increasing compression level. See
http://www.open-zfs.org/w/images/b/b3/03-OpenZFS_2017_-_ZStandard_in_ZFS.pdf
page 9 for a benchmark. The benchmark even shows this seems to apply
to gz as well...

SquashFS has also added support for zstd and squashfs-tools uses level
15 as the default level (see
https://git.kernel.org/pub/scm/fs/squashfs/squashfs-tools.git/commit/?id=e38956b92f738518c29734399629e7cdb33072d3
at the bottom).

While the kernel compression level should maybe stay at 3, for
mkfs.ubifs where speed doesn't matter that much, a higher level such
as 15 might not be bad after all. So I have two different proposals:
either just set level 15 OR set level 15 and also provide an option
for mkfs.ubifs to override it if one for some reason wants to generate
the image faster.

What do you think?

/Emil
Sebastian Andrzej Siewior June 11, 2019, 5:21 p.m. UTC | #5
On 2019-06-10 19:34:53 [+0200], Emil Lenngren wrote:
> Hi all,
Hi,

> After some more investigations, although increasing compression level
> certainly increases compression time, decompression time does not seem
> to be increased by increasing compression level. See
> http://www.open-zfs.org/w/images/b/b3/03-OpenZFS_2017_-_ZStandard_in_ZFS.pdf
> page 9 for a benchmark. The benchmark even shows this seems to apply
> to gz as well...
> 
> SquashFS has also added support for zstd and squashfs-tools uses level
> 15 as the default level (see
> https://git.kernel.org/pub/scm/fs/squashfs/squashfs-tools.git/commit/?id=e38956b92f738518c29734399629e7cdb33072d3
> at the bottom).
> 
> While the kernel compression level should maybe stay at 3, for
> mkfs.ubifs where speed doesn't matter that much, a higher level such
> as 15 might not be bad after all. So I have two different proposals:
> either just set level 15 OR set level 15 and also provide an option
> for mkfs.ubifs to override it if one for some reason wants to generate
> the image faster.
> 
> What do you think?

So the original problem was that ZSTD_CLEVEL_DEFAULT is not defined in
earlier releases of zstd and I would suggest to use `0' as stated here
in this thread.
Larger levels take more time to compress and I don't see the benefit in
doing compared to the size of the compressed image. I included my
numbers in the initial commit.
Also, if you use a larger compression level in mkfs compared to the
kernel then your image will grow if you rewrite existing files.

I've been told that for RO images (where higher compression levels
matter and compression is done once at creation time) the best thing to
do is to use squashfs on top of ubi.

> /Emil

Sebastian
diff mbox series

Patch

diff --git a/Makefile.am b/Makefile.am
index 1bc4684b191df..aacf589771389 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -10,6 +10,10 @@  if WITHOUT_LZO
 AM_CPPFLAGS += -DWITHOUT_LZO
 endif
 
+if WITHOUT_ZSTD
+AM_CPPFLAGS += -DWITHOUT_ZSTD
+endif
+
 if WITH_SELINUX
 AM_CPPFLAGS += -DWITH_SELINUX
 endif
diff --git a/configure.ac b/configure.ac
index 1f862ec1d2d74..b64841c2b42e8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -66,6 +66,7 @@  need_pthread="no"
 need_uuid="no"
 need_zlib="no"
 need_lzo="no"
+need_zstd="no"
 need_xattr="no"
 need_cmocka="no"
 need_selinux="no"
@@ -138,6 +139,7 @@  AM_COND_IF([BUILD_UBIFS], [
 	need_xattr="yes"
 	need_zlib="yes"
 	need_lzo="yes"
+	need_zstd="yes"
 	need_openssl="yes"
 ])
 
@@ -164,6 +166,14 @@  AC_ARG_WITH([lzo],
 	*) AC_MSG_ERROR([bad value ${withval} for --without-lzo]) ;;
 	esac])
 
+AC_ARG_WITH([zstd],
+	[AS_HELP_STRING([--without-zstd], [Disable support for ZSTD compression])],
+	[case "${withval}" in
+	yes) ;;
+	no) need_zstd="no" ;;
+	*) AC_MSG_ERROR([bad value ${withval} for --without-zstd]) ;;
+	esac])
+
 AC_ARG_WITH([selinux],
 	[AS_HELP_STRING([--with-selinux],
 		[Enable support for selinux extended attributes])],
@@ -189,6 +199,7 @@  pthread_missing="no"
 uuid_missing="no"
 zlib_missing="no"
 lzo_missing="no"
+zstd_missing="no"
 xattr_missing="no"
 cmocka_missing="no"
 selinux_missing="no"
@@ -225,6 +236,10 @@  if test "x$need_lzo" = "xyes"; then
 	)
 fi
 
+if test "x$need_zstd" = "xyes"; then
+	PKG_CHECK_MODULES([ZSTD], [libzstd],, zstd_missing="yes")
+fi
+
 if test "x$need_xattr" = "xyes"; then
 	AC_CHECK_HEADERS([sys/xattr.h], [], [xattr_missing="yes"])
 	AC_CHECK_HEADERS([sys/acl.h], [], [xattr_missing="yes"])
@@ -283,6 +298,13 @@  if test "x$lzo_missing" = "xyes"; then
 	dep_missing="yes"
 fi
 
+if test "x$zstd_missing" = "xyes"; then
+	AC_MSG_WARN([cannot find ZSTD library required for mkfs program])
+	AC_MSG_NOTICE([mtd-utils can optionally be built without mkfs.ubifs])
+	AC_MSG_NOTICE([mtd-utils can optionally be built without ZSTD support])
+	dep_missing="yes"
+fi
+
 if test "x$xattr_missing" = "xyes"; then
 	AC_MSG_WARN([cannot find headers for extended attributes])
 	AC_MSG_WARN([disabling XATTR support])
@@ -314,6 +336,7 @@  fi
 ##### generate output #####
 
 AM_CONDITIONAL([WITHOUT_LZO], [test "x$need_lzo" != "xyes"])
+AM_CONDITIONAL([WITHOUT_ZSTD], [test "x$need_zstd" != "xyes"])
 AM_CONDITIONAL([WITHOUT_XATTR], [test "x$need_xattr" != "xyes"])
 AM_CONDITIONAL([WITH_SELINUX], [test "x$need_selinux" == "xyes"])
 AM_CONDITIONAL([WITH_CRYPTO], [test "x$need_openssl" == "xyes"])
diff --git a/include/mtd/ubifs-media.h b/include/mtd/ubifs-media.h
index e69ba1687134e..7751ac72c2288 100644
--- a/include/mtd/ubifs-media.h
+++ b/include/mtd/ubifs-media.h
@@ -343,12 +343,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,
 };
 
diff --git a/ubifs-utils/Makemodule.am b/ubifs-utils/Makemodule.am
index b8e4075c9d2ae..164ce09cef586 100644
--- a/ubifs-utils/Makemodule.am
+++ b/ubifs-utils/Makemodule.am
@@ -22,8 +22,8 @@  mkfs_ubifs_SOURCES += ubifs-utils/mkfs.ubifs/crypto.c \
 		ubifs-utils/mkfs.ubifs/fscrypt.c
 endif
 
-mkfs_ubifs_LDADD = libmtd.a libubi.a $(ZLIB_LIBS) $(LZO_LIBS) $(UUID_LIBS) $(LIBSELINUX_LIBS) $(OPENSSL_LIBS) -lm
-mkfs_ubifs_CPPFLAGS = $(AM_CPPFLAGS) $(ZLIB_CFLAGS) $(LZO_CFLAGS) $(UUID_CFLAGS) $(LIBSELINUX_CFLAGS)\
+mkfs_ubifs_LDADD = libmtd.a libubi.a $(ZLIB_LIBS) $(LZO_LIBS) $(ZSTD_LIBS) $(UUID_LIBS) $(LIBSELINUX_LIBS) $(OPENSSL_LIBS) -lm
+mkfs_ubifs_CPPFLAGS = $(AM_CPPFLAGS) $(ZLIB_CFLAGS) $(LZO_CFLAGS) $(ZSTD_CFLAGS) $(UUID_CFLAGS) $(LIBSELINUX_CFLAGS)\
 	-I$(top_srcdir)/ubi-utils/include -I$(top_srcdir)/ubifs-utils/mkfs.ubifs/
 
 UBIFS_BINS = \
diff --git a/ubifs-utils/mkfs.ubifs/compr.c b/ubifs-utils/mkfs.ubifs/compr.c
index 8eff1865d96ef..62398357c5351 100644
--- a/ubifs-utils/mkfs.ubifs/compr.c
+++ b/ubifs-utils/mkfs.ubifs/compr.c
@@ -28,6 +28,9 @@ 
 #include <lzo/lzo1x.h>
 #endif
 #include <linux/types.h>
+#ifndef WITHOUT_ZSTSD
+#include <zstd.h>
+#endif
 
 #define crc32 __zlib_crc32
 #include <zlib.h>
@@ -109,6 +112,25 @@  static int lzo_compress(void *in_buf, size_t in_len, void *out_buf,
 }
 #endif
 
+#ifndef WITHOUT_ZSTD
+static ZSTD_CCtx *zctx;
+
+static int zstd_compress(void *in_buf, size_t in_len, void *out_buf,
+			 size_t *out_len)
+{
+	size_t ret;
+
+	ret = ZSTD_compressCCtx(zctx, out_buf, *out_len, in_buf, in_len,
+				ZSTD_CLEVEL_DEFAULT);
+	if (ZSTD_isError(ret)) {
+		errcnt += 1;
+		return -1;
+	}
+	*out_len = ret;
+	return 0;
+}
+#endif
+
 static int no_compress(void *in_buf, size_t in_len, void *out_buf,
 		       size_t *out_len)
 {
@@ -192,6 +214,11 @@  int compress_data(void *in_buf, size_t in_len, void *out_buf, size_t *out_len,
 		case MKFS_UBIFS_COMPR_ZLIB:
 			ret = zlib_deflate(in_buf, in_len, out_buf, out_len);
 			break;
+#ifndef WITHOUT_ZSTD
+		case MKFS_UBIFS_COMPR_ZSTD:
+			ret = zstd_compress(in_buf, in_len, out_buf, out_len);
+			break;
+#endif
 		case MKFS_UBIFS_COMPR_NONE:
 			ret = 1;
 			break;
@@ -219,18 +246,29 @@  int init_compression(void)
 #endif
 
 	zlib_buf = malloc(UBIFS_BLOCK_SIZE * WORST_COMPR_FACTOR);
-	if (!zlib_buf) {
-		free(lzo_mem);
-		return -1;
-	}
+	if (!zlib_buf)
+		goto err;
+
+#ifndef WITHOUT_ZSTD
+	zctx = ZSTD_createCCtx();
+	if (!zctx)
+		goto err;
+#endif
 
 	return 0;
+err:
+	free(zlib_buf);
+	free(lzo_mem);
+	return -1;
 }
 
 void destroy_compression(void)
 {
 	free(zlib_buf);
 	free(lzo_mem);
+#ifndef WITHOUT_ZSTD
+	ZSTD_freeCCtx(zctx);
+#endif
 	if (errcnt)
 		fprintf(stderr, "%llu compression errors occurred\n", errcnt);
 }
diff --git a/ubifs-utils/mkfs.ubifs/compr.h b/ubifs-utils/mkfs.ubifs/compr.h
index e3dd95ce1487b..d58c7c7bd313f 100644
--- a/ubifs-utils/mkfs.ubifs/compr.h
+++ b/ubifs-utils/mkfs.ubifs/compr.h
@@ -36,6 +36,7 @@  enum compression_type
 	MKFS_UBIFS_COMPR_NONE,
 	MKFS_UBIFS_COMPR_LZO,
 	MKFS_UBIFS_COMPR_ZLIB,
+	MKFS_UBIFS_COMPR_ZSTD,
 };
 
 int compress_data(void *in_buf, size_t in_len, void *out_buf, size_t *out_len,
diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index 4b31979d9d2c2..6d8e55dc32c07 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -35,6 +35,10 @@ 
 #include <selinux/label.h>
 #endif
 
+#ifndef WITHOUT_ZSTD
+#include <zstd.h>
+#endif
+
 #include "crypto.h"
 #include "fscrypt.h"
 
@@ -209,8 +213,8 @@  static const char *helptext =
 "-o, --output=FILE        output to FILE\n"
 "-j, --jrn-size=SIZE      journal size\n"
 "-R, --reserved=SIZE      how much space should be reserved for the super-user\n"
-"-x, --compr=TYPE         compression type - \"lzo\", \"favor_lzo\", \"zlib\" or\n"
-"                         \"none\" (default: \"lzo\")\n"
+"-x, --compr=TYPE         compression type - \"lzo\", \"favor_lzo\", \"zlib\"\n"
+"                         \"zstd\" or \"none\" (default: \"lzo\")\n"
 "-X, --favor-percent      may only be used with favor LZO compression and defines\n"
 "                         how many percent better zlib should compress to make\n"
 "                         mkfs.ubifs use zlib instead of LZO (default 20%)\n"
@@ -654,6 +658,10 @@  static int get_options(int argc, char**argv)
 				c->default_compr = UBIFS_COMPR_NONE;
 			else if (strcmp(optarg, "zlib") == 0)
 				c->default_compr = UBIFS_COMPR_ZLIB;
+#ifndef WITHOUT_ZSTD
+			else if (strcmp(optarg, "zstd") == 0)
+				c->default_compr = UBIFS_COMPR_ZSTD;
+#endif
 #ifndef WITHOUT_LZO
 			else if (strcmp(optarg, "favor_lzo") == 0) {
 				c->default_compr = UBIFS_COMPR_LZO;
diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h
index 8f0186043079c..f1425c5af31a8 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.h
@@ -77,6 +77,9 @@ 
 #if MKFS_UBIFS_COMPR_ZLIB != UBIFS_COMPR_ZLIB
 #error MKFS_UBIFS_COMPR_ZLIB != UBIFS_COMPR_ZLIB
 #endif
+#if MKFS_UBIFS_COMPR_ZSTD != UBIFS_COMPR_ZSTD
+#error MKFS_UBIFS_COMPR_ZSTD != UBIFS_COMPR_ZSTD
+#endif
 
 extern int verbose;
 extern int debug_level;