| Message ID | 20190515210202.21169-1-richard@nod.at |
|---|---|
| State | Accepted |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | ubifs: Add support for zstd compression. | expand |
On 2019-05-15 23:02:02 [+0200], Richard Weinberger wrote: That is a lot less compared to what I needed for mkfs.ubifs. I just tested this in nandsim and it works - I can read the image generated by mkfs. As for my compression level testing: Do we want to keep -L parameter or rather drop it? The max level hardly makes a difference (and we don't have it / use it for LZO/ZLIB)… Sebastian
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
Emil, ----- Ursprüngliche Mail ----- > 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? Where would you add it? If we add it after UBIFS_COMPR_ZLIB, it will effectively never get selected, unless someone builds a kernel without lzo and zlib but zstd. If we add it before UBIFS_COMPR_ZLIB, it will be used always and users end up with unreadable files if they reboot to an older kernel. Please note that we didn't raise the UBIFS format version for zstd. So I'm not sure what is the best choice for the default filesystem. Thanks, //richard
Hi, Den fre 7 juni 2019 kl 22:09 skrev Richard Weinberger <richard@nod.at>: > > Emil, > > ----- Ursprüngliche Mail ----- > > 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? > > Where would you add it? If we add it after UBIFS_COMPR_ZLIB, > it will effectively never get selected, unless someone builds a kernel > without lzo and zlib but zstd. > If we add it before UBIFS_COMPR_ZLIB, it will be used always and users > end up with unreadable files if they reboot to an older kernel. > Please note that we didn't raise the UBIFS format version for zstd. > > So I'm not sure what is the best choice for the default filesystem. My idea was at the end, i.e. it will only be used when LZO and ZLIB are not selected to be included for UBIFS, for example when someone compiles a minimal kernel who knows exactly which compression algorithms will be used on that system. I guess that's the same reason why zlib exists at all and is placed after lzo. But of course the other positions also have their pros. If we perform more benchmarks speed/size and conclude that zstd outperforms zlib for all aspects, then maybe placing it between lzo and zlib could be an option, but as you say we should avoid breaking compatibility with older systems. I did a single test today and compared lzo and zstd and on that test lzo had faster decompression speed but resulted in larger space. I'll do more tests later. /Emil
----- Ursprüngliche Mail ----- > Von: "Emil Lenngren" <emil.lenngren@gmail.com> > An: "richard" <richard@nod.at> > CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "Sebastian Andrzej Siewior" <sebastian@breakpoint.cc>, "linux-kernel" > <linux-kernel@vger.kernel.org>, "Michele Dionisio" <michele.dionisio@gmail.com> > Gesendet: Freitag, 7. Juni 2019 22:27:09 > Betreff: Re: [PATCH] ubifs: Add support for zstd compression. >> So I'm not sure what is the best choice for the default filesystem. > > My idea was at the end, i.e. it will only be used when LZO and ZLIB > are not selected to be included for UBIFS, for example when someone > compiles a minimal kernel who knows exactly which compression > algorithms will be used on that system. BTW: you can always select the compressor using the compr= mount option. Also for the default filesystem. Putting it at the end does not harm but IMHO the use is little. But for the sake of completes, I agree with you. Can you send a follow-up patch? > I did a single test today and compared lzo and zstd and on that test > lzo had faster decompression speed but resulted in larger space. I'll > do more tests later. Can you please share more details? I'm interested what CPU this was. Thanks, //richard
Hi, Den fre 7 juni 2019 kl 22:49 skrev Richard Weinberger <richard@nod.at>: > > ----- Ursprüngliche Mail ----- > > Von: "Emil Lenngren" <emil.lenngren@gmail.com> > > An: "richard" <richard@nod.at> > > CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "Sebastian Andrzej Siewior" <sebastian@breakpoint.cc>, "linux-kernel" > > <linux-kernel@vger.kernel.org>, "Michele Dionisio" <michele.dionisio@gmail.com> > > Gesendet: Freitag, 7. Juni 2019 22:27:09 > > Betreff: Re: [PATCH] ubifs: Add support for zstd compression. > >> So I'm not sure what is the best choice for the default filesystem. > > > > My idea was at the end, i.e. it will only be used when LZO and ZLIB > > are not selected to be included for UBIFS, for example when someone > > compiles a minimal kernel who knows exactly which compression > > algorithms will be used on that system. > > BTW: you can always select the compressor using the compr= mount option. > Also for the default filesystem. Yep that's what I'm using while I'm testing. > Putting it at the end does not harm but IMHO the use is little. > But for the sake of completes, I agree with you. Can you send a follow-up > patch? Ok > > > I did a single test today and compared lzo and zstd and on that test > > lzo had faster decompression speed but resulted in larger space. I'll > > do more tests later. > > Can you please share more details? I'm interested what CPU this was. ARM Cortex-A7. The kernel is compiled with gcc 7.3.1. Next week I'll test some more. I have a question about how the decompression is done while reading. When a large file is read from the filesystem (assuming not in any cache), is it the case that first a chunk is read from flash, that chunk is then decompressed, later next chunk is read from flash, that one is then decompressed and so on, or can the decompression be done in parallel while reading the next chunk from flash? /Emil
----- Ursprüngliche Mail ----- > ARM Cortex-A7. The kernel is compiled with gcc 7.3.1. Next week I'll > test some more. Good to know! > I have a question about how the decompression is done while reading. > When a large file is read from the filesystem (assuming not in any > cache), is it the case that first a chunk is read from flash, that > chunk is then decompressed, later next chunk is read from flash, that > one is then decompressed and so on, or can the decompression be done > in parallel while reading the next chunk from flash? No, this is a serial operation. Thanks, //richard
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, };