[PATCH 2/2] Squashfs: Add XZ compression configuration option

[PATCH 2/2] Squashfs: Add XZ compression configuration option

[Phillip Lougher]

Signed-off-by: Phillip Lougher <phillip@lougher.demon.co.uk>
---
  fs/squashfs/Kconfig        |   16 ++++++++++++++++
  fs/squashfs/Makefile       |    1 +
  fs/squashfs/decompressor.c |   11 +++++++++++
  fs/squashfs/squashfs.h     |    3 +++
  4 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
index e5f63da..e96d99a 100644
--- a/fs/squashfs/Kconfig
+++ b/fs/squashfs/Kconfig
@@ -53,6 +53,22 @@ config SQUASHFS_LZO

  	  If unsure, say N.

+config SQUASHFS_XZ
+	bool "Include support for XZ compressed file systems"
+	depends on SQUASHFS
+	default n
+	select XZ_DEC
+	help
+	  Saying Y here includes support for reading Squashfs file systems
+	  compressed with XZ compresssion.  XZ gives better compression than
+	  the default zlib compression, at the expense of greater CPU and
+	  memory overhead.
+
+	  XZ is not the standard compression used in Squashfs and so most
+	  file systems will be readable without selecting this option.
+
+	  If unsure, say N.
+
  config SQUASHFS_EMBEDDED
  	bool "Additional option for memory-constrained systems"
  	depends on SQUASHFS
diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
index 7672bac..cecf2be 100644
--- a/fs/squashfs/Makefile
+++ b/fs/squashfs/Makefile
@@ -7,3 +7,4 @@ squashfs-y += block.o cache.o dir.o export.o file.o fragment.o id.o inode.o
  squashfs-y += namei.o super.o symlink.o zlib_wrapper.o decompressor.o
  squashfs-$(CONFIG_SQUASHFS_XATTR) += xattr.o xattr_id.o
  squashfs-$(CONFIG_SQUASHFS_LZO) += lzo_wrapper.o
+squashfs-$(CONFIG_SQUASHFS_XZ) += xz_wrapper.o
diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c
index 24af9ce..ac333b8 100644
--- a/fs/squashfs/decompressor.c
+++ b/fs/squashfs/decompressor.c
@@ -46,6 +46,12 @@ static const struct squashfs_decompressor squashfs_lzo_unsupported_comp_ops = {
  };
  #endif

+#ifndef CONFIG_SQUASHFS_XZ
+static const struct squashfs_decompressor squashfs_xz_unsupported_comp_ops = {
+	NULL, NULL, NULL, XZ_COMPRESSION, "xz", 0
+};
+#endif
+
  static const struct squashfs_decompressor squashfs_unknown_comp_ops = {
  	NULL, NULL, NULL, 0, "unknown", 0
  };
@@ -58,6 +64,11 @@ static const struct squashfs_decompressor *decompressor[] = {
  #else
  	&squashfs_lzo_unsupported_comp_ops,
  #endif
+#ifdef CONFIG_SQUASHFS_XZ
+	&squashfs_xz_comp_ops,
+#else
+	&squashfs_xz_unsupported_comp_ops,
+#endif
  	&squashfs_unknown_comp_ops
  };

diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 5d45569..1096e2e 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -107,3 +107,6 @@ extern const struct squashfs_decompressor squashfs_zlib_comp_ops;

  /* lzo_wrapper.c */
  extern const struct squashfs_decompressor squashfs_lzo_comp_ops;
+
+/* xz_wrapper.c */
+extern const struct squashfs_decompressor squashfs_xz_comp_ops;
-- 
1.6.3.3

Re: [PATCH 2/2] Squashfs: Add XZ compression configuration option

[Geert Uytterhoeven]

On Thu, Dec 9, 2010 at 07:11, Phillip Lougher
<phillip@lougher.demon.co.uk> wrote:
> --- a/fs/squashfs/Kconfig
> +++ b/fs/squashfs/Kconfig
> @@ -53,6 +53,22 @@ config SQUASHFS_LZO
>
>          If unsure, say N.
>
> +config SQUASHFS_XZ
> +       bool "Include support for XZ compressed file systems"
> +       depends on SQUASHFS
> +       default n

"default n" is the default, no reason to specify it.

Do we need a checkpatch test for this?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] Squashfs: Add XZ compression configuration option

[Lasse Collin]

On 2010-12-09 Phillip Lougher wrote:
> +config SQUASHFS_XZ
> +       bool "Include support for XZ compressed file systems"
> +       depends on SQUASHFS
> +       default n
> +       select XZ_DEC

Should "select XZ_DEC" be replaced with "depends on XZ_DEC"? XZ_DEC 
requires CRC32, so if "select XZ_DEC" is used, there needs to be also 
"select CRC32".

XZ_DEC may optionally use other XZ_DEC_* symbols, which the user will 
want to choose when building for an embedded system. With "depends on 
XZ_DEC" the user will see that there's more than a single option that 
affects the details of the XZ support in Squashfs.

-- 
Lasse Collin  |  IRC: Larhzu @ IRCnet & Freenode

Re: [PATCH 2/2] Squashfs: Add XZ compression configuration option

[Phillip Lougher]

Geert Uytterhoeven wrote:
> On Thu, Dec 9, 2010 at 07:11, Phillip Lougher
> <phillip@lougher.demon.co.uk> wrote:
>> --- a/fs/squashfs/Kconfig
>> +++ b/fs/squashfs/Kconfig
>> @@ -53,6 +53,22 @@ config SQUASHFS_LZO
>>
>>          If unsure, say N.
>>
>> +config SQUASHFS_XZ
>> +       bool "Include support for XZ compressed file systems"
>> +       depends on SQUASHFS
>> +       default n
> 
> "default n" is the default, no reason to specify it.
> 

Yes, thanks for pointing that out.

It seems there's a lot of "default n"s in the mainline kernel

% find . -name "Kconfig"| xargs grep "default n" | wc
     485    1535   18753

including two in arch/m68k ;-)

Phillip

Re: [PATCH 2/2] Squashfs: Add XZ compression configuration option

[Phillip Lougher]

Lasse Collin wrote:
> On 2010-12-09 Phillip Lougher wrote:
>> +config SQUASHFS_XZ
>> +       bool "Include support for XZ compressed file systems"
>> +       depends on SQUASHFS
>> +       default n
>> +       select XZ_DEC
> 
> Should "select XZ_DEC" be replaced with "depends on XZ_DEC"? XZ_DEC 
> requires CRC32, so if "select XZ_DEC" is used, there needs to be also 
> "select CRC32".
> 

XZ_DEC selects CRC32, kbuild handles these nested selects quite happily,
so if something selects XZ_DEC it knows it has to also select CRC32.

Depends on has quite different semantics to selects.  If SQUASHFS_XZ
was made to depend on XZ_DEC then the option simply won't appear unless
the user knew to select XZ_DEC first (as it's default n).  This would
prove extremely confusing, and probably lead to most people thinking
Squashfs didn't have XZ support, which is somewhat undesirable.

> XZ_DEC may optionally use other XZ_DEC_* symbols, which the user will 
> want to choose when building for an embedded system. With "depends on 
> XZ_DEC" the user will see that there's more than a single option that 
> affects the details of the XZ support in Squashfs.
>

With depends on XZ_DEC the user will simply not see that Squashfs has
XZ support (as the option won't appear unless XZ_DEC is explicitly
selected).

With selects XZ_DEC users will see that Squashfs has XZ support, if
enabled, they'll simply see that XZ_DEC has been automatically
selected in the "Library routines" sub-menu.  If EMBEDDED is not
selected the XZ_DEC options will be automatically selected (as
they're only user selectable if EMBEDDED is selected).  If
EMBEDDED is selected, then they'll have the choice then to decide
which options they wish to de-select.

I think this is preferable to needing XZ_DEC to be selected before
the SQUASHFS_XZ option even appears.

Phillip

Re: [PATCH 2/2] Squashfs: Add XZ compression configuration option

[Lasse Collin]

On 2010-12-10 Phillip Lougher wrote:
> Lasse Collin wrote:
> > On 2010-12-09 Phillip Lougher wrote:
> >> +config SQUASHFS_XZ
> >> +       bool "Include support for XZ compressed file systems"
> >> +       depends on SQUASHFS
> >> +       default n
> >> +       select XZ_DEC
> > 
> > Should "select XZ_DEC" be replaced with "depends on XZ_DEC"? XZ_DEC
> > requires CRC32, so if "select XZ_DEC" is used, there needs to be
> > also "select CRC32".
> 
> XZ_DEC selects CRC32, kbuild handles these nested selects quite
> happily, so if something selects XZ_DEC it knows it has to also
> select CRC32.

OK, I had misunderstood the notice about "select" and dependencies in 
Documentation/kbuild/kconfig-language.txt.

> Depends on has quite different semantics to selects.  If SQUASHFS_XZ
> was made to depend on XZ_DEC then the option simply won't appear
> unless the user knew to select XZ_DEC first (as it's default n). 
> This would prove extremely confusing, and probably lead to most
> people thinking Squashfs didn't have XZ support, which is somewhat
> undesirable.

Good point. I always keep the inactive options visible in xconfig so I 
missed this. I will need to update my XZ boot-time support patch to use 
"select" instead of "depends on" too. Thanks!

-- 
Lasse Collin  |  IRC: Larhzu @ IRCnet & Freenode

Re: [PATCH 2/2] Squashfs: Add XZ compression configuration option

[Geert Uytterhoeven]

On Fri, Dec 10, 2010 at 07:23, Phillip Lougher
<phillip@lougher.demon.co.uk> wrote:
> Geert Uytterhoeven wrote:
>> On Thu, Dec 9, 2010 at 07:11, Phillip Lougher
>> <phillip@lougher.demon.co.uk> wrote:
>>>
>>> --- a/fs/squashfs/Kconfig
>>> +++ b/fs/squashfs/Kconfig
>>> @@ -53,6 +53,22 @@ config SQUASHFS_LZO
>>>
>>>         If unsure, say N.
>>>
>>> +config SQUASHFS_XZ
>>> +       bool "Include support for XZ compressed file systems"
>>> +       depends on SQUASHFS
>>> +       default n
>>
>> "default n" is the default, no reason to specify it.
>>
>
> Yes, thanks for pointing that out.
>
> It seems there's a lot of "default n"s in the mainline kernel
>
> % find . -name "Kconfig"| xargs grep "default n" | wc
>    485    1535   18753
>
> including two in arch/m68k ;-)

Thanks, fixed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds