* lz4hc compression in UBIFS? @ 2013-09-20 12:16 Konstantin Tokarev 2013-10-04 3:09 ` Brent Taylor 0 siblings, 1 reply; 16+ messages in thread From: Konstantin Tokarev @ 2013-09-20 12:16 UTC (permalink / raw) To: linux-mtd Hi all, Are there any plans for adding lz4hc [1] compression support to UBIFS? Benchmarks are quite impressive, and it looks like lz4hc can achieve better compression ration while providing 3.5x faster decompression. From quick look at UBIFS module sources it looks like adding new compressor supported by crypto API is a matter of a few lines of code in compress.c. Or are there any known obstacles? [1] http://code.google.com/p/lz4/ -- Regards, Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-09-20 12:16 lz4hc compression in UBIFS? Konstantin Tokarev @ 2013-10-04 3:09 ` Brent Taylor 2013-10-04 7:44 ` Artem Bityutskiy ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: Brent Taylor @ 2013-10-04 3:09 UTC (permalink / raw) To: Konstantin Tokarev; +Cc: linux-mtd@lists.infradead.org, Artem Bityutskiy On Fri, Sep 20, 2013 at 7:16 AM, Konstantin Tokarev <annulen@yandex.ru> wrote: > Hi all, > > Are there any plans for adding lz4hc [1] compression support to UBIFS? > > Benchmarks are quite impressive, and it looks like lz4hc can achieve better compression ration while providing 3.5x faster decompression. From quick look at UBIFS module sources it looks like adding new compressor supported by crypto API is a matter of a few lines of code in compress.c. > > Or are there any known obstacles? > > [1] http://code.google.com/p/lz4/ > > -- > Regards, > Konstantin > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ Here is a patch based on linux-3.12-rc3. I haven't performed any performance testing UBIFS using lz4hc, but I can mount UBIFS volumes and haven't seen any problems yet. The only think I know that isn't correct about the patch is the description for the Kconfig element for select lz4hc as a compression option. I only copied the description from the lzo description. diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/compress.c linux-3.12-rc3/fs/ubifs/compress.c --- linux-3.12-rc3.orig/fs/ubifs/compress.c 2013-09-29 17:02:38.000000000 -0500 +++ linux-3.12-rc3/fs/ubifs/compress.c 2013-07-17 21:57:27.440653860 -0500 @@ -53,6 +53,22 @@ }; #endif +#ifdef CONFIG_UBIFS_FS_LZ4HC +static DEFINE_MUTEX(lz4hc_mutex); + +static struct ubifs_compressor lz4hc_compr = { + .compr_type = UBIFS_COMPR_LZ4HC, + .comp_mutex = &lz4hc_mutex, + .name = "lz4hc", + .capi_name = "lz4hc", +}; +#else +static struct ubifs_compressor lz4hc_compr = { + .compr_type = UBIFS_COMPR_LZ4HC, + .name = "lz4hc", +}; +#endif + #ifdef CONFIG_UBIFS_FS_ZLIB static DEFINE_MUTEX(deflate_mutex); static DEFINE_MUTEX(inflate_mutex); @@ -224,10 +240,14 @@ { int err; - err = compr_init(&lzo_compr); + err = compr_init(&lz4hc_compr); if (err) return err; + err = compr_init(&lzo_compr); + if (err) + goto out_lz4hc; + err = compr_init(&zlib_compr); if (err) goto out_lzo; @@ -237,6 +257,8 @@ out_lzo: compr_exit(&lzo_compr); +out_lz4hc: + compr_exit(&lz4hc_compr); return err; } @@ -245,6 +267,7 @@ */ void ubifs_compressors_exit(void) { + compr_exit(&lz4hc_compr); compr_exit(&lzo_compr); compr_exit(&zlib_compr); } diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/Kconfig linux-3.12-rc3/fs/ubifs/Kconfig --- linux-3.12-rc3.orig/fs/ubifs/Kconfig 2013-09-29 17:02:38.000000000 -0500 +++ linux-3.12-rc3/fs/ubifs/Kconfig 2013-10-03 21:40:39.098747630 -0500 @@ -29,6 +29,14 @@ LZO compressor is generally faster than zlib but compresses worse. Say 'Y' if unsure. +config UBIFS_FS_LZ4HC + bool "LZ4HC compression support" if UBIFS_FS_ADVANCED_COMPR + depends on UBIFS_FS && CRYPTO_LZ4HC + default y + help + LZ4HC compressor is generally faster than zlib but compresses worse. + Say 'Y' if unsure. + config UBIFS_FS_ZLIB bool "ZLIB compression support" if UBIFS_FS_ADVANCED_COMPR depends on UBIFS_FS diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/super.c linux-3.12-rc3/fs/ubifs/super.c --- linux-3.12-rc3.orig/fs/ubifs/super.c 2013-09-29 17:02:38.000000000 -0500 +++ linux-3.12-rc3/fs/ubifs/super.c 2013-09-30 23:01:06.899526709 -0500 @@ -1040,6 +1040,8 @@ return -ENOMEM; if (!strcmp(name, "none")) c->mount_opts.compr_type = UBIFS_COMPR_NONE; + else if (!strcmp(name, "lz4hc")) + c->mount_opts.compr_type = UBIFS_COMPR_LZ4HC; else if (!strcmp(name, "lzo")) c->mount_opts.compr_type = UBIFS_COMPR_LZO; else if (!strcmp(name, "zlib")) diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/ubifs-media.h linux-3.12-rc3/fs/ubifs/ubifs-media.h --- linux-3.12-rc3.orig/fs/ubifs/ubifs-media.h 2013-09-29 17:02:38.000000000 -0500 +++ linux-3.12-rc3/fs/ubifs/ubifs-media.h 2013-07-16 22:56:02.435523610 -0500 @@ -332,12 +332,14 @@ * UBIFS_COMPR_NONE: no compression * UBIFS_COMPR_LZO: LZO compression * UBIFS_COMPR_ZLIB: ZLIB compression + * UBIFS_COMPR_LZ4HZ: LZ4HZ compression * UBIFS_COMPR_TYPES_CNT: count of supported compression types */ enum { UBIFS_COMPR_NONE, UBIFS_COMPR_LZO, UBIFS_COMPR_ZLIB, + UBIFS_COMPR_LZ4HC, UBIFS_COMPR_TYPES_CNT, }; Enjoy, Brent Taylor ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-04 3:09 ` Brent Taylor @ 2013-10-04 7:44 ` Artem Bityutskiy 2013-10-04 8:06 ` Konstantin Tokarev ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: Artem Bityutskiy @ 2013-10-04 7:44 UTC (permalink / raw) To: Brent Taylor; +Cc: linux-mtd@lists.infradead.org, Konstantin Tokarev On Thu, 2013-10-03 at 22:09 -0500, Brent Taylor wrote: > +config UBIFS_FS_LZ4HC > + bool "LZ4HC compression support" if UBIFS_FS_ADVANCED_COMPR > + depends on UBIFS_FS && CRYPTO_LZ4HC > + default y > + help > + LZ4HC compressor is generally faster than zlib but compresses worse. > + Say 'Y' if unsure. I'd say something like: LZ4HC is generally beats LZO on decompression speed while provides a lot better compression ratio (comparable to zlib). Compression speed is generally slower comparing to LZO, but faster comparing to zlib. Makes sense? -- Best Regards, Artem Bityutskiy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-04 3:09 ` Brent Taylor 2013-10-04 7:44 ` Artem Bityutskiy @ 2013-10-04 8:06 ` Konstantin Tokarev 2013-10-21 15:59 ` Konstantin Tokarev 2013-10-23 18:19 ` Yann Collet 3 siblings, 0 replies; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-04 8:06 UTC (permalink / raw) To: Brent Taylor; +Cc: linux-mtd@lists.infradead.org, Artem Bityutskiy 04.10.2013, 07:09, "Brent Taylor" <motobud@gmail.com>: > Here is a patch based on linux-3.12-rc3. I haven't performed any > performance testing UBIFS using lz4hc, but I can mount UBIFS volumes > and haven't seen any problems yet. Thank you! Are you planning to implement a patch for mtd-utils (mkfs.ubifs) as well? > The only think I know that isn't > correct about the patch is the description for the Kconfig element for > select lz4hc as a compression option. I only copied the description > from the lzo description. > > diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/compress.c > linux-3.12-rc3/fs/ubifs/compress.c > --- linux-3.12-rc3.orig/fs/ubifs/compress.c 2013-09-29 > 17:02:38.000000000 -0500 > +++ linux-3.12-rc3/fs/ubifs/compress.c 2013-07-17 21:57:27.440653860 -0500 > @@ -53,6 +53,22 @@ > }; > #endif > > +#ifdef CONFIG_UBIFS_FS_LZ4HC > +static DEFINE_MUTEX(lz4hc_mutex); > + > +static struct ubifs_compressor lz4hc_compr = { > + .compr_type = UBIFS_COMPR_LZ4HC, > + .comp_mutex = &lz4hc_mutex, > + .name = "lz4hc", > + .capi_name = "lz4hc", > +}; > +#else > +static struct ubifs_compressor lz4hc_compr = { > + .compr_type = UBIFS_COMPR_LZ4HC, > + .name = "lz4hc", > +}; > +#endif > + > #ifdef CONFIG_UBIFS_FS_ZLIB > static DEFINE_MUTEX(deflate_mutex); > static DEFINE_MUTEX(inflate_mutex); > @@ -224,10 +240,14 @@ > { > int err; > > - err = compr_init(&lzo_compr); > + err = compr_init(&lz4hc_compr); > if (err) > return err; > > + err = compr_init(&lzo_compr); > + if (err) > + goto out_lz4hc; > + > err = compr_init(&zlib_compr); > if (err) > goto out_lzo; > @@ -237,6 +257,8 @@ > > out_lzo: > compr_exit(&lzo_compr); > +out_lz4hc: > + compr_exit(&lz4hc_compr); > return err; > } > > @@ -245,6 +267,7 @@ > */ > void ubifs_compressors_exit(void) > { > + compr_exit(&lz4hc_compr); > compr_exit(&lzo_compr); > compr_exit(&zlib_compr); > } > diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/Kconfig > linux-3.12-rc3/fs/ubifs/Kconfig > --- linux-3.12-rc3.orig/fs/ubifs/Kconfig 2013-09-29 > 17:02:38.000000000 -0500 > +++ linux-3.12-rc3/fs/ubifs/Kconfig 2013-10-03 21:40:39.098747630 -0500 > @@ -29,6 +29,14 @@ > LZO compressor is generally faster than zlib but compresses worse. > Say 'Y' if unsure. > > +config UBIFS_FS_LZ4HC > + bool "LZ4HC compression support" if UBIFS_FS_ADVANCED_COMPR > + depends on UBIFS_FS && CRYPTO_LZ4HC > + default y > + help > + LZ4HC compressor is generally faster than zlib but compresses worse. > + Say 'Y' if unsure. > + > config UBIFS_FS_ZLIB > bool "ZLIB compression support" if UBIFS_FS_ADVANCED_COMPR > depends on UBIFS_FS > diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/super.c > linux-3.12-rc3/fs/ubifs/super.c > --- linux-3.12-rc3.orig/fs/ubifs/super.c 2013-09-29 > 17:02:38.000000000 -0500 > +++ linux-3.12-rc3/fs/ubifs/super.c 2013-09-30 23:01:06.899526709 -0500 > @@ -1040,6 +1040,8 @@ > return -ENOMEM; > if (!strcmp(name, "none")) > c->mount_opts.compr_type = UBIFS_COMPR_NONE; > + else if (!strcmp(name, "lz4hc")) > + c->mount_opts.compr_type = UBIFS_COMPR_LZ4HC; > else if (!strcmp(name, "lzo")) > c->mount_opts.compr_type = UBIFS_COMPR_LZO; > else if (!strcmp(name, "zlib")) > diff -uN -uNr linux-3.12-rc3.orig/fs/ubifs/ubifs-media.h > linux-3.12-rc3/fs/ubifs/ubifs-media.h > --- linux-3.12-rc3.orig/fs/ubifs/ubifs-media.h 2013-09-29 > 17:02:38.000000000 -0500 > +++ linux-3.12-rc3/fs/ubifs/ubifs-media.h 2013-07-16 > 22:56:02.435523610 -0500 > @@ -332,12 +332,14 @@ > * UBIFS_COMPR_NONE: no compression > * UBIFS_COMPR_LZO: LZO compression > * UBIFS_COMPR_ZLIB: ZLIB compression > + * UBIFS_COMPR_LZ4HZ: LZ4HZ compression > * UBIFS_COMPR_TYPES_CNT: count of supported compression types > */ > enum { > UBIFS_COMPR_NONE, > UBIFS_COMPR_LZO, > UBIFS_COMPR_ZLIB, > + UBIFS_COMPR_LZ4HC, > UBIFS_COMPR_TYPES_CNT, > }; > > Enjoy, > Brent Taylor -- Regards, Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-04 3:09 ` Brent Taylor 2013-10-04 7:44 ` Artem Bityutskiy 2013-10-04 8:06 ` Konstantin Tokarev @ 2013-10-21 15:59 ` Konstantin Tokarev 2013-10-22 3:43 ` Brent Taylor 2013-10-23 18:19 ` Yann Collet 3 siblings, 1 reply; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-21 15:59 UTC (permalink / raw) To: Brent Taylor; +Cc: linux-mtd@lists.infradead.org, Artem Bityutskiy 04.10.2013, 07:09, "Brent Taylor" <motobud@gmail.com>: > Here is a patch based on linux-3.12-rc3. I haven't performed any > performance testing UBIFS using lz4hc, but I can mount UBIFS volumes > and haven't seen any problems yet. The only think I know that isn't > correct about the patch is the description for the Kconfig element for > select lz4hc as a compression option. I only copied the description > from the lzo description. Hi Brent, I'm testing your patch on my SH4 device. When I create new partition with lz4hc compressor, it works fine: I can copy file into it, and md5sums of original and copy match. However, after reboot I cannot read the file anymore: UBIFS error (pid 1101): ubifs_decompress: cannot decompress 934 bytes, compressor lz4hc, error -22 UBIFS error (pid 1101): read_block: bad data node (block 1, inode 65) UBIFS error (pid 1101): do_readpage: cannot read page 1 of inode 65, error -22 The same error appears if I use lz4hc-compressed ubifs image to flash rootfs (using patched mkfs.ubifs). Decompression error occurs in lz4_uncompress() function (lib/lz4/lz4_decompress.c), on the line 101: /* Error: offset create reference outside destination buffer */ if (unlikely(ref < (BYTE *const) dest)) goto _output_error; Brent: are you able to read data from lz4hc volume on your device? Anyone: any ideas what may happen here? -- Regards, Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-21 15:59 ` Konstantin Tokarev @ 2013-10-22 3:43 ` Brent Taylor 2013-10-22 10:10 ` Konstantin Tokarev 0 siblings, 1 reply; 16+ messages in thread From: Brent Taylor @ 2013-10-22 3:43 UTC (permalink / raw) To: Konstantin Tokarev; +Cc: linux-mtd@lists.infradead.org, Artem Bityutskiy On Mon, Oct 21, 2013 at 10:59 AM, Konstantin Tokarev <annulen@yandex.ru> wrote: > > > 04.10.2013, 07:09, "Brent Taylor" <motobud@gmail.com>: >> Here is a patch based on linux-3.12-rc3. I haven't performed any >> performance testing UBIFS using lz4hc, but I can mount UBIFS volumes >> and haven't seen any problems yet. The only think I know that isn't >> correct about the patch is the description for the Kconfig element for >> select lz4hc as a compression option. I only copied the description >> from the lzo description. > > Hi Brent, > > I'm testing your patch on my SH4 device. When I create new partition > with lz4hc compressor, it works fine: I can copy file into it, and > md5sums of original and copy match. However, after reboot I cannot > read the file anymore: > > UBIFS error (pid 1101): ubifs_decompress: cannot decompress 934 bytes, compressor lz4hc, error -22 > UBIFS error (pid 1101): read_block: bad data node (block 1, inode 65) > UBIFS error (pid 1101): do_readpage: cannot read page 1 of inode 65, error -22 > > The same error appears if I use lz4hc-compressed ubifs image to flash rootfs > (using patched mkfs.ubifs). > > Decompression error occurs in lz4_uncompress() function (lib/lz4/lz4_decompress.c), > on the line 101: > > /* Error: offset create reference outside destination buffer */ > if (unlikely(ref < (BYTE *const) dest)) > goto _output_error; > > Brent: are you able to read data from lz4hc volume on your device? > Anyone: any ideas what may happen here? > > -- > Regards, > Konstantin Konstantin, I haven't seen anything like that on my at91sam9m10g45-ek development board. I haven't used a flash image from mkfs.ubifs yet. Is it possible the file system was not umounted cleanly before the reboot and UBIFS went through a recovery procedure? Maybe something breaks with lz4hc when UBIFS does a recovery? That's just a guess. Regards, Brent ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-22 3:43 ` Brent Taylor @ 2013-10-22 10:10 ` Konstantin Tokarev 2013-10-23 5:26 ` Brent Taylor 0 siblings, 1 reply; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-22 10:10 UTC (permalink / raw) To: Brent Taylor; +Cc: linux-mtd@lists.infradead.org, Artem Bityutskiy [-- Attachment #1: Type: text/plain, Size: 2162 bytes --] 22.10.2013, 07:43, "Brent Taylor" <motobud@gmail.com>: > On Mon, Oct 21, 2013 at 10:59 AM, Konstantin Tokarev <annulen@yandex.ru> wrote: > >> 04.10.2013, 07:09, "Brent Taylor" <motobud@gmail.com>: >>> Here is a patch based on linux-3.12-rc3. I haven't performed any >>> performance testing UBIFS using lz4hc, but I can mount UBIFS volumes >>> and haven't seen any problems yet. The only think I know that isn't >>> correct about the patch is the description for the Kconfig element for >>> select lz4hc as a compression option. I only copied the description >>> from the lzo description. >> Hi Brent, >> >> I'm testing your patch on my SH4 device. When I create new partition >> with lz4hc compressor, it works fine: I can copy file into it, and >> md5sums of original and copy match. However, after reboot I cannot >> read the file anymore: >> >> UBIFS error (pid 1101): ubifs_decompress: cannot decompress 934 bytes, compressor lz4hc, error -22 >> UBIFS error (pid 1101): read_block: bad data node (block 1, inode 65) >> UBIFS error (pid 1101): do_readpage: cannot read page 1 of inode 65, error -22 >> >> The same error appears if I use lz4hc-compressed ubifs image to flash rootfs >> (using patched mkfs.ubifs). >> >> Decompression error occurs in lz4_uncompress() function (lib/lz4/lz4_decompress.c), >> on the line 101: >> >> /* Error: offset create reference outside destination buffer */ >> if (unlikely(ref < (BYTE *const) dest)) >> goto _output_error; >> >> Brent: are you able to read data from lz4hc volume on your device? >> Anyone: any ideas what may happen here? >> >> -- >> Regards, >> Konstantin > > Konstantin, > I haven't seen anything like that on my at91sam9m10g45-ek > development board. I haven't used a flash image from mkfs.ubifs yet. > Is it possible the file system was not umounted cleanly before the > reboot and UBIFS went through a recovery procedure? Maybe something > breaks with lz4hc when UBIFS does a recovery? That's just a guess. Could you save attached file on lz4hc volume, umount it and mount again? I get aforementioned error when doing `cat set11.cfg` -- Regards, Konstantin [-- Attachment #2: set11.cfg --] [-- Type: application/octet-stream, Size: 169 bytes --] = true hideSingleSubtitle = true hideSingleAudioTrack = true ;showRecomended = true ;showRemoteUrl = true ;useIframeRewind = true iframeRewindTi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-22 10:10 ` Konstantin Tokarev @ 2013-10-23 5:26 ` Brent Taylor 2013-10-23 7:40 ` Konstantin Tokarev 0 siblings, 1 reply; 16+ messages in thread From: Brent Taylor @ 2013-10-23 5:26 UTC (permalink / raw) To: Konstantin Tokarev Cc: akpm, linux-mtd@lists.infradead.org, linux-kernel, Artem Bityutskiy Konstantin, I did my testing with data from /dev/urandom (which I now realize wasn't the best choice of data source), but if I use /dev/zero (which actually causes data compression to occur), the decompressor fails. I don't know the internal workings of the lz4hc compressor or the lz4 decompressor. I couldn't find any examples of any code in the kernel actually using the compressor. I've cc'ed the maintainers of the lz4hc_compress.c to see if they my have some more insight to the issue. -- Brent On Tue, Oct 22, 2013 at 5:10 AM, Konstantin Tokarev <annulen@yandex.ru> wrote: > > > 22.10.2013, 07:43, "Brent Taylor" <motobud@gmail.com>: >> On Mon, Oct 21, 2013 at 10:59 AM, Konstantin Tokarev <annulen@yandex.ru> wrote: >> >>> 04.10.2013, 07:09, "Brent Taylor" <motobud@gmail.com>: >>>> Here is a patch based on linux-3.12-rc3. I haven't performed any >>>> performance testing UBIFS using lz4hc, but I can mount UBIFS volumes >>>> and haven't seen any problems yet. The only think I know that isn't >>>> correct about the patch is the description for the Kconfig element for >>>> select lz4hc as a compression option. I only copied the description >>>> from the lzo description. >>> Hi Brent, >>> >>> I'm testing your patch on my SH4 device. When I create new partition >>> with lz4hc compressor, it works fine: I can copy file into it, and >>> md5sums of original and copy match. However, after reboot I cannot >>> read the file anymore: >>> >>> UBIFS error (pid 1101): ubifs_decompress: cannot decompress 934 bytes, compressor lz4hc, error -22 >>> UBIFS error (pid 1101): read_block: bad data node (block 1, inode 65) >>> UBIFS error (pid 1101): do_readpage: cannot read page 1 of inode 65, error -22 >>> >>> The same error appears if I use lz4hc-compressed ubifs image to flash rootfs >>> (using patched mkfs.ubifs). >>> >>> Decompression error occurs in lz4_uncompress() function (lib/lz4/lz4_decompress.c), >>> on the line 101: >>> >>> /* Error: offset create reference outside destination buffer */ >>> if (unlikely(ref < (BYTE *const) dest)) >>> goto _output_error; >>> >>> Brent: are you able to read data from lz4hc volume on your device? >>> Anyone: any ideas what may happen here? >>> >>> -- >>> Regards, >>> Konstantin >> >> Konstantin, >> I haven't seen anything like that on my at91sam9m10g45-ek >> development board. I haven't used a flash image from mkfs.ubifs yet. >> Is it possible the file system was not umounted cleanly before the >> reboot and UBIFS went through a recovery procedure? Maybe something >> breaks with lz4hc when UBIFS does a recovery? That's just a guess. > > Could you save attached file on lz4hc volume, umount it and mount again? > I get aforementioned error when doing `cat set11.cfg` > > -- > Regards, > Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-23 5:26 ` Brent Taylor @ 2013-10-23 7:40 ` Konstantin Tokarev 2013-10-23 12:49 ` Brent Taylor 0 siblings, 1 reply; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-23 7:40 UTC (permalink / raw) To: Brent Taylor Cc: akpm@linux-foundation.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Artem Bityutskiy 23.10.2013, 09:26, "Brent Taylor" <motobud@gmail.com>: > Konstantin, > I did my testing with data from /dev/urandom (which I now realize > wasn't the best choice of data source), but if I use /dev/zero (which > actually causes data compression to occur), the decompressor fails. I > don't know the internal workings of the lz4hc compressor or the lz4 > decompressor. I couldn't find any examples of any code in the kernel > actually using the compressor. I've cc'ed the maintainers of the > lz4hc_compress.c to see if they my have some more insight to the > issue. Does decompressor fail for you with the same error messages? Have you tried to copy my file to the volume? It looks like minimal test case for my board, if I remove any line decompressor works fine. -- Regards, Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-23 7:40 ` Konstantin Tokarev @ 2013-10-23 12:49 ` Brent Taylor 2013-10-23 13:39 ` Konstantin Tokarev 0 siblings, 1 reply; 16+ messages in thread From: Brent Taylor @ 2013-10-23 12:49 UTC (permalink / raw) To: Konstantin Tokarev Cc: akpm@linux-foundation.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Artem Bityutskiy On Wed, Oct 23, 2013 at 2:40 AM, Konstantin Tokarev <annulen@yandex.ru> wrote: > > > 23.10.2013, 09:26, "Brent Taylor" <motobud@gmail.com>: >> Konstantin, >> I did my testing with data from /dev/urandom (which I now realize >> wasn't the best choice of data source), but if I use /dev/zero (which >> actually causes data compression to occur), the decompressor fails. I >> don't know the internal workings of the lz4hc compressor or the lz4 >> decompressor. I couldn't find any examples of any code in the kernel >> actually using the compressor. I've cc'ed the maintainers of the >> lz4hc_compress.c to see if they my have some more insight to the >> issue. > > Does decompressor fail for you with the same error messages? > > Have you tried to copy my file to the volume? It looks like minimal test case > for my board, if I remove any line decompressor works fine. > > -- > Regards, > Konstantin Yes, I get the same error, here's a dump from UBIFS when I cat a file filled with data from /dev/zero: UBIFS error (pid 4288): ubifs_decompress: cannot decompress 12 bytes, compressor lz4hc, error -22 UBIFS error (pid 4288): read_block: bad data node (block 0, inode 71) magic 0x6101831 crc 0xff61a078 node_type 1 (data node) group_type 0 (no node group) sqnum 2700 len 60 key (71, data, 0) size 512 compr_typ 3 data size 12 data: 00000000: 1f 00 01 00 ff e8 50 00 00 00 00 00 UBIFS error (pid 4288): do_readpage: cannot read page 0 of inode 71, error -22 cat: /opt/data/zero.bin: Input/output error Steps to reproduce are: 1. Create a file with all zeros: dd if=/dev/zero bs=512 count=1 of=/opt/data/zero.bin 2. Unmount ubifs and detach ubi partition 3. attach ubi partition and mount ubifs 4. cat /opt/data/zero.bin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-23 12:49 ` Brent Taylor @ 2013-10-23 13:39 ` Konstantin Tokarev 0 siblings, 0 replies; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-23 13:39 UTC (permalink / raw) To: Brent Taylor Cc: akpm@linux-foundation.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Artem Bityutskiy 23.10.2013, 16:49, "Brent Taylor" <motobud@gmail.com>: > On Wed, Oct 23, 2013 at 2:40 AM, Konstantin Tokarev <annulen@yandex.ru> wrote: > >> 23.10.2013, 09:26, "Brent Taylor" <motobud@gmail.com>: >>> Konstantin, >>> I did my testing with data from /dev/urandom (which I now realize >>> wasn't the best choice of data source), but if I use /dev/zero (which >>> actually causes data compression to occur), the decompressor fails. I >>> don't know the internal workings of the lz4hc compressor or the lz4 >>> decompressor. I couldn't find any examples of any code in the kernel >>> actually using the compressor. I've cc'ed the maintainers of the >>> lz4hc_compress.c to see if they my have some more insight to the >>> issue. >> Does decompressor fail for you with the same error messages? >> >> Have you tried to copy my file to the volume? It looks like minimal test case >> for my board, if I remove any line decompressor works fine. >> >> -- >> Regards, >> Konstantin > > Yes, I get the same error, here's a dump from UBIFS when I cat a file > filled with data from /dev/zero: > > UBIFS error (pid 4288): ubifs_decompress: cannot decompress 12 bytes, > compressor lz4hc, error -22 > UBIFS error (pid 4288): read_block: bad data node (block 0, inode 71) > magic 0x6101831 > crc 0xff61a078 > node_type 1 (data node) > group_type 0 (no node group) > sqnum 2700 > len 60 > key (71, data, 0) > size 512 > compr_typ 3 > data size 12 > data: > 00000000: 1f 00 01 00 ff e8 50 00 00 00 00 00 > UBIFS error (pid 4288): do_readpage: cannot read page 0 of inode 71, error -22 > cat: /opt/data/zero.bin: Input/output error > > Steps to reproduce are: > 1. Create a file with all zeros: dd if=/dev/zero bs=512 count=1 > of=/opt/data/zero.bin > 2. Unmount ubifs and detach ubi partition > 3. attach ubi partition and mount ubifs > 4. cat /opt/data/zero.bin Reproduced here. -- Regards, Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-04 3:09 ` Brent Taylor ` (2 preceding siblings ...) 2013-10-21 15:59 ` Konstantin Tokarev @ 2013-10-23 18:19 ` Yann Collet 2013-10-24 14:12 ` Konstantin Tokarev 3 siblings, 1 reply; 16+ messages in thread From: Yann Collet @ 2013-10-23 18:19 UTC (permalink / raw) To: linux-mtd UBIFS error (pid 4288): ubifs_decompress: cannot decompress 12 bytes, (...) data size 12 data: 00000000: 1f 00 01 00 ff e8 50 00 00 00 00 00 The compressed format is correct. It describes a flow of 00, of size ~500. So the problem seems more likely on the decompression side. Are you sure you are providing "12" as the "input size" parameter ? and that the "maximum output size" parameter is correctly provided ? (i.e. >= to original data size) Regards ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-23 18:19 ` Yann Collet @ 2013-10-24 14:12 ` Konstantin Tokarev 2013-10-24 15:15 ` Konstantin Tokarev 0 siblings, 1 reply; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-24 14:12 UTC (permalink / raw) To: Yann Collet, linux-mtd@lists.infradead.org 23.10.2013, 22:26, "Yann Collet" <yann.collet.73@gmail.com>: > UBIFS error (pid 4288): ubifs_decompress: cannot decompress 12 bytes, > (...) > data size 12 > data: > 00000000: 1f 00 01 00 ff e8 50 00 00 00 00 00 > > The compressed format is correct. > It describes a flow of 00, of size ~500. > > So the problem seems more likely on the decompression side. > > Are you sure you are providing "12" as the "input size" parameter ? and that > the "maximum output size" parameter is correctly provided ? (i.e. >= to > original data size) > Decompression code in kernel[1] is heavily modified. In particular, lz4_uncompress function (used in this case) does not have input size parameter at all, while it's present in lz4_uncompress_unknownoutputsize. [1] lib/lz4/lz4_decompress.c -- Regards, Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-24 14:12 ` Konstantin Tokarev @ 2013-10-24 15:15 ` Konstantin Tokarev 2013-10-28 16:22 ` Konstantin Tokarev 0 siblings, 1 reply; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-24 15:15 UTC (permalink / raw) To: Yann Collet, linux-mtd@lists.infradead.org, Brent Taylor, Artem Bityutskiy, linux-kernel, akpm [-- Attachment #1: Type: text/plain, Size: 1440 bytes --] 24.10.2013, 18:20, "Konstantin Tokarev" <annulen@yandex.ru>: > 23.10.2013, 22:26, "Yann Collet" <yann.collet.73@gmail.com>: > >> UBIFS error (pid 4288): ubifs_decompress: cannot decompress 12 bytes, >> (...) >> data size 12 >> data: >> 00000000: 1f 00 01 00 ff e8 50 00 00 00 00 00 >> >> The compressed format is correct. >> It describes a flow of 00, of size ~500. >> >> So the problem seems more likely on the decompression side. >> >> Are you sure you are providing "12" as the "input size" parameter ? and that >> the "maximum output size" parameter is correctly provided ? (i.e. >= to >> original data size) > > Decompression code in kernel[1] is heavily modified. In particular, lz4_uncompress > function (used in this case) does not have input size parameter at all, > while it's present in lz4_uncompress_unknownoutputsize. > > [1] lib/lz4/lz4_decompress.c Attached patch to crypto API wrapper for lz4hc seems to fix the issue. I can save and read files on LZ4HC-compressed volume, and I'm running on LZ4HC-compressed rootfs, flashed from mkfs.ubifs generated image (patch by Elie De Brauwer). My software works correctly. Unfortunately, on my board LZ4HC-compressed UBIFS volume performs noticeable worse than LZO, while still faster than zlib. I guess the reason is that CPU is no longer a bottleneck for my system, but NAND speed. Thank you all for your help! -- Regards, Konstantin [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: crypto_lz4.patch --] [-- Type: text/x-diff; name="crypto_lz4.patch", Size: 852 bytes --] diff --git a/crypto/lz4.c b/crypto/lz4.c index 4586dd1..91a0613 100644 --- a/crypto/lz4.c +++ b/crypto/lz4.c @@ -66,9 +66,8 @@ static int lz4_decompress_crypto(struct crypto_tfm *tfm, const u8 *src, { int err; size_t tmp_len = *dlen; - size_t __slen = slen; - err = lz4_decompress(src, &__slen, dst, tmp_len); + err = lz4_decompress_unknownoutputsize(src, slen, dst, &tmp_len); if (err < 0) return -EINVAL; diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c index 151ba31..9987741 100644 --- a/crypto/lz4hc.c +++ b/crypto/lz4hc.c @@ -66,9 +66,8 @@ static int lz4hc_decompress_crypto(struct crypto_tfm *tfm, const u8 *src, { int err; size_t tmp_len = *dlen; - size_t __slen = slen; - err = lz4_decompress(src, &__slen, dst, tmp_len); + err = lz4_decompress_unknownoutputsize(src, slen, dst, &tmp_len); if (err < 0) return -EINVAL; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-24 15:15 ` Konstantin Tokarev @ 2013-10-28 16:22 ` Konstantin Tokarev 2013-10-28 16:45 ` Florian Fainelli 0 siblings, 1 reply; 16+ messages in thread From: Konstantin Tokarev @ 2013-10-28 16:22 UTC (permalink / raw) To: Yann Collet, linux-mtd@lists.infradead.org, Brent Taylor, Artem Bityutskiy, linux-kernel@vger.kernel.org, akpm@linux-foundation.org 24.10.2013, 19:15, "Konstantin Tokarev" <annulen@yandex.ru>: > Attached patch to crypto API wrapper for lz4hc seems to fix the issue. I can save > and read files on LZ4HC-compressed volume, and I'm running on LZ4HC-compressed rootfs, > flashed from mkfs.ubifs generated image (patch by Elie De Brauwer). My software > works correctly. > > Unfortunately, on my board LZ4HC-compressed UBIFS volume performs noticeable worse > than LZO, while still faster than zlib. I guess the reason is that CPU is no longer a > bottleneck for my system, but NAND speed. > > Thank you all for your help! Can anyone review the correctness of my patch? Looks like API of LZ4 decompressor is used wrong way in crypto API. -- Regards, Konstantin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: lz4hc compression in UBIFS? 2013-10-28 16:22 ` Konstantin Tokarev @ 2013-10-28 16:45 ` Florian Fainelli 0 siblings, 0 replies; 16+ messages in thread From: Florian Fainelli @ 2013-10-28 16:45 UTC (permalink / raw) To: Konstantin Tokarev Cc: Artem Bityutskiy, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, Yann Collet, akpm@linux-foundation.org, Brent Taylor 2013/10/28 Konstantin Tokarev <annulen@yandex.ru>: > > > 24.10.2013, 19:15, "Konstantin Tokarev" <annulen@yandex.ru>: >> Attached patch to crypto API wrapper for lz4hc seems to fix the issue. I can save >> and read files on LZ4HC-compressed volume, and I'm running on LZ4HC-compressed rootfs, >> flashed from mkfs.ubifs generated image (patch by Elie De Brauwer). My software >> works correctly. >> >> Unfortunately, on my board LZ4HC-compressed UBIFS volume performs noticeable worse >> than LZO, while still faster than zlib. I guess the reason is that CPU is no longer a >> bottleneck for my system, but NAND speed. >> >> Thank you all for your help! > > Can anyone review the correctness of my patch? Looks like API of LZ4 decompressor is > used wrong way in crypto API. Can you make a formal submission of it? That would probably help reviewing it. -- Florian ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-10-28 16:46 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-20 12:16 lz4hc compression in UBIFS? Konstantin Tokarev 2013-10-04 3:09 ` Brent Taylor 2013-10-04 7:44 ` Artem Bityutskiy 2013-10-04 8:06 ` Konstantin Tokarev 2013-10-21 15:59 ` Konstantin Tokarev 2013-10-22 3:43 ` Brent Taylor 2013-10-22 10:10 ` Konstantin Tokarev 2013-10-23 5:26 ` Brent Taylor 2013-10-23 7:40 ` Konstantin Tokarev 2013-10-23 12:49 ` Brent Taylor 2013-10-23 13:39 ` Konstantin Tokarev 2013-10-23 18:19 ` Yann Collet 2013-10-24 14:12 ` Konstantin Tokarev 2013-10-24 15:15 ` Konstantin Tokarev 2013-10-28 16:22 ` Konstantin Tokarev 2013-10-28 16:45 ` Florian Fainelli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox