Re: [Buildroot] [PATCH 1/2] fs/squashfs: Add block size option

["Yann E. MORIN" <yann.morin.1998@free.fr>]

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