From: Emil Lenngren <emil.lenngren@gmail.com>
To: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Cc: Richard Weinberger <richard@sigma-star.at>,
linux-mtd@lists.infradead.org,
David Oberhollenzer <david.oberhollenzer@sigma-star.at>
Subject: Re: [PATCH v2] mkfs.ubifs: Add ZSTD compression
Date: Fri, 7 Jun 2019 16:20:32 +0200 [thread overview]
Message-ID: <CAO1O6sdeY6ZY_PhoZrVuqCg20F0Dt3Y_mXXr-OYUMD4HZMihvg@mail.gmail.com> (raw)
In-Reply-To: <20190601104322.57inoggnek3crg55@flow>
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2019-06-07 14:20 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-01 10:43 [PATCH v2] mkfs.ubifs: Add ZSTD compression Sebastian Andrzej Siewior
2019-06-02 21:37 ` David Oberhollenzer
2019-06-07 14:20 ` Emil Lenngren [this message]
2019-06-07 20:24 ` Richard Weinberger
2019-06-10 17:34 ` Emil Lenngren
2019-06-11 17:21 ` Sebastian Andrzej Siewior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAO1O6sdeY6ZY_PhoZrVuqCg20F0Dt3Y_mXXr-OYUMD4HZMihvg@mail.gmail.com \
--to=emil.lenngren@gmail.com \
--cc=david.oberhollenzer@sigma-star.at \
--cc=linux-mtd@lists.infradead.org \
--cc=richard@sigma-star.at \
--cc=sebastian@breakpoint.cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.