All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option
@ 2022-05-17  3:24 Linus Kaschulla via buildroot
  2022-05-17  3:24 ` [Buildroot] [PATCH 2/2] fs/squashfs: Add option to compress with extreme options Linus Kaschulla via buildroot
  2022-05-17  5:38 ` [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Yann E. MORIN
  0 siblings, 2 replies; 6+ messages in thread
From: Linus Kaschulla via buildroot @ 2022-05-17  3:24 UTC (permalink / raw)
  To: buildroot; +Cc: Linus Kaschulla, Yann E . MORIN

One advantage of squashfs over similar technologies is the support for
bigger block sizes. However the default size is not a lot bigger
(typically 128k if no `-b` flag specified).

This patch adds the ability to select from common block sizes
which for example can aid in improving compression ratio.

Signed-off-by: Linus Kaschulla <linus@cosmos-ink.net>
---
 fs/squashfs/Config.in   | 50 +++++++++++++++++++++++++++++++++++++++++
 fs/squashfs/squashfs.mk | 14 +++++++++++-
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in
index 70d4a20cf0..341e50a08c 100644
--- a/fs/squashfs/Config.in
+++ b/fs/squashfs/Config.in
@@ -5,6 +5,56 @@ config BR2_TARGET_ROOTFS_SQUASHFS
 
 if BR2_TARGET_ROOTFS_SQUASHFS
 
+choice
+	prompt "block size"
+	default BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
+	help
+	  Data block size. Bigger values can improve
+	  compression ratio.
+
+	  If unsure, leave at 128k (default).
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_4K
+	bool "4k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_8K
+	bool "8k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_16K
+	bool "16k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_32K
+	bool "32k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_64K
+	bool "64k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
+	bool "128k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_256K
+	bool "256k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_512K
+	bool "512k"
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS_1024K
+	bool "1024k"
+
+endchoice
+
+config BR2_TARGET_ROOTFS_SQUASHFS_BS
+	string
+	default "4K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_4K
+	default "8K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_84K
+	default "16K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_16K
+	default "32K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_32K
+	default "64K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_64K
+	default "128K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
+	default "256K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_256K
+	default "512K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_512K
+	default "1024K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_1024K
+
 config BR2_TARGET_ROOTFS_SQUASHFS_PAD
 	bool "pad to a 4K boundary"
 	default y # legacy was always ON
diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk
index 7a5e3e313e..1790773f7e 100644
--- a/fs/squashfs/squashfs.mk
+++ b/fs/squashfs/squashfs.mk
@@ -6,7 +6,19 @@
 
 ROOTFS_SQUASHFS_DEPENDENCIES = host-squashfs
 
-ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS)
+ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS) -b $(BR2_TARGET_ROOTFS_SQUASHFS_BS)
+
+ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_64K),y)
+ROOTFS_SQUASHFS_ARGS += -b 64K
+else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_128K),y)
+ROOTFS_SQUASHFS_ARGS += -b 128K
+else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_256K),y)
+ROOTFS_SQUASHFS_ARGS += -b 256K
+else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_512K),y)
+ROOTFS_SQUASHFS_ARGS += -b 512K
+else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_1024K),y)
+ROOTFS_SQUASHFS_ARGS += -b 1024K
+endif
 
 ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS_PAD),)
 ROOTFS_SQUASHFS_ARGS += -nopad
-- 
2.35.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 2/2] fs/squashfs: Add option to compress with extreme options
  2022-05-17  3:24 [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Linus Kaschulla via buildroot
@ 2022-05-17  3:24 ` Linus Kaschulla via buildroot
  2022-05-17  6:09   ` Yann E. MORIN
  2022-05-17  5:38 ` [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Yann E. MORIN
  1 sibling, 1 reply; 6+ messages in thread
From: Linus Kaschulla via buildroot @ 2022-05-17  3:24 UTC (permalink / raw)
  To: buildroot; +Cc: Linus Kaschulla, Yann E . MORIN

mksquashfs allows to tweak many compressions parameters. Currently they
can't be changed from kmenu. Leaving out potential space savings.

This adds the option to enable a set of predetermined compression
options. This option is enabled by default for lz4 since lz4 currently
implicitly added the extreme to it in the makefile. So this aids in
keeping backward compatibility.

Signed-off-by: Linus Kaschulla <linus@cosmos-ink.net>
---
 fs/squashfs/Config.in   | 26 ++++++++++++++++++++++++++
 fs/squashfs/squashfs.mk |  6 +++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in
index 341e50a08c..41c782105d 100644
--- a/fs/squashfs/Config.in
+++ b/fs/squashfs/Config.in
@@ -89,4 +89,30 @@ config BR2_TARGET_ROOTFS_SQUASHFS4_ZSTD
 
 endchoice
 
+config BR2_TARGET_ROOTFS_SQUASHFS_EXTREME_COMP
+	bool "extreme compression if possible"
+	default BR2_TARGET_ROOTFS_SQUASHFS4_LZ4
+	help
+	  Use options to increase compression ration as much as
+	  possible, like using architecture-specific options, at
+	  the cost of time when assembling the filesystem image.
+
+	  For example:
+	   - with gzip and lzo, use -Xcompression-level 9
+	   - with xz use a architecture specific bjc (branch-call-jump) filter
+	   - with zstd use -Xcompression-level 22
+	   - and more
+
+config BR2_TARGET_ROOTFS_SQUASHFS_COMP_OPTS
+	string
+	depends on BR2_TARGET_ROOTFS_SQUASHFS_EXTREME_COMP
+	default "-Xcompression-level 9" if BR2_TARGET_ROOTFS_SQUASHFS4_GZIP
+	default "-Xcompression-level 9" if BR2_TARGET_ROOTFS_SQUASHFS4_LZO
+	default "-Xhc" if BR2_TARGET_ROOTFS_SQUASHFS4_LZ4
+	default "-Xbcj arm,armthumb" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_arm || BR_aarch64)
+	default "-Xbcj powerpc" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_powerpc || BR2_powerpc64)
+	default "-Xbcj sparc" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_sparc || BR2_sparc64)
+	default "-Xbcj x86" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_i386 || BR2_x86_64)
+	default "-Xcompression-level 22" if BR2_TARGET_ROOTFS_SQUASHFS4_ZSTD
+
 endif
diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk
index 1790773f7e..51f8d0d7c9 100644
--- a/fs/squashfs/squashfs.mk
+++ b/fs/squashfs/squashfs.mk
@@ -25,7 +25,7 @@ ROOTFS_SQUASHFS_ARGS += -nopad
 endif
 
 ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZ4),y)
-ROOTFS_SQUASHFS_ARGS += -comp lz4 -Xhc
+ROOTFS_SQUASHFS_ARGS += -comp lz4
 else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZO),y)
 ROOTFS_SQUASHFS_ARGS += -comp lzo
 else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZMA),y)
@@ -38,6 +38,10 @@ else
 ROOTFS_SQUASHFS_ARGS += -comp gzip
 endif
 
+ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS_EXTREME_COMP),y)
+ROOTFS_SQUASHFS_ARGS += $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_COMP_OPTS))
+endif
+
 define ROOTFS_SQUASHFS_CMD
 	$(HOST_DIR)/bin/mksquashfs $(TARGET_DIR) $@ $(ROOTFS_SQUASHFS_ARGS)
 endef
-- 
2.35.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option
  2022-05-17  3:24 [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Linus Kaschulla via buildroot
  2022-05-17  3:24 ` [Buildroot] [PATCH 2/2] fs/squashfs: Add option to compress with extreme options Linus Kaschulla via buildroot
@ 2022-05-17  5:38 ` Yann E. MORIN
  2022-05-17  6:05   ` Yann E. MORIN
  2022-05-17 14:04   ` Linus via buildroot
  1 sibling, 2 replies; 6+ messages in thread
From: Yann E. MORIN @ 2022-05-17  5:38 UTC (permalink / raw)
  To: Linus Kaschulla; +Cc: buildroot

Linus, All,

On 2022-05-17 03:24 +0000, Linus Kaschulla spake thusly:
> One advantage of squashfs over similar technologies is the support for
> bigger block sizes. However the default size is not a lot bigger
> (typically 128k if no `-b` flag specified).
> 
> This patch adds the ability to select from common block sizes
> which for example can aid in improving compression ratio.
> 
> Signed-off-by: Linus Kaschulla <linus@cosmos-ink.net>
> ---
>  fs/squashfs/Config.in   | 50 +++++++++++++++++++++++++++++++++++++++++
>  fs/squashfs/squashfs.mk | 14 +++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in
> index 70d4a20cf0..341e50a08c 100644
> --- a/fs/squashfs/Config.in
> +++ b/fs/squashfs/Config.in
> @@ -5,6 +5,56 @@ config BR2_TARGET_ROOTFS_SQUASHFS
>  
>  if BR2_TARGET_ROOTFS_SQUASHFS
>  
> +choice
> +	prompt "block size"
> +	default BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
> +	help
> +	  Data block size. Bigger values can improve
> +	  compression ratio.
> +
> +	  If unsure, leave at 128k (default).
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_4K
> +	bool "4k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_8K
> +	bool "8k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_16K
> +	bool "16k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_32K
> +	bool "32k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_64K
> +	bool "64k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
> +	bool "128k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_256K
> +	bool "256k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_512K
> +	bool "512k"
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_1024K
> +	bool "1024k"
> +
> +endchoice
> +
> +config BR2_TARGET_ROOTFS_SQUASHFS_BS
> +	string
> +	default "4K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_4K
> +	default "8K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_84K
> +	default "16K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_16K
> +	default "32K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_32K
> +	default "64K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_64K
> +	default "128K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
> +	default "256K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_256K
> +	default "512K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_512K
> +	default "1024K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_1024K
> +
>  config BR2_TARGET_ROOTFS_SQUASHFS_PAD
>  	bool "pad to a 4K boundary"
>  	default y # legacy was always ON
> diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk
> index 7a5e3e313e..1790773f7e 100644
> --- a/fs/squashfs/squashfs.mk
> +++ b/fs/squashfs/squashfs.mk
> @@ -6,7 +6,19 @@
>  
>  ROOTFS_SQUASHFS_DEPENDENCIES = host-squashfs
>  
> -ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS)
> +ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS) -b $(BR2_TARGET_ROOTFS_SQUASHFS_BS)

Here you added the value to the option list using the hidden option
string, so...

> +ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_64K),y)
> +ROOTFS_SQUASHFS_ARGS += -b 64K
> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_128K),y)
> +ROOTFS_SQUASHFS_ARGS += -b 128K
> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_256K),y)
> +ROOTFS_SQUASHFS_ARGS += -b 256K
> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_512K),y)
> +ROOTFS_SQUASHFS_ARGS += -b 512K
> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_1024K),y)
> +ROOTFS_SQUASHFS_ARGS += -b 1024K
> +endif

... there is no need to add it a second time based on the booleans.

Finally, it would have been nice to add two test cases that at least the
4K and 1024K options do work (the existing test tests the defaultm i.e
128K). Tests are in:  support/testing/tests/fs/test_squashfs.py

Note that adding tests can be done in a followup patch, of course.

Regards,
Yann E. MORIN.

>  ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS_PAD),)
>  ROOTFS_SQUASHFS_ARGS += -nopad
> -- 
> 2.35.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option
  2022-05-17  5:38 ` [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Yann E. MORIN
@ 2022-05-17  6:05   ` Yann E. MORIN
  2022-05-17 14:04   ` Linus via buildroot
  1 sibling, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2022-05-17  6:05 UTC (permalink / raw)
  To: Linus Kaschulla; +Cc: buildroot

Linus, All,

On 2022-05-17 07:38 +0200, Yann E. MORIN spake thusly:
> On 2022-05-17 03:24 +0000, Linus Kaschulla spake thusly:
> > This patch adds the ability to select from common block sizes
> > which for example can aid in improving compression ratio.
[--SNIP--]
> > +ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_64K),y)

I got a bit confused here, in fact: BR2_TARGET_ROOTFS_SQUASHFS4_BS_64K
has a '4' after the 'SQUASHFS', but the options in the choice do not,
so none of this conditional block was ever hit in fact. I dropped it.

And I see where that '4' comes from: you mimicked the compression type
choice. I totally missed that initially. That's a remnant from days pat,
and if we were to add that code today, we'd not keep the '4'.

An yway, applied to master with:
  - drop spurious boolean-based setting in .mk
  - split into multi-line
  - qstrip variable expansion

Thanks!

> Finally, it would have been nice to add two test cases that at least the
> 4K and 1024K options do work (the existing test tests the defaultm i.e
> 128K). Tests are in:  support/testing/tests/fs/test_squashfs.py

Still interested in seeing a test-case, though! ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 2/2] fs/squashfs: Add option to compress with extreme options
  2022-05-17  3:24 ` [Buildroot] [PATCH 2/2] fs/squashfs: Add option to compress with extreme options Linus Kaschulla via buildroot
@ 2022-05-17  6:09   ` Yann E. MORIN
  0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2022-05-17  6:09 UTC (permalink / raw)
  To: Linus Kaschulla; +Cc: buildroot

Linus, All,

On 2022-05-17 03:24 +0000, Linus Kaschulla via buildroot spake thusly:
> mksquashfs allows to tweak many compressions parameters. Currently they
> can't be changed from kmenu. Leaving out potential space savings.
> 
> This adds the option to enable a set of predetermined compression
> options. This option is enabled by default for lz4 since lz4 currently
> implicitly added the extreme to it in the makefile. So this aids in
> keeping backward compatibility.
> 
> Signed-off-by: Linus Kaschulla <linus@cosmos-ink.net>
> ---
>  fs/squashfs/Config.in   | 26 ++++++++++++++++++++++++++
>  fs/squashfs/squashfs.mk |  6 +++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in
> index 341e50a08c..41c782105d 100644
> --- a/fs/squashfs/Config.in
> +++ b/fs/squashfs/Config.in
> @@ -89,4 +89,30 @@ config BR2_TARGET_ROOTFS_SQUASHFS4_ZSTD
>  
>  endchoice
>  
> +config BR2_TARGET_ROOTFS_SQUASHFS_EXTREME_COMP
> +	bool "extreme compression if possible"
> +	default BR2_TARGET_ROOTFS_SQUASHFS4_LZ4

This is technically correct, but we are not using that code style
anywhere else (that I am aware of), so I tweaked it to the more
tracitional form we use everywhere else. I also added a comment to
explain why it is:
    default y if BR2_TARGET_ROOTFS_SQUASHFS4_LZ4  # legacy

> +	help
> +	  Use options to increase compression ration as much as
> +	  possible, like using architecture-specific options, at
> +	  the cost of time when assembling the filesystem image.
> +
> +	  For example:
> +	   - with gzip and lzo, use -Xcompression-level 9
> +	   - with xz use a architecture specific bjc (branch-call-jump) filter
> +	   - with zstd use -Xcompression-level 22
> +	   - and more

    $ make check-package
    WARNING: fs/squashfs/Config.in:102: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)

> +config BR2_TARGET_ROOTFS_SQUASHFS_COMP_OPTS
> +	string
> +	depends on BR2_TARGET_ROOTFS_SQUASHFS_EXTREME_COMP
> +	default "-Xcompression-level 9" if BR2_TARGET_ROOTFS_SQUASHFS4_GZIP
> +	default "-Xcompression-level 9" if BR2_TARGET_ROOTFS_SQUASHFS4_LZO
> +	default "-Xhc" if BR2_TARGET_ROOTFS_SQUASHFS4_LZ4
> +	default "-Xbcj arm,armthumb" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_arm || BR_aarch64)
> +	default "-Xbcj powerpc" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_powerpc || BR2_powerpc64)
> +	default "-Xbcj sparc" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_sparc || BR2_sparc64)
> +	default "-Xbcj x86" if BR2_TARGET_ROOTFS_SQUASHFS4_XZ && (BR2_i386 || BR2_x86_64)
> +	default "-Xcompression-level 22" if BR2_TARGET_ROOTFS_SQUASHFS4_ZSTD

    $ make check-package
    WARNING: fs/squashfs/Config.in:109: attributes order: type, default, depends on, select, help (http://nightly.buildroot.org/#_config_files)

So I fixed that too.

>  endif
> diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk
> index 1790773f7e..51f8d0d7c9 100644
> --- a/fs/squashfs/squashfs.mk
> +++ b/fs/squashfs/squashfs.mk
> @@ -25,7 +25,7 @@ ROOTFS_SQUASHFS_ARGS += -nopad
>  endif
>  
>  ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZ4),y)
> -ROOTFS_SQUASHFS_ARGS += -comp lz4 -Xhc
> +ROOTFS_SQUASHFS_ARGS += -comp lz4
>  else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZO),y)
>  ROOTFS_SQUASHFS_ARGS += -comp lzo
>  else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZMA),y)
> @@ -38,6 +38,10 @@ else
>  ROOTFS_SQUASHFS_ARGS += -comp gzip
>  endif
>  
> +ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS_EXTREME_COMP),y)
> +ROOTFS_SQUASHFS_ARGS += $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_COMP_OPTS))
> +endif

If BR2_TARGET_ROOTFS_SQUASHFS_EXTREME_COMP is not set, or if no
condition match, then BR2_TARGET_ROOTFS_SQUASHFS_COMP_OPTS will be an
empty string, so we can just always expand it unconditionally.

Applied to master with the abov e fixed;
  - fix check-package
  - change the default code-style, add the legacy comment
  - always add the qstriped string, as it's empty when not used

Thanks!

Ditto, a test-case would be awesome!

Regards,
Yann E. MORIN.

>  define ROOTFS_SQUASHFS_CMD
>  	$(HOST_DIR)/bin/mksquashfs $(TARGET_DIR) $@ $(ROOTFS_SQUASHFS_ARGS)
>  endef
> -- 
> 2.35.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option
  2022-05-17  5:38 ` [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Yann E. MORIN
  2022-05-17  6:05   ` Yann E. MORIN
@ 2022-05-17 14:04   ` Linus via buildroot
  1 sibling, 0 replies; 6+ messages in thread
From: Linus via buildroot @ 2022-05-17 14:04 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot


[-- Attachment #1.1.1: Type: text/plain, Size: 5109 bytes --]

Hi Yann, All,

> ... there is no need to add it a second time based on the booleans.
Ups, that was a late-night oversight of me when changing the aproach
on the usage "4"-suffix and usage of booleans in the makefile. Had 2
repos (one clean for the patch and one work one to test it out on). I
seems I forgot to properly fixup the first patch draft of block size.
Thanks for fixing my mistake here!

Also didn't know about the check-package. Thanks for fixing that
as well! (I Should probably read more docs xD)

Because of the space I only put the value of "-b" into
BR2_TARGET_ROOTFS_SQUASHFS_BS to not need qsplit.
The usage of qsplit is therefore currently not necessary.
Maybe just put the "-b" also in the config and discard it
in the makefile for better clarity then. (commit 555f8dfd)

> Note that adding tests can be done in a followup patch, of course.

Will send a patch shortly for that.

Also, thanks for taking your time in testing, fixing and helping
me with these changes!

Greetings,
Linus Kaschulla

Am 17.05.22 um 07:38 schrieb Yann E. MORIN:
> Linus, All,
>
> On 2022-05-17 03:24 +0000, Linus Kaschulla spake thusly:
>> One advantage of squashfs over similar technologies is the support for
>> bigger block sizes. However the default size is not a lot bigger
>> (typically 128k if no `-b` flag specified).
>>
>> This patch adds the ability to select from common block sizes
>> which for example can aid in improving compression ratio.
>>
>> Signed-off-by: Linus Kaschulla <linus@cosmos-ink.net>
>> ---
>>   fs/squashfs/Config.in   | 50 +++++++++++++++++++++++++++++++++++++++++
>>   fs/squashfs/squashfs.mk | 14 +++++++++++-
>>   2 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in
>> index 70d4a20cf0..341e50a08c 100644
>> --- a/fs/squashfs/Config.in
>> +++ b/fs/squashfs/Config.in
>> @@ -5,6 +5,56 @@ config BR2_TARGET_ROOTFS_SQUASHFS
>>   
>>   if BR2_TARGET_ROOTFS_SQUASHFS
>>   
>> +choice
>> +	prompt "block size"
>> +	default BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
>> +	help
>> +	  Data block size. Bigger values can improve
>> +	  compression ratio.
>> +
>> +	  If unsure, leave at 128k (default).
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_4K
>> +	bool "4k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_8K
>> +	bool "8k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_16K
>> +	bool "16k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_32K
>> +	bool "32k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_64K
>> +	bool "64k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
>> +	bool "128k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_256K
>> +	bool "256k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_512K
>> +	bool "512k"
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS_1024K
>> +	bool "1024k"
>> +
>> +endchoice
>> +
>> +config BR2_TARGET_ROOTFS_SQUASHFS_BS
>> +	string
>> +	default "4K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_4K
>> +	default "8K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_84K
>> +	default "16K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_16K
>> +	default "32K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_32K
>> +	default "64K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_64K
>> +	default "128K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_128K
>> +	default "256K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_256K
>> +	default "512K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_512K
>> +	default "1024K" if BR2_TARGET_ROOTFS_SQUASHFS_BS_1024K
>> +
>>   config BR2_TARGET_ROOTFS_SQUASHFS_PAD
>>   	bool "pad to a 4K boundary"
>>   	default y # legacy was always ON
>> diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk
>> index 7a5e3e313e..1790773f7e 100644
>> --- a/fs/squashfs/squashfs.mk
>> +++ b/fs/squashfs/squashfs.mk
>> @@ -6,7 +6,19 @@
>>   
>>   ROOTFS_SQUASHFS_DEPENDENCIES = host-squashfs
>>   
>> -ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS)
>> +ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS) -b $(BR2_TARGET_ROOTFS_SQUASHFS_BS)
> Here you added the value to the option list using the hidden option
> string, so...
>
>> +ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_64K),y)
>> +ROOTFS_SQUASHFS_ARGS += -b 64K
>> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_128K),y)
>> +ROOTFS_SQUASHFS_ARGS += -b 128K
>> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_256K),y)
>> +ROOTFS_SQUASHFS_ARGS += -b 256K
>> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_512K),y)
>> +ROOTFS_SQUASHFS_ARGS += -b 512K
>> +else ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_BS_1024K),y)
>> +ROOTFS_SQUASHFS_ARGS += -b 1024K
>> +endif
> ... there is no need to add it a second time based on the booleans.
>
> Finally, it would have been nice to add two test cases that at least the
> 4K and 1024K options do work (the existing test tests the defaultm i.e
> 128K). Tests are in:  support/testing/tests/fs/test_squashfs.py
>
> Note that adding tests can be done in a followup patch, of course.
>
> Regards,
> Yann E. MORIN.
>
>>   ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS_PAD),)
>>   ROOTFS_SQUASHFS_ARGS += -nopad
>> -- 
>> 2.35.1
>>

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-17 14:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-17  3:24 [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Linus Kaschulla via buildroot
2022-05-17  3:24 ` [Buildroot] [PATCH 2/2] fs/squashfs: Add option to compress with extreme options Linus Kaschulla via buildroot
2022-05-17  6:09   ` Yann E. MORIN
2022-05-17  5:38 ` [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option Yann E. MORIN
2022-05-17  6:05   ` Yann E. MORIN
2022-05-17 14:04   ` Linus via buildroot

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.