Re: [PATCH] squashfs: Add optional full compressed block caching

[Phillip Lougher <phillip@squashfs.org.uk>]

On 02/05/2025 09:19, Chanho Min wrote:
> The commit 93e72b3c612adcaca1("squashfs: migrate from ll_rw_block usage to BIO")
> removed caching of compressed blocks in SquashFS, causing fio performance
> regression in workloads with repeated file reads. Without caching, every read

Please run scripts/checkpatch.pl on your patch.  It complains that the above
line is over 75 characters in length.

It also complains of DOS line endings.

> triggers disk I/O, severely impacting performance in tools like fio.
> 
> This patch introduces a new CONFIG_SQUASHFS_COMP_CACHE_FULL Kconfig option to
> enable caching of all compressed blocks, restoring performance to pre-BIO
> migration levels. When enabled, all pages in a BIO are cached in the page
> cache, reducing disk I/O for repeated reads. The fio test results with this
> patch confirm the performance restoration:
> 
> For example, fio tests (iodepth=1, numjobs=1, 
> ioengine=psync) show a notable performance restoration:
> 
> Disable CONFIG_SQUASHFS_COMP_CACHE_FULL:
>    IOPS=815, BW=102MiB/s (107MB/s)(6113MiB/60001msec)
> Enable CONFIG_SQUASHFS_COMP_CACHE_FULL:
>    IOPS=2223, BW=278MiB/s (291MB/s)(16.3GiB/59999msec)
> 
> The trade-off is increased memory usage due to caching all compressed blocks.
> The CONFIG_SQUASHFS_COMP_CACHE_FULL option allows users to enable this feature
> selectively, balancing performance and memory usage for workloads with frequent
> repeated reads.
> 
> Signed-off-by: Chanho Min <chanho.min@lge.com>
> ---
>   fs/squashfs/Kconfig | 21 +++++++++++++++++++++
>   fs/squashfs/block.c | 28 ++++++++++++++++++++++++++++
>   2 files changed, 49 insertions(+)
> 
> diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
> index 60fc98bdf421..cc5e4be58afa 100644
> --- a/fs/squashfs/Kconfig
> +++ b/fs/squashfs/Kconfig
> @@ -149,6 +149,27 @@ config SQUASHFS_XATTR
>   
>   	  If unsure, say N.
>   
> +config SQUASHFS_COMP_CACHE_FULL
> +	bool "Enable full caching of compressed blocks"
> +	depends on SQUASHFS
> +	default n
> +	help
> +	  This option enables caching of all compressed blocks, Without caching,
> +	  repeated reads of the same files trigger excessive disk I/O, significantly
> +	  reducinng performance in workloads like fio-based benchmarks.
> +
> +	  For example, fio tests (iodepth=1, numjobs=1, ioengine=psync) show:
> +	   Without caching: IOPS=2223, BW=278MiB/s (291MB/s)
> +	   With caching:    IOPS=815, BW=102MiB/s (107MB/s)
> +

I think the figures for caching and without caching are transposed above.

Apart from that the patch looks good.

Please send a V2 fixing the above.

Thanks

Phillip