lz4hc compression in UBIFS?

lz4hc compression in UBIFS?

[Konstantin Tokarev]

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

Re: lz4hc compression in UBIFS?

[Brent Taylor]

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

Re: lz4hc compression in UBIFS?

[Artem Bityutskiy]

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

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

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

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

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

Re: lz4hc compression in UBIFS?

[Brent Taylor]

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

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

[-- 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

Re: lz4hc compression in UBIFS?

[Brent Taylor]

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

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

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

Re: lz4hc compression in UBIFS?

[Brent Taylor]

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

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

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

Re: lz4hc compression in UBIFS?

[Yann Collet]

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 

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

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

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

[-- 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;
 

Re: lz4hc compression in UBIFS?

[Konstantin Tokarev]

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

Re: lz4hc compression in UBIFS?

[Florian Fainelli]

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