* [PATCH 0/2] squashfs: Add the mount parameter "threads="
@ 2022-08-15 3:10 Xiaoming Ni
2022-08-15 3:10 ` [PATCH 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni
` (2 more replies)
0 siblings, 3 replies; 38+ messages in thread
From: Xiaoming Ni @ 2022-08-15 3:10 UTC (permalink / raw)
To: linux-kernel, phillip
Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3
Currently, Squashfs supports multiple decompressor parallel modes. However, this
mode can be configured only during kernel building and does not support flexible
selection during runtime.
In the current patch set, the mount parameter "threads=" is added to allow users
to select the parallel decompressor mode and configure the number of decompressors
when mounting a file system.
Xiaoming Ni (2):
squashfs: add the mount parameter theads=<single|multi|percpu>
squashfs: Allows users to configure the number of decompression
threads.
fs/squashfs/Kconfig | 24 ++++++++--
fs/squashfs/decompressor_multi.c | 32 ++++++++------
fs/squashfs/decompressor_multi_percpu.c | 37 ++++++++++------
fs/squashfs/decompressor_single.c | 23 ++++++----
fs/squashfs/squashfs.h | 39 ++++++++++++++---
fs/squashfs/squashfs_fs_sb.h | 4 +-
fs/squashfs/super.c | 77 ++++++++++++++++++++++++++++++++-
7 files changed, 191 insertions(+), 45 deletions(-)
--
2.12.3
^ permalink raw reply [flat|nested] 38+ messages in thread* [PATCH 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-08-15 3:10 [PATCH 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni @ 2022-08-15 3:10 ` Xiaoming Ni 2022-08-15 11:19 ` kernel test robot 2022-08-15 3:11 ` [PATCH 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2 siblings, 1 reply; 38+ messages in thread From: Xiaoming Ni @ 2022-08-15 3:10 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Squashfs supports three decompression concurrency modes: Single-thread mode: concurrent reads are blocked and the memory overhead is small. Multi-thread mode/percpu mode: reduces concurrent read blocking but increases memory overhead. The corresponding schema must be fixed at compile time. During mounting, the concurrent decompression mode cannot be adjusted based on file read blocking. The mount parameter theads=<single|multi|percpu> is added to select the concurrent decompression mode of a single SquashFS file system image. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 20 ++++++++++-- fs/squashfs/decompressor_multi.c | 28 +++++++++------- fs/squashfs/decompressor_multi_percpu.c | 37 +++++++++++++-------- fs/squashfs/decompressor_single.c | 23 ++++++++----- fs/squashfs/squashfs.h | 39 +++++++++++++++++++--- fs/squashfs/squashfs_fs_sb.h | 3 +- fs/squashfs/super.c | 58 ++++++++++++++++++++++++++++++++- 7 files changed, 166 insertions(+), 42 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 916e78fabcaa..af9f8c41f2de 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -55,7 +55,7 @@ config SQUASHFS_FILE_DIRECT endchoice choice - prompt "Decompressor parallelisation options" + prompt "default Decompressor parallelisation options" depends on SQUASHFS help Squashfs now supports three parallelisation options for @@ -64,8 +64,23 @@ choice If in doubt, select "Single threaded compression" + config SQUASHFS_DECOMP_DEFAULT_SINGLE + bool "Single threaded compression" + select SQUASHFS_DECOMP_SINGLE + + config SQUASHFS_DECOMP_DEFAULT_MULTI + bool "Use multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI + + config SQUASHFS_DECOMP_DEFAULT_PERCPU + bool "Use percpu multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI_PERCPU + +endchoice + config SQUASHFS_DECOMP_SINGLE bool "Single threaded compression" + depends on SQUASHFS help Traditionally Squashfs has used single-threaded decompression. Only one block (data or metadata) can be decompressed at any @@ -73,6 +88,7 @@ config SQUASHFS_DECOMP_SINGLE config SQUASHFS_DECOMP_MULTI bool "Use multiple decompressors for parallel I/O" + depends on SQUASHFS help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -87,6 +103,7 @@ config SQUASHFS_DECOMP_MULTI config SQUASHFS_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" + depends on SQUASHFS help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -96,7 +113,6 @@ config SQUASHFS_DECOMP_MULTI_PERCPU decompressor per core. It uses percpu variables to ensure decompression is load-balanced across the cores. -endchoice config SQUASHFS_XATTR bool "Squashfs XATTR support" diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index db9f12a3ea05..7b2723b77e75 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -29,13 +29,12 @@ #define MAX_DECOMPRESSOR (num_online_cpus() * 2) -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_multi(void) { return MAX_DECOMPRESSOR; } - -struct squashfs_stream { +struct squashfs_stream_multi { void *comp_opts; struct list_head strm_list; struct mutex mutex; @@ -51,7 +50,7 @@ struct decomp_stream { static void put_decomp_stream(struct decomp_stream *decomp_strm, - struct squashfs_stream *stream) + struct squashfs_stream_multi *stream) { mutex_lock(&stream->mutex); list_add(&decomp_strm->list, &stream->strm_list); @@ -59,10 +58,10 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, wake_up(&stream->wait); } -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_multi(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; + struct squashfs_stream_multi *stream; struct decomp_stream *decomp_strm = NULL; int err = -ENOMEM; @@ -103,9 +102,9 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_multi(struct squashfs_sb_info *msblk) { - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_multi *stream = msblk->stream; if (stream) { struct decomp_stream *decomp_strm; @@ -125,7 +124,7 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, - struct squashfs_stream *stream) + struct squashfs_stream_multi *stream) { struct decomp_stream *decomp_strm; @@ -180,12 +179,12 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_multi(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_multi *stream = msblk->stream; struct decomp_stream *decomp_stream = get_decomp_stream(msblk, stream); res = msblk->decompressor->decompress(msblk, decomp_stream->stream, bio, offset, length, output); @@ -195,3 +194,10 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, msblk->decompressor->name); return res; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { + .create = squashfs_decompressor_create_multi, + .destroy = squashfs_decompressor_destroy_multi, + .decompress = squashfs_decompress_multi, + .max_decompressors = squashfs_max_decompressors_multi, +}; diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c index b881b9283b7f..2d540bb13970 100644 --- a/fs/squashfs/decompressor_multi_percpu.c +++ b/fs/squashfs/decompressor_multi_percpu.c @@ -20,19 +20,19 @@ * variables, one thread per cpu core. */ -struct squashfs_stream { +struct squashfs_stream_percpu { void *stream; local_lock_t lock; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_percpu(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; - struct squashfs_stream __percpu *percpu; + struct squashfs_stream_percpu *stream; + struct squashfs_stream_percpu __percpu *percpu; int err, cpu; - percpu = alloc_percpu(struct squashfs_stream); + percpu = alloc_percpu(struct squashfs_stream_percpu); if (percpu == NULL) return ERR_PTR(-ENOMEM); @@ -59,11 +59,11 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_percpu(struct squashfs_sb_info *msblk) { - struct squashfs_stream __percpu *percpu = - (struct squashfs_stream __percpu *) msblk->stream; - struct squashfs_stream *stream; + struct squashfs_stream_percpu __percpu *percpu = + (struct squashfs_stream_percpu __percpu *) msblk->stream; + struct squashfs_stream_percpu *stream; int cpu; if (msblk->stream) { @@ -75,19 +75,21 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { - struct squashfs_stream *stream; + struct squashfs_stream_percpu *stream; int res; - local_lock(&msblk->stream->lock); + preempt_disable(); stream = this_cpu_ptr(msblk->stream); + local_lock(&stream->lock); res = msblk->decompressor->decompress(msblk, stream->stream, bio, offset, length, output); - local_unlock(&msblk->stream->lock); + local_unlock(&stream->lock); + preempt_enable(); if (res < 0) ERROR("%s decompression failed, data probably corrupt\n", @@ -96,7 +98,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_percpu(void) { return num_possible_cpus(); } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { + .create = squashfs_decompressor_create_percpu, + .destroy = squashfs_decompressor_destroy_percpu, + .decompress = squashfs_decompress_percpu, + .max_decompressors = squashfs_max_decompressors_percpu, +}; diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c index 4eb3d083d45e..41449de0ea4c 100644 --- a/fs/squashfs/decompressor_single.c +++ b/fs/squashfs/decompressor_single.c @@ -19,15 +19,15 @@ * decompressor framework */ -struct squashfs_stream { +struct squashfs_stream_single { void *stream; struct mutex mutex; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_single(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; + struct squashfs_stream_single *stream; int err = -ENOMEM; stream = kmalloc(sizeof(*stream), GFP_KERNEL); @@ -49,9 +49,9 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_single(struct squashfs_sb_info *msblk) { - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_single *stream = msblk->stream; if (stream) { msblk->decompressor->free(stream->stream); @@ -59,12 +59,12 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_single(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_single *stream = msblk->stream; mutex_lock(&stream->mutex); res = msblk->decompressor->decompress(msblk, stream->stream, bio, @@ -78,7 +78,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_single(void) { return 1; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { + .create = squashfs_decompressor_create_single, + .destroy = squashfs_decompressor_destroy_single, + .decompress = squashfs_decompress_single, + .max_decompressors = squashfs_max_decompressors_single, +}; diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h index 9783e01c8100..514cba134f11 100644 --- a/fs/squashfs/squashfs.h +++ b/fs/squashfs/squashfs.h @@ -38,11 +38,40 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); /* decompressor_xxx.c */ -extern void *squashfs_decompressor_create(struct squashfs_sb_info *, void *); -extern void squashfs_decompressor_destroy(struct squashfs_sb_info *); -extern int squashfs_decompress(struct squashfs_sb_info *, struct bio *, - int, int, struct squashfs_page_actor *); -extern int squashfs_max_decompressors(void); + +struct squashfs_decompressor_thread_ops { + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); + void (*destroy)(struct squashfs_sb_info *msblk); + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output); + int (*max_decompressors)(void); +}; + +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; +#endif + +static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) +{ + return msblk->thread_ops->create(msblk, comp_opts); +} + +static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +{ + msblk->thread_ops->destroy(msblk); +} + +static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output) +{ + return msblk->thread_ops->decompress(msblk, bio, offset, length, output); +} /* export.c */ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index 1e90c2575f9b..f1e5dad8ae0a 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -53,7 +53,7 @@ struct squashfs_sb_info { __le64 *xattr_id_table; struct mutex meta_index_mutex; struct meta_index *meta_index; - struct squashfs_stream *stream; + void *stream; __le64 *inode_lookup_table; u64 inode_table; u64 directory_table; @@ -66,5 +66,6 @@ struct squashfs_sb_info { int xattr_ids; unsigned int ids; bool panic_on_errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 32565dafa7f3..6fd44683a84b 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -47,10 +47,12 @@ enum Opt_errors { enum squashfs_param { Opt_errors, + Opt_threads, }; struct squashfs_mount_opts { enum Opt_errors errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; static const struct constant_table squashfs_param_errors[] = { @@ -61,9 +63,33 @@ static const struct constant_table squashfs_param_errors[] = { static const struct fs_parameter_spec squashfs_fs_parameters[] = { fsparam_enum("errors", Opt_errors, squashfs_param_errors), + fsparam_string("threads", Opt_threads), {} }; +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (strcmp(str, "single") == 0) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + if (strcmp(str, "multi") == 0) { + opts->thread_ops = &squashfs_decompressor_multi; + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + if (strcmp(str, "percpu") == 0) { + opts->thread_ops = &squashfs_decompressor_percpu; + return 0; + } +#endif + return -EINVAL; +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -78,6 +104,10 @@ static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *para case Opt_errors: opts->errors = result.uint_32; break; + case Opt_threads: + if (squashfs_parse_param_threads(param->string, opts) != 0) + return -EINVAL; + break; default: return -EINVAL; } @@ -167,6 +197,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_bdev); goto failed_mount; } + msblk->thread_ops = opts->thread_ops; /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -252,7 +283,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - squashfs_max_decompressors(), msblk->block_size); + msblk->thread_ops->max_decompressors(), msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -435,6 +466,24 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + if (msblk->thread_ops == &squashfs_decompressor_multi) { + seq_puts(s, ",threads=multi"); + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + if (msblk->thread_ops == &squashfs_decompressor_percpu) { + seq_puts(s, ",threads=percpu"); + return 0; + } +#endif return 0; } @@ -446,6 +495,13 @@ static int squashfs_init_fs_context(struct fs_context *fc) if (!opts) return -ENOMEM; +#ifdef CONFIG_SQUASHFS_DECOMP_DEFAULT_SINGLE + opts->thread_ops = &squashfs_decompressor_single; +#elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_MULTI) + opts->thread_ops = &squashfs_decompressor_multi, +#elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) + opts->thread_ops = &squashfs_decompressor_percpu; +#endif fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-08-15 3:10 ` [PATCH 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-08-15 11:19 ` kernel test robot 0 siblings, 0 replies; 38+ messages in thread From: kernel test robot @ 2022-08-15 11:19 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel, phillip Cc: kbuild-all, nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Hi Xiaoming, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on v6.0-rc1] [also build test WARNING on linus/master next-20220815] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Xiaoming-Ni/squashfs-Add-the-mount-parameter-threads/20220815-111318 base: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868 config: sparc-randconfig-s042-20220814 (https://download.01.org/0day-ci/archive/20220815/202208151921.Y0vJnA6c-lkp@intel.com/config) compiler: sparc-linux-gcc (GCC) 12.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.4-39-gce1a6720-dirty # https://github.com/intel-lab-lkp/linux/commit/dcb72eaed7732d884148ce37d23442956296d0d6 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Xiaoming-Ni/squashfs-Add-the-mount-parameter-threads/20220815-111318 git checkout dcb72eaed7732d884148ce37d23442956296d0d6 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sparc SHELL=/bin/bash fs/squashfs/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> fs/squashfs/decompressor_multi_percpu.c:85:18: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] __percpu *__vpp_verify @@ got void * @@ fs/squashfs/decompressor_multi_percpu.c:85:18: sparse: expected void const [noderef] __percpu *__vpp_verify fs/squashfs/decompressor_multi_percpu.c:85:18: sparse: got void * fs/squashfs/decompressor_multi_percpu.c:86:9: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] __percpu *__vpp_verify @@ got struct local_lock_t * @@ fs/squashfs/decompressor_multi_percpu.c:86:9: sparse: expected void const [noderef] __percpu *__vpp_verify fs/squashfs/decompressor_multi_percpu.c:86:9: sparse: got struct local_lock_t * fs/squashfs/decompressor_multi_percpu.c:91:9: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] __percpu *__vpp_verify @@ got struct local_lock_t * @@ fs/squashfs/decompressor_multi_percpu.c:91:9: sparse: expected void const [noderef] __percpu *__vpp_verify fs/squashfs/decompressor_multi_percpu.c:91:9: sparse: got struct local_lock_t * vim +85 fs/squashfs/decompressor_multi_percpu.c d208383d640727 Phillip Lougher 2013-11-18 77 dcb72eaed7732d Xiaoming Ni 2022-08-15 78 static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio *bio, 93e72b3c612adc Philippe Liard 2020-06-01 79 int offset, int length, struct squashfs_page_actor *output) d208383d640727 Phillip Lougher 2013-11-18 80 { dcb72eaed7732d Xiaoming Ni 2022-08-15 81 struct squashfs_stream_percpu *stream; fd56200a16c72c Julia Cartwright 2020-05-27 82 int res; fd56200a16c72c Julia Cartwright 2020-05-27 83 dcb72eaed7732d Xiaoming Ni 2022-08-15 84 preempt_disable(); fd56200a16c72c Julia Cartwright 2020-05-27 @85 stream = this_cpu_ptr(msblk->stream); dcb72eaed7732d Xiaoming Ni 2022-08-15 86 local_lock(&stream->lock); fd56200a16c72c Julia Cartwright 2020-05-27 87 93e72b3c612adc Philippe Liard 2020-06-01 88 res = msblk->decompressor->decompress(msblk, stream->stream, bio, 846b730e99518a Phillip Lougher 2013-11-18 89 offset, length, output); fd56200a16c72c Julia Cartwright 2020-05-27 90 dcb72eaed7732d Xiaoming Ni 2022-08-15 91 local_unlock(&stream->lock); dcb72eaed7732d Xiaoming Ni 2022-08-15 92 preempt_enable(); d208383d640727 Phillip Lougher 2013-11-18 93 d208383d640727 Phillip Lougher 2013-11-18 94 if (res < 0) d208383d640727 Phillip Lougher 2013-11-18 95 ERROR("%s decompression failed, data probably corrupt\n", d208383d640727 Phillip Lougher 2013-11-18 96 msblk->decompressor->name); d208383d640727 Phillip Lougher 2013-11-18 97 d208383d640727 Phillip Lougher 2013-11-18 98 return res; d208383d640727 Phillip Lougher 2013-11-18 99 } d208383d640727 Phillip Lougher 2013-11-18 100 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 2/2] squashfs: Allows users to configure the number of decompression threads. 2022-08-15 3:10 [PATCH 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-08-15 3:10 ` [PATCH 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-08-15 3:11 ` Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-08-15 3:11 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 The maximum number of threads in the decompressor_multi.c file is fixed and cannot be adjusted according to user needs. Therefore, the mount parameter needs to be added to allow users to configure the number of threads as required. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 4 +++- fs/squashfs/decompressor_multi.c | 4 ++-- fs/squashfs/squashfs_fs_sb.h | 1 + fs/squashfs/super.c | 45 ++++++++++++++++++++++++++++------------ 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index af9f8c41f2de..3f42b4099dc7 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -99,7 +99,9 @@ config SQUASHFS_DECOMP_MULTI This decompressor implementation uses up to two parallel decompressors per core. It dynamically allocates decompressors - on a demand basis. + on a demand basis. You can also set the number of threads by setting threads=<number> of + the mount parameter. + config SQUASHFS_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index 7b2723b77e75..6d1cea325cca 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -144,7 +144,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, * If there is no available decomp and already full, * let's wait for releasing decomp from other users. */ - if (stream->avail_decomp >= MAX_DECOMPRESSOR) + if (stream->avail_decomp >= msblk->max_thread_num) goto wait; /* Let's allocate new decomp */ @@ -160,7 +160,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } stream->avail_decomp++; - WARN_ON(stream->avail_decomp > MAX_DECOMPRESSOR); + WARN_ON(stream->avail_decomp > msblk->max_thread_num); mutex_unlock(&stream->mutex); break; diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index f1e5dad8ae0a..659082e9e51d 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -67,5 +67,6 @@ struct squashfs_sb_info { unsigned int ids; bool panic_on_errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int max_thread_num; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 6fd44683a84b..f3533e20d0b7 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -53,6 +53,7 @@ enum squashfs_param { struct squashfs_mount_opts { enum Opt_errors errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int thread_num; }; static const struct constant_table squashfs_param_errors[] = { @@ -69,6 +70,8 @@ static const struct fs_parameter_spec squashfs_fs_parameters[] = { static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) { + unsigned long num; + int ret; #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE if (strcmp(str, "single") == 0) { opts->thread_ops = &squashfs_decompressor_single; @@ -87,7 +90,24 @@ static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_o return 0; } #endif + ret = kstrtoul(str, 0, &num); + if (ret != 0 || num == 0) + return -EINVAL; +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (num == 1) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + num = opts->thread_ops->max_decompressors(); + opts->thread_num = (int)num; + return 0; +#else return -EINVAL; +#endif } static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) @@ -198,6 +218,11 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) goto failed_mount; } msblk->thread_ops = opts->thread_ops; + if (opts->thread_num == 0) { + msblk->max_thread_num = opts->thread_ops->max_decompressors(); + } else { + msblk->max_thread_num = opts->thread_num; + } /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -283,7 +308,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - msblk->thread_ops->max_decompressors(), msblk->block_size); + msblk->max_thread_num, msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -466,24 +491,17 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); -#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE - if (msblk->thread_ops == &squashfs_decompressor_single) { - seq_puts(s, ",threads=single"); - return 0; - } -#endif -#ifdef CONFIG_SQUASHFS_DECOMP_MULTI - if (msblk->thread_ops == &squashfs_decompressor_multi) { - seq_puts(s, ",threads=multi"); - return 0; - } -#endif #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + /* + * thread=percpu and thread=<number> have different configuration effects. + * the parameters need to be treated differently when they are displayed. + */ if (msblk->thread_ops == &squashfs_decompressor_percpu) { seq_puts(s, ",threads=percpu"); return 0; } #endif + seq_printf(s, ",threads=%u", msblk->max_thread_num); return 0; } @@ -502,6 +520,7 @@ static int squashfs_init_fs_context(struct fs_context *fc) #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) opts->thread_ops = &squashfs_decompressor_percpu; #endif + opts->thread_num = 0; fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" 2022-08-15 3:10 [PATCH 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-08-15 3:10 ` [PATCH 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-08-15 3:11 ` [PATCH 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni @ 2022-08-16 1:00 ` Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni ` (3 more replies) 2 siblings, 4 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-08-16 1:00 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Currently, Squashfs supports multiple decompressor parallel modes. However, this mode can be configured only during kernel building and does not support flexible selection during runtime. In the current patch set, the mount parameter "threads=" is added to allow users to select the parallel decompressor mode and configure the number of decompressors when mounting a file system. v2: fix warning: sparse: incorrect type in initializer (different address spaces) Reported-by: kernel test robot <lkp@intel.com> v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ ---- Xiaoming Ni (2): squashfs: add the mount parameter theads=<single|multi|percpu> squashfs: Allows users to configure the number of decompression threads. fs/squashfs/Kconfig | 24 ++++++++-- fs/squashfs/decompressor_multi.c | 32 ++++++++------ fs/squashfs/decompressor_multi_percpu.c | 39 ++++++++++------- fs/squashfs/decompressor_single.c | 23 ++++++---- fs/squashfs/squashfs.h | 39 ++++++++++++++--- fs/squashfs/squashfs_fs_sb.h | 4 +- fs/squashfs/super.c | 77 ++++++++++++++++++++++++++++++++- 7 files changed, 192 insertions(+), 46 deletions(-) -- 2.12.3 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-08-16 1:00 ` [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni @ 2022-08-16 1:00 ` Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni ` (2 subsequent siblings) 3 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-08-16 1:00 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Squashfs supports three decompression concurrency modes: Single-thread mode: concurrent reads are blocked and the memory overhead is small. Multi-thread mode/percpu mode: reduces concurrent read blocking but increases memory overhead. The corresponding schema must be fixed at compile time. During mounting, the concurrent decompression mode cannot be adjusted based on file read blocking. The mount parameter theads=<single|multi|percpu> is added to select the concurrent decompression mode of a single SquashFS file system image. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 20 ++++++++++-- fs/squashfs/decompressor_multi.c | 28 +++++++++------- fs/squashfs/decompressor_multi_percpu.c | 39 +++++++++++++--------- fs/squashfs/decompressor_single.c | 23 ++++++++----- fs/squashfs/squashfs.h | 39 +++++++++++++++++++--- fs/squashfs/squashfs_fs_sb.h | 3 +- fs/squashfs/super.c | 58 ++++++++++++++++++++++++++++++++- 7 files changed, 167 insertions(+), 43 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 916e78fabcaa..af9f8c41f2de 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -55,7 +55,7 @@ config SQUASHFS_FILE_DIRECT endchoice choice - prompt "Decompressor parallelisation options" + prompt "default Decompressor parallelisation options" depends on SQUASHFS help Squashfs now supports three parallelisation options for @@ -64,8 +64,23 @@ choice If in doubt, select "Single threaded compression" + config SQUASHFS_DECOMP_DEFAULT_SINGLE + bool "Single threaded compression" + select SQUASHFS_DECOMP_SINGLE + + config SQUASHFS_DECOMP_DEFAULT_MULTI + bool "Use multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI + + config SQUASHFS_DECOMP_DEFAULT_PERCPU + bool "Use percpu multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI_PERCPU + +endchoice + config SQUASHFS_DECOMP_SINGLE bool "Single threaded compression" + depends on SQUASHFS help Traditionally Squashfs has used single-threaded decompression. Only one block (data or metadata) can be decompressed at any @@ -73,6 +88,7 @@ config SQUASHFS_DECOMP_SINGLE config SQUASHFS_DECOMP_MULTI bool "Use multiple decompressors for parallel I/O" + depends on SQUASHFS help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -87,6 +103,7 @@ config SQUASHFS_DECOMP_MULTI config SQUASHFS_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" + depends on SQUASHFS help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -96,7 +113,6 @@ config SQUASHFS_DECOMP_MULTI_PERCPU decompressor per core. It uses percpu variables to ensure decompression is load-balanced across the cores. -endchoice config SQUASHFS_XATTR bool "Squashfs XATTR support" diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index db9f12a3ea05..7b2723b77e75 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -29,13 +29,12 @@ #define MAX_DECOMPRESSOR (num_online_cpus() * 2) -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_multi(void) { return MAX_DECOMPRESSOR; } - -struct squashfs_stream { +struct squashfs_stream_multi { void *comp_opts; struct list_head strm_list; struct mutex mutex; @@ -51,7 +50,7 @@ struct decomp_stream { static void put_decomp_stream(struct decomp_stream *decomp_strm, - struct squashfs_stream *stream) + struct squashfs_stream_multi *stream) { mutex_lock(&stream->mutex); list_add(&decomp_strm->list, &stream->strm_list); @@ -59,10 +58,10 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, wake_up(&stream->wait); } -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_multi(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; + struct squashfs_stream_multi *stream; struct decomp_stream *decomp_strm = NULL; int err = -ENOMEM; @@ -103,9 +102,9 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_multi(struct squashfs_sb_info *msblk) { - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_multi *stream = msblk->stream; if (stream) { struct decomp_stream *decomp_strm; @@ -125,7 +124,7 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, - struct squashfs_stream *stream) + struct squashfs_stream_multi *stream) { struct decomp_stream *decomp_strm; @@ -180,12 +179,12 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_multi(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_multi *stream = msblk->stream; struct decomp_stream *decomp_stream = get_decomp_stream(msblk, stream); res = msblk->decompressor->decompress(msblk, decomp_stream->stream, bio, offset, length, output); @@ -195,3 +194,10 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, msblk->decompressor->name); return res; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { + .create = squashfs_decompressor_create_multi, + .destroy = squashfs_decompressor_destroy_multi, + .decompress = squashfs_decompress_multi, + .max_decompressors = squashfs_max_decompressors_multi, +}; diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c index b881b9283b7f..e24455a7b04d 100644 --- a/fs/squashfs/decompressor_multi_percpu.c +++ b/fs/squashfs/decompressor_multi_percpu.c @@ -20,19 +20,19 @@ * variables, one thread per cpu core. */ -struct squashfs_stream { +struct squashfs_stream_percpu { void *stream; local_lock_t lock; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_percpu(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; - struct squashfs_stream __percpu *percpu; + struct squashfs_stream_percpu *stream; + struct squashfs_stream_percpu __percpu *percpu; int err, cpu; - percpu = alloc_percpu(struct squashfs_stream); + percpu = alloc_percpu(struct squashfs_stream_percpu); if (percpu == NULL) return ERR_PTR(-ENOMEM); @@ -59,11 +59,11 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_percpu(struct squashfs_sb_info *msblk) { - struct squashfs_stream __percpu *percpu = - (struct squashfs_stream __percpu *) msblk->stream; - struct squashfs_stream *stream; + struct squashfs_stream_percpu __percpu *percpu = + (struct squashfs_stream_percpu __percpu *) msblk->stream; + struct squashfs_stream_percpu *stream; int cpu; if (msblk->stream) { @@ -75,19 +75,21 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { - struct squashfs_stream *stream; + struct squashfs_stream_percpu *stream; + struct squashfs_stream_percpu __percpu *percpu = + (struct squashfs_stream_percpu __percpu *) msblk->stream; int res; - local_lock(&msblk->stream->lock); - stream = this_cpu_ptr(msblk->stream); + local_lock(&percpu->lock); + stream = this_cpu_ptr(percpu); res = msblk->decompressor->decompress(msblk, stream->stream, bio, offset, length, output); - local_unlock(&msblk->stream->lock); + local_unlock(&percpu->lock); if (res < 0) ERROR("%s decompression failed, data probably corrupt\n", @@ -96,7 +98,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_percpu(void) { return num_possible_cpus(); } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { + .create = squashfs_decompressor_create_percpu, + .destroy = squashfs_decompressor_destroy_percpu, + .decompress = squashfs_decompress_percpu, + .max_decompressors = squashfs_max_decompressors_percpu, +}; diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c index 4eb3d083d45e..41449de0ea4c 100644 --- a/fs/squashfs/decompressor_single.c +++ b/fs/squashfs/decompressor_single.c @@ -19,15 +19,15 @@ * decompressor framework */ -struct squashfs_stream { +struct squashfs_stream_single { void *stream; struct mutex mutex; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_single(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; + struct squashfs_stream_single *stream; int err = -ENOMEM; stream = kmalloc(sizeof(*stream), GFP_KERNEL); @@ -49,9 +49,9 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_single(struct squashfs_sb_info *msblk) { - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_single *stream = msblk->stream; if (stream) { msblk->decompressor->free(stream->stream); @@ -59,12 +59,12 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_single(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_single *stream = msblk->stream; mutex_lock(&stream->mutex); res = msblk->decompressor->decompress(msblk, stream->stream, bio, @@ -78,7 +78,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_single(void) { return 1; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { + .create = squashfs_decompressor_create_single, + .destroy = squashfs_decompressor_destroy_single, + .decompress = squashfs_decompress_single, + .max_decompressors = squashfs_max_decompressors_single, +}; diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h index 9783e01c8100..514cba134f11 100644 --- a/fs/squashfs/squashfs.h +++ b/fs/squashfs/squashfs.h @@ -38,11 +38,40 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); /* decompressor_xxx.c */ -extern void *squashfs_decompressor_create(struct squashfs_sb_info *, void *); -extern void squashfs_decompressor_destroy(struct squashfs_sb_info *); -extern int squashfs_decompress(struct squashfs_sb_info *, struct bio *, - int, int, struct squashfs_page_actor *); -extern int squashfs_max_decompressors(void); + +struct squashfs_decompressor_thread_ops { + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); + void (*destroy)(struct squashfs_sb_info *msblk); + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output); + int (*max_decompressors)(void); +}; + +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; +#endif + +static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) +{ + return msblk->thread_ops->create(msblk, comp_opts); +} + +static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +{ + msblk->thread_ops->destroy(msblk); +} + +static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output) +{ + return msblk->thread_ops->decompress(msblk, bio, offset, length, output); +} /* export.c */ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index 1e90c2575f9b..f1e5dad8ae0a 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -53,7 +53,7 @@ struct squashfs_sb_info { __le64 *xattr_id_table; struct mutex meta_index_mutex; struct meta_index *meta_index; - struct squashfs_stream *stream; + void *stream; __le64 *inode_lookup_table; u64 inode_table; u64 directory_table; @@ -66,5 +66,6 @@ struct squashfs_sb_info { int xattr_ids; unsigned int ids; bool panic_on_errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 32565dafa7f3..6fd44683a84b 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -47,10 +47,12 @@ enum Opt_errors { enum squashfs_param { Opt_errors, + Opt_threads, }; struct squashfs_mount_opts { enum Opt_errors errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; static const struct constant_table squashfs_param_errors[] = { @@ -61,9 +63,33 @@ static const struct constant_table squashfs_param_errors[] = { static const struct fs_parameter_spec squashfs_fs_parameters[] = { fsparam_enum("errors", Opt_errors, squashfs_param_errors), + fsparam_string("threads", Opt_threads), {} }; +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (strcmp(str, "single") == 0) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + if (strcmp(str, "multi") == 0) { + opts->thread_ops = &squashfs_decompressor_multi; + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + if (strcmp(str, "percpu") == 0) { + opts->thread_ops = &squashfs_decompressor_percpu; + return 0; + } +#endif + return -EINVAL; +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -78,6 +104,10 @@ static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *para case Opt_errors: opts->errors = result.uint_32; break; + case Opt_threads: + if (squashfs_parse_param_threads(param->string, opts) != 0) + return -EINVAL; + break; default: return -EINVAL; } @@ -167,6 +197,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_bdev); goto failed_mount; } + msblk->thread_ops = opts->thread_ops; /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -252,7 +283,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - squashfs_max_decompressors(), msblk->block_size); + msblk->thread_ops->max_decompressors(), msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -435,6 +466,24 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + if (msblk->thread_ops == &squashfs_decompressor_multi) { + seq_puts(s, ",threads=multi"); + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + if (msblk->thread_ops == &squashfs_decompressor_percpu) { + seq_puts(s, ",threads=percpu"); + return 0; + } +#endif return 0; } @@ -446,6 +495,13 @@ static int squashfs_init_fs_context(struct fs_context *fc) if (!opts) return -ENOMEM; +#ifdef CONFIG_SQUASHFS_DECOMP_DEFAULT_SINGLE + opts->thread_ops = &squashfs_decompressor_single; +#elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_MULTI) + opts->thread_ops = &squashfs_decompressor_multi, +#elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) + opts->thread_ops = &squashfs_decompressor_percpu; +#endif fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v2 2/2] squashfs: Allows users to configure the number of decompression threads. 2022-08-16 1:00 ` [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-08-16 1:00 ` Xiaoming Ni 2022-08-26 6:19 ` ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 " Xiaoming Ni 3 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-08-16 1:00 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 The maximum number of threads in the decompressor_multi.c file is fixed and cannot be adjusted according to user needs. Therefore, the mount parameter needs to be added to allow users to configure the number of threads as required. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 4 +++- fs/squashfs/decompressor_multi.c | 4 ++-- fs/squashfs/squashfs_fs_sb.h | 1 + fs/squashfs/super.c | 45 ++++++++++++++++++++++++++++------------ 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index af9f8c41f2de..3f42b4099dc7 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -99,7 +99,9 @@ config SQUASHFS_DECOMP_MULTI This decompressor implementation uses up to two parallel decompressors per core. It dynamically allocates decompressors - on a demand basis. + on a demand basis. You can also set the number of threads by setting threads=<number> of + the mount parameter. + config SQUASHFS_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index 7b2723b77e75..6d1cea325cca 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -144,7 +144,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, * If there is no available decomp and already full, * let's wait for releasing decomp from other users. */ - if (stream->avail_decomp >= MAX_DECOMPRESSOR) + if (stream->avail_decomp >= msblk->max_thread_num) goto wait; /* Let's allocate new decomp */ @@ -160,7 +160,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } stream->avail_decomp++; - WARN_ON(stream->avail_decomp > MAX_DECOMPRESSOR); + WARN_ON(stream->avail_decomp > msblk->max_thread_num); mutex_unlock(&stream->mutex); break; diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index f1e5dad8ae0a..659082e9e51d 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -67,5 +67,6 @@ struct squashfs_sb_info { unsigned int ids; bool panic_on_errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int max_thread_num; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 6fd44683a84b..f3533e20d0b7 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -53,6 +53,7 @@ enum squashfs_param { struct squashfs_mount_opts { enum Opt_errors errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int thread_num; }; static const struct constant_table squashfs_param_errors[] = { @@ -69,6 +70,8 @@ static const struct fs_parameter_spec squashfs_fs_parameters[] = { static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) { + unsigned long num; + int ret; #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE if (strcmp(str, "single") == 0) { opts->thread_ops = &squashfs_decompressor_single; @@ -87,7 +90,24 @@ static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_o return 0; } #endif + ret = kstrtoul(str, 0, &num); + if (ret != 0 || num == 0) + return -EINVAL; +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (num == 1) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + num = opts->thread_ops->max_decompressors(); + opts->thread_num = (int)num; + return 0; +#else return -EINVAL; +#endif } static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) @@ -198,6 +218,11 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) goto failed_mount; } msblk->thread_ops = opts->thread_ops; + if (opts->thread_num == 0) { + msblk->max_thread_num = opts->thread_ops->max_decompressors(); + } else { + msblk->max_thread_num = opts->thread_num; + } /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -283,7 +308,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - msblk->thread_ops->max_decompressors(), msblk->block_size); + msblk->max_thread_num, msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -466,24 +491,17 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); -#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE - if (msblk->thread_ops == &squashfs_decompressor_single) { - seq_puts(s, ",threads=single"); - return 0; - } -#endif -#ifdef CONFIG_SQUASHFS_DECOMP_MULTI - if (msblk->thread_ops == &squashfs_decompressor_multi) { - seq_puts(s, ",threads=multi"); - return 0; - } -#endif #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + /* + * thread=percpu and thread=<number> have different configuration effects. + * the parameters need to be treated differently when they are displayed. + */ if (msblk->thread_ops == &squashfs_decompressor_percpu) { seq_puts(s, ",threads=percpu"); return 0; } #endif + seq_printf(s, ",threads=%u", msblk->max_thread_num); return 0; } @@ -502,6 +520,7 @@ static int squashfs_init_fs_context(struct fs_context *fc) #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) opts->thread_ops = &squashfs_decompressor_percpu; #endif + opts->thread_num = 0; fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" 2022-08-16 1:00 ` [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni @ 2022-08-26 6:19 ` Xiaoming Ni 2022-08-28 23:18 ` Phillip Lougher 2022-09-02 9:48 ` [PATCH v3 " Xiaoming Ni 3 siblings, 1 reply; 38+ messages in thread From: Xiaoming Ni @ 2022-08-26 6:19 UTC (permalink / raw) To: linux-kernel, phillip Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 ping On 2022/8/16 9:00, Xiaoming Ni wrote: > Currently, Squashfs supports multiple decompressor parallel modes. However, this > mode can be configured only during kernel building and does not support flexible > selection during runtime. > > In the current patch set, the mount parameter "threads=" is added to allow users > to select the parallel decompressor mode and configure the number of decompressors > when mounting a file system. > > v2: fix warning: sparse: incorrect type in initializer (different address spaces) > Reported-by: kernel test robot <lkp@intel.com> > > v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ > ---- > > Xiaoming Ni (2): > squashfs: add the mount parameter theads=<single|multi|percpu> > squashfs: Allows users to configure the number of decompression > threads. > > fs/squashfs/Kconfig | 24 ++++++++-- > fs/squashfs/decompressor_multi.c | 32 ++++++++------ > fs/squashfs/decompressor_multi_percpu.c | 39 ++++++++++------- > fs/squashfs/decompressor_single.c | 23 ++++++---- > fs/squashfs/squashfs.h | 39 ++++++++++++++--- > fs/squashfs/squashfs_fs_sb.h | 4 +- > fs/squashfs/super.c | 77 ++++++++++++++++++++++++++++++++- > 7 files changed, 192 insertions(+), 46 deletions(-) > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" 2022-08-26 6:19 ` ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni @ 2022-08-28 23:18 ` Phillip Lougher 2022-08-30 13:38 ` Xiaoming Ni 0 siblings, 1 reply; 38+ messages in thread From: Phillip Lougher @ 2022-08-28 23:18 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 26/08/2022 07:19, Xiaoming Ni wrote: > ping > > > On 2022/8/16 9:00, Xiaoming Ni wrote: >> Currently, Squashfs supports multiple decompressor parallel modes. >> However, this >> mode can be configured only during kernel building and does not >> support flexible >> selection during runtime. >> >> In the current patch set, the mount parameter "threads=" is added to >> allow users >> to select the parallel decompressor mode and configure the number of >> decompressors >> when mounting a file system. >> >> v2: fix warning: sparse: incorrect type in initializer (different >> address spaces) >> Reported-by: kernel test robot <lkp@intel.com> I have made an initial review of the patches, and I have the following comments. Good things about the patch-series. 1. In principle I have no objections to making this configurable at mount time. But, a use-case for why this has become necessary would help in the evaluation. 2. In general the code changes are good. They are predominantly exposing the existing different decompressor functionality into structures which can be selected at mount time. They do not change existing functionality, and so there are no issues about unexpected regressions. Things which I don't like about the patch-series. 1. There is no default kernel configuration option to keep the existing behaviour, that is build time selectable only. There may be many companies/people where for "security" reasons the ability to switch to a more CPU/memory intensive decompressor or more threads is a risk. Yes, I know the new kernel configuration options allow only the selected default decompressor mode to be built. In theory that will restrict the available decompressors to the single decompressor selected at build time. So not much different to the current position? But, if the CONFIG_SQUASHFS_DECOMP_MULTI decompressor is selected, that will now allow more threads to be used than is current, where it is currently restricted to num_online_cpus() * 2. 2. You have decided to allow the mutiple decompressor implementations to be selected at mount time - but you have also allowed only one decompressor to be built at kernel build time. This means you end up in the fairly silly situation of having a mount time option which allows the user to select between one decompressor. There doesn't seem much point in having an option which allows nothing to be changed. 3. Using thread=<number>, where thread=1 you use SQUASHFS_DECOMP_SINGLE if it has been built, otherwise you fall back to SQUASHFS_DECOMP_MULTI. This meants the effect of thread=1 is indeterminate and depends on the build options. I would suggest thread=1 should always mean use SQUASHFS_DECOMP_SINGLE. 4. If SQUASHFS_DECOMP_MULTI is selected, there isn't a limit on the maximum amount of threads allowed, and there is no ability to set the maximum number of threads allowed at kernel build time either. All of the above seems to be a bit of a mess. As regards points 1 - 3, personally I would add a default kernel configuration option that keeps the existing behaviour, build time selectable only, no additional mount time options. Then a kernel configuration option that allows the different decompressors to be selected at mount time, but which always builds all the decompressors. This will avoid the silliness of point 2, and the indeterminate behaviour of point 3. As regards point 4, I think you should require the maximum number of threads allowable to be determined at build time, this is good for security and avoids attempts to use too much CPU and memory. The default at kernel build time should be minimal, to avoid cases where an unchanged value can cause a potential security hazard on a low end system. In otherwords it is up to the user at build time to set the value to an appropriate value for their system. Phillip --- Phillip Lougher, Squashfs author and maintainer. >> >> v1: >> https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ >> >> ---- >> >> Xiaoming Ni (2): >> squashfs: add the mount parameter theads=<single|multi|percpu> >> squashfs: Allows users to configure the number of decompression >> threads. >> >> fs/squashfs/Kconfig | 24 ++++++++-- >> fs/squashfs/decompressor_multi.c | 32 ++++++++------ >> fs/squashfs/decompressor_multi_percpu.c | 39 ++++++++++------- >> fs/squashfs/decompressor_single.c | 23 ++++++---- >> fs/squashfs/squashfs.h | 39 ++++++++++++++--- >> fs/squashfs/squashfs_fs_sb.h | 4 +- >> fs/squashfs/super.c | 77 >> ++++++++++++++++++++++++++++++++- >> 7 files changed, 192 insertions(+), 46 deletions(-) >> > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" 2022-08-28 23:18 ` Phillip Lougher @ 2022-08-30 13:38 ` Xiaoming Ni 2022-08-30 18:08 ` Phillip Lougher 0 siblings, 1 reply; 38+ messages in thread From: Xiaoming Ni @ 2022-08-30 13:38 UTC (permalink / raw) To: Phillip Lougher, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 2022/8/29 7:18, Phillip Lougher wrote: > On 26/08/2022 07:19, Xiaoming Ni wrote: >> ping >> >> >> On 2022/8/16 9:00, Xiaoming Ni wrote: >>> Currently, Squashfs supports multiple decompressor parallel modes. >>> However, this >>> mode can be configured only during kernel building and does not >>> support flexible >>> selection during runtime. >>> >>> In the current patch set, the mount parameter "threads=" is added to >>> allow users >>> to select the parallel decompressor mode and configure the number of >>> decompressors >>> when mounting a file system. >>> >>> v2: fix warning: sparse: incorrect type in initializer (different >>> address spaces) >>> Reported-by: kernel test robot <lkp@intel.com> > > I have made an initial review of the patches, and I have the following > comments. > > Good things about the patch-series. > > 1. In principle I have no objections to making this configurable at > mount time. But, a use-case for why this has become necessary > would help in the evaluation. > > 2. In general the code changes are good. They are predominantly > exposing the existing different decompressor functionality into > structures which can be selected at mount time. They do not > change existing functionality, and so there are no issues > about unexpected regressions. > > Things which I don't like about the patch-series. > > 1. There is no default kernel configuration option to keep the existing > behaviour, that is build time selectable only. There may be many > companies/people where for "security" reasons the ability to > switch to a more CPU/memory intensive decompressor or more threads > is a risk. > > Yes, I know the new kernel configuration options allow only the > selected default decompressor mode to be built. In theory that > will restrict the available decompressors to the single decompressor > selected at build time. So not much different to the current > position? But, if the CONFIG_SQUASHFS_DECOMP_MULTI decompressor > is selected, that will now allow more threads to be used than is No more threads than before the patch. > current, where it is currently restricted to num_online_cpus() * 2. After the patch is installed, the maximum number of threads is still num_online_cpus() * 2. [PATCH v2 2/2] squashfs: Allows users to configure the number of decompression threads +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + num = opts->thread_ops->max_decompressors(); + opts->thread_num = (int)num; + return 0; +#else > > 2. You have decided to allow the mutiple decompressor implementations > to be selected at mount time - but you have also allowed only one > decompressor to be built at kernel build time. This means you > end up in the fairly silly situation of having a mount time > option which allows the user to select between one decompressor. > There doesn't seem much point in having an option which allows > nothing to be changed. When multiple decompression modes are selected during kernel build, or only SQUASHFS_DECOMP_MULTI is selected during kernel build, the mount parameter "threads=" is meaningful, However, when only SQUASHFS_DECOMP_SINGLE or SQUASHFS_DECOMP_MULTI_PERCPU is selected, the mount parameter "threads=" is meaningless. Thank you for your guidance > > 3. Using thread=<number>, where thread=1 you use SQUASHFS_DECOMP_SINGLE > if it has been built, otherwise you fall back to > SQUASHFS_DECOMP_MULTI. This meants the effect of thread=1 is > indeterminate and depends on the build options. I would suggest > thread=1 should always mean use SQUASHFS_DECOMP_SINGLE. SQUASHFS_DECOMP_MULTI and SQUASHFS_DECOMP_SINGLE are selected during construction. Thread=1 indicates that SQUASHFS_DECOMP_SINGLEI is used. If only SQUASHFS_DECOMP_MULTI is selected during construction, thread=1 indicates that SQUASHFS_DECOMP_MULTI is used, and only one decompression thread is created. Would it be better to provide more flexible mount options for images that build only SQUASHFS_DECOMP_MULTI? > > 4. If SQUASHFS_DECOMP_MULTI is selected, there isn't a limit on the > maximum amount of threads allowed, and there is no ability to > set the maximum number of threads allowed at kernel build time > either. After the patch is installed, the maximum number of threads is still num_online_cpus() * 2. [PATCH v2 2/2] squashfs: Allows users to configure the number of decompression threads +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + num = opts->thread_ops->max_decompressors(); + opts->thread_num = (int)num; + return 0; Did I misunderstand your question? > > All of the above seems to be a bit of a mess. > > As regards points 1 - 3, personally I would add a default kernel > configuration option that keeps the existing behaviour, build time > selectable only, no additional mount time options. Then a > kernel configuration option that allows the different decompressors > to be selected at mount time, but which always builds all the > decompressors. This will avoid the silliness of point 2, and Would it be better to allow flexible selection of decompression mode combinations? > the indeterminate behaviour of point 3. > > As regards point 4, I think you should require the maximum number > of threads allowable to be determined at build time, this is > good for security and avoids attempts to use too much CPU > and memory. The default at kernel build time should be minimal, > to avoid cases where an unchanged value can cause a potential > security hazard on a low end system. In otherwords it is > up to the user at build time to set the value to an appropriate > value for their system. In patch 2, the maximum number of threads has been limited, Have I misunderstood your question > > Phillip > > --- > Phillip Lougher, Squashfs author and maintainer. > Thanks Xiaoming Ni >>> >>> v1: >>> https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ >>> >>> ---- >>> >>> Xiaoming Ni (2): >>> squashfs: add the mount parameter theads=<single|multi|percpu> >>> squashfs: Allows users to configure the number of decompression >>> threads. >>> >>> fs/squashfs/Kconfig | 24 ++++++++-- >>> fs/squashfs/decompressor_multi.c | 32 ++++++++------ >>> fs/squashfs/decompressor_multi_percpu.c | 39 ++++++++++------- >>> fs/squashfs/decompressor_single.c | 23 ++++++---- >>> fs/squashfs/squashfs.h | 39 ++++++++++++++--- >>> fs/squashfs/squashfs_fs_sb.h | 4 +- >>> fs/squashfs/super.c | 77 >>> ++++++++++++++++++++++++++++++++- >>> 7 files changed, 192 insertions(+), 46 deletions(-) >>> >> > > > . ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" 2022-08-30 13:38 ` Xiaoming Ni @ 2022-08-30 18:08 ` Phillip Lougher 2022-08-30 18:34 ` Phillip Lougher 0 siblings, 1 reply; 38+ messages in thread From: Phillip Lougher @ 2022-08-30 18:08 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 30/08/2022 14:38, Xiaoming Ni wrote: > On 2022/8/29 7:18, Phillip Lougher wrote: >> On 26/08/2022 07:19, Xiaoming Ni wrote: >>> ping >>> >>> >>> On 2022/8/16 9:00, Xiaoming Ni wrote: >>>> Currently, Squashfs supports multiple decompressor parallel modes. >>>> However, this >>>> mode can be configured only during kernel building and does not >>>> support flexible >>>> selection during runtime. >>>> >>>> In the current patch set, the mount parameter "threads=" is added to >>>> allow users >>>> to select the parallel decompressor mode and configure the number of >>>> decompressors >>>> when mounting a file system. >>>> >>>> v2: fix warning: sparse: incorrect type in initializer (different >>>> address spaces) >>>> Reported-by: kernel test robot <lkp@intel.com> >> >> I have made an initial review of the patches, and I have the following >> comments. >> >> Good things about the patch-series. >> >> 1. In principle I have no objections to making this configurable at >> mount time. But, a use-case for why this has become necessary >> would help in the evaluation. >> >> 2. In general the code changes are good. They are predominantly >> exposing the existing different decompressor functionality into >> structures which can be selected at mount time. They do not >> change existing functionality, and so there are no issues >> about unexpected regressions. >> >> Things which I don't like about the patch-series. >> >> 1. There is no default kernel configuration option to keep the existing >> behaviour, that is build time selectable only. There may be many >> companies/people where for "security" reasons the ability to >> switch to a more CPU/memory intensive decompressor or more threads >> is a risk. >> >> Yes, I know the new kernel configuration options allow only the >> selected default decompressor mode to be built. In theory that >> will restrict the available decompressors to the single decompressor >> selected at build time. So not much different to the current >> position? But, if the CONFIG_SQUASHFS_DECOMP_MULTI decompressor >> is selected, that will now allow more threads to be used than is > No more threads than before the patch. > >> current, where it is currently restricted to num_online_cpus() * 2. > After the patch is installed, the maximum number of threads is still > num_online_cpus() * 2. > > [PATCH v2 2/2] squashfs: Allows users to configure the number of > decompression threads > > +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI > + opts->thread_ops = &squashfs_decompressor_multi; > + if (num > opts->thread_ops->max_decompressors()) > + num = opts->thread_ops->max_decompressors(); > + opts->thread_num = (int)num; > + return 0; > +#else > I missed that the maximum number of threads is still limited to num_online_cpus() * 2. You should make it clear in the patch commit message what the thread maximum is (and that it is unchanged). This means some of my reservations go away. >> >> 2. You have decided to allow the mutiple decompressor implementations >> to be selected at mount time - but you have also allowed only one >> decompressor to be built at kernel build time. This means you >> end up in the fairly silly situation of having a mount time >> option which allows the user to select between one decompressor. >> There doesn't seem much point in having an option which allows >> nothing to be changed. > When multiple decompression modes are selected during kernel build, or > only SQUASHFS_DECOMP_MULTI is selected during kernel build, the mount > parameter "threads=" is meaningful, > However, when only SQUASHFS_DECOMP_SINGLE or > SQUASHFS_DECOMP_MULTI_PERCPU is selected, the mount parameter "threads=" > is meaningless. > > Thank you for your guidance > > >> >> 3. Using thread=<number>, where thread=1 you use SQUASHFS_DECOMP_SINGLE >> if it has been built, otherwise you fall back to >> SQUASHFS_DECOMP_MULTI. This meants the effect of thread=1 is >> indeterminate and depends on the build options. I would suggest >> thread=1 should always mean use SQUASHFS_DECOMP_SINGLE. > > SQUASHFS_DECOMP_MULTI and SQUASHFS_DECOMP_SINGLE are selected during > construction. Thread=1 indicates that SQUASHFS_DECOMP_SINGLEI is used. > > If only SQUASHFS_DECOMP_MULTI is selected during construction, thread=1 > indicates that SQUASHFS_DECOMP_MULTI is used, and only one decompression > thread is created. That is wrong. It violates the principles of KISS (keep it simple) and least surprise. I will spell it out for you. thread=1 meaning either SQUASHFS_DECOMP_SINGLE or SQUASHFS_DECOMP_MULTI depending on the built, adds an ambiguity which cannot be determined unless you know how the kernel was built. This violates KISS. SQUASHFS_DECOMP_MULTI is for parallel decompression - it's in the name. Over-loading it to do single decompression again violates KISS and it also violates the principle of least suprise. Many people will not think single decompression should be possible with only SQUASHFS_DECOMP_MULTI built in. SQUASHFS_DECOMP_SINGLE is for doing single decompression - again it's in the name. So SQUASHFS_DECOMP_MULTI for threads >= 2 SQUASHFS_DECOMP_SINGLE for thread = 1 This keeps it simple and follows the principle of least suprise. If you want single threaded operation as a choice, then build the SQUASHFS_DECOMP_SINGLE decompressor. > > Would it be better to provide more flexible mount options for images > that build only SQUASHFS_DECOMP_MULTI? > >> 4. If SQUASHFS_DECOMP_MULTI is selected, there isn't a limit on the >> maximum amount of threads allowed, and there is no ability to >> set the maximum number of threads allowed at kernel build time >> either. > After the patch is installed, the maximum number of threads is still > num_online_cpus() * 2. > > [PATCH v2 2/2] squashfs: Allows users to configure the number of > decompression threads > > +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI > + opts->thread_ops = &squashfs_decompressor_multi; > + if (num > opts->thread_ops->max_decompressors()) > + num = opts->thread_ops->max_decompressors(); > + opts->thread_num = (int)num; > + return 0; > > Did I misunderstand your question? > > >> >> All of the above seems to be a bit of a mess. >> >> As regards points 1 - 3, personally I would add a default kernel >> configuration option that keeps the existing behaviour, build time >> selectable only, no additional mount time options. Then a >> kernel configuration option that allows the different decompressors >> to be selected at mount time, but which always builds all the >> decompressors. This will avoid the silliness of point 2, and > Would it be better to allow flexible selection of decompression mode > combinations? I told you I don't like that (*). I also told you I want the default behaviour to be the current behaviour. Feel free to disagree, but that isn't a good way to get your patch reviewed or accepted by me. Cheers Phillip (*) Adding options to select the decompressor at mount time, but, also allowing only 1 - 2 decompressors to be built is a waste of time. It has the effect of giving something with one hand and taking it alway with the other. Build the lot, this also keeps it simple. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" 2022-08-30 18:08 ` Phillip Lougher @ 2022-08-30 18:34 ` Phillip Lougher 2022-08-31 1:09 ` Xiaoming Ni 0 siblings, 1 reply; 38+ messages in thread From: Phillip Lougher @ 2022-08-30 18:34 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 30/08/2022 19:08, Phillip Lougher wrote: > On 30/08/2022 14:38, Xiaoming Ni wrote: >> On 2022/8/29 7:18, Phillip Lougher wrote: >>> As regards points 1 - 3, personally I would add a default kernel >>> configuration option that keeps the existing behaviour, build time >>> selectable only, no additional mount time options. Then a >>> kernel configuration option that allows the different decompressors >>> to be selected at mount time, but which always builds all the >>> decompressors. This will avoid the silliness of point 2, and >> Would it be better to allow flexible selection of decompression mode >> combinations? > > I told you I don't like that (*). I also told you I want the default > behaviour to be the current behaviour. > > Feel free to disagree, but that isn't a good way to get your patch > reviewed or accepted by me. > > Cheers > > Phillip > > (*) Adding options to select the decompressor at mount time, but, > also allowing only 1 - 2 decompressors to be built is a waste of > time. It has the effect of giving something with one hand and > taking it alway with the other. Build the lot, this also > keeps it simple. > > It also splatters super.c with #ifdef CONFIG lines. phillip@phoenix:/external/stripe/linux/fs/squashfs$ grep "CONFIG_" super.c #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE #ifdef CONFIG_SQUASHFS_DECOMP_MULTI #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE #ifdef CONFIG_SQUASHFS_DECOMP_MULTI #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU #ifdef CONFIG_SQUASHFS_DECOMP_DEFAULT_SINGLE #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_MULTI) #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) Or static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) { unsigned long num; int ret; #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE if (strcmp(str, "single") == 0) { opts->thread_ops = &squashfs_decompressor_single; return 0; } #endif #ifdef CONFIG_SQUASHFS_DECOMP_MULTI if (strcmp(str, "multi") == 0) { opts->thread_ops = &squashfs_decompressor_multi; return 0; } #endif #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU if (strcmp(str, "percpu") == 0) { opts->thread_ops = &squashfs_decompressor_percpu; return 0; } #endif ret = kstrtoul(str, 0, &num); if (ret != 0 || num == 0) return -EINVAL; #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE if (num == 1) { opts->thread_ops = &squashfs_decompressor_single; return 0; } #endif #ifdef CONFIG_SQUASHFS_DECOMP_MULTI opts->thread_ops = &squashfs_decompressor_multi; if (num > opts->thread_ops->max_decompressors()) num = opts->thread_ops->max_decompressors(); opts->thread_num = (int)num; return 0; #else return -EINVAL; #endif } SNIP static int squashfs_show_options(struct seq_file *s, struct dentry *root) { struct super_block *sb = root->d_sb; struct squashfs_sb_info *msblk = sb->s_fs_info; if (msblk->panic_on_errors) seq_puts(s, ",errors=panic"); else seq_puts(s, ",errors=continue"); #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU /* * thread=percpu and thread=<number> have different configuration effects. * the parameters need to be treated differently when they are displayed. */ if (msblk->thread_ops == &squashfs_decompressor_percpu) { seq_puts(s, ",threads=percpu"); return 0; } #endif seq_printf(s, ",threads=%u", msblk->max_thread_num); return 0; } static int squashfs_init_fs_context(struct fs_context *fc) { struct squashfs_mount_opts *opts; opts = kzalloc(sizeof(*opts), GFP_KERNEL); if (!opts) return -ENOMEM; #ifdef CONFIG_SQUASHFS_DECOMP_DEFAULT_SINGLE opts->thread_ops = &squashfs_decompressor_single; #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_MULTI) opts->thread_ops = &squashfs_decompressor_multi, #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) opts->thread_ops = &squashfs_decompressor_percpu; #endif ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" 2022-08-30 18:34 ` Phillip Lougher @ 2022-08-31 1:09 ` Xiaoming Ni 0 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-08-31 1:09 UTC (permalink / raw) To: Phillip Lougher, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 2022/8/31 2:34, Phillip Lougher wrote: > On 30/08/2022 19:08, Phillip Lougher wrote: >> On 30/08/2022 14:38, Xiaoming Ni wrote: >>> On 2022/8/29 7:18, Phillip Lougher wrote: > >>>> As regards points 1 - 3, personally I would add a default kernel >>>> configuration option that keeps the existing behaviour, build time >>>> selectable only, no additional mount time options. Then a >>>> kernel configuration option that allows the different decompressors >>>> to be selected at mount time, but which always builds all the >>>> decompressors. This will avoid the silliness of point 2, and >>> Would it be better to allow flexible selection of decompression mode >>> combinations? >> >> I told you I don't like that (*). I also told you I want the default >> behaviour to be the current behaviour. >> >> Feel free to disagree, but that isn't a good way to get your patch >> reviewed or accepted by me. >> >> Cheers >> >> Phillip >> >> (*) Adding options to select the decompressor at mount time, but, >> also allowing only 1 - 2 decompressors to be built is a waste of >> time. It has the effect of giving something with one hand and >> taking it alway with the other. Build the lot, this also >> keeps it simple. >> >> > > It also splatters super.c with #ifdef CONFIG lines. > > phillip@phoenix:/external/stripe/linux/fs/squashfs$ grep "CONFIG_" super.c > #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > #ifdef CONFIG_SQUASHFS_DECOMP_DEFAULT_SINGLE > #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_MULTI) > #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) > > Or > > static int squashfs_parse_param_threads(const char *str, struct > squashfs_mount_opts *opts) > { > unsigned long num; > int ret; > #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > if (strcmp(str, "single") == 0) { > opts->thread_ops = &squashfs_decompressor_single; > return 0; > } > #endif > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI > if (strcmp(str, "multi") == 0) { > opts->thread_ops = &squashfs_decompressor_multi; > return 0; > } > #endif > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > if (strcmp(str, "percpu") == 0) { > opts->thread_ops = &squashfs_decompressor_percpu; > return 0; > } > #endif > ret = kstrtoul(str, 0, &num); > if (ret != 0 || num == 0) > return -EINVAL; > #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > if (num == 1) { > opts->thread_ops = &squashfs_decompressor_single; > return 0; > } > #endif > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI > opts->thread_ops = &squashfs_decompressor_multi; > if (num > opts->thread_ops->max_decompressors()) > num = opts->thread_ops->max_decompressors(); > opts->thread_num = (int)num; > return 0; > #else > return -EINVAL; > #endif > } > > SNIP > > static int squashfs_show_options(struct seq_file *s, struct dentry *root) > { > struct super_block *sb = root->d_sb; > struct squashfs_sb_info *msblk = sb->s_fs_info; > > if (msblk->panic_on_errors) > seq_puts(s, ",errors=panic"); > else > seq_puts(s, ",errors=continue"); > > #ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > /* > * thread=percpu and thread=<number> have different > configuration effects. > * the parameters need to be treated differently when they are > displayed. > */ > if (msblk->thread_ops == &squashfs_decompressor_percpu) { > seq_puts(s, ",threads=percpu"); > return 0; > } > #endif > seq_printf(s, ",threads=%u", msblk->max_thread_num); > return 0; > } > > static int squashfs_init_fs_context(struct fs_context *fc) > { > struct squashfs_mount_opts *opts; > > opts = kzalloc(sizeof(*opts), GFP_KERNEL); > if (!opts) > return -ENOMEM; > > #ifdef CONFIG_SQUASHFS_DECOMP_DEFAULT_SINGLE > opts->thread_ops = &squashfs_decompressor_single; > #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_MULTI) > opts->thread_ops = &squashfs_decompressor_multi, > #elif defined(CONFIG_SQUASHFS_DECOMP_DEFAULT_PERCPU) > opts->thread_ops = &squashfs_decompressor_percpu; > #endif > > Thanks for your guidance, I will update it in v3 patch as soon as possible Thanks Xiaoming Ni ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 0/2] squashfs: Add the mount parameter "threads=" 2022-08-16 1:00 ` [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni ` (2 preceding siblings ...) 2022-08-26 6:19 ` ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni @ 2022-09-02 9:48 ` Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni ` (3 more replies) 3 siblings, 4 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-02 9:48 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Currently, Squashfs supports multiple decompressor parallel modes. However, this mode can be configured only during kernel building and does not support flexible selection during runtime. In the current patch set, the mount parameter "threads=" is added to allow users to select the parallel decompressor mode and configure the number of decompressors when mounting a file system. "threads=<single|multi|percpu|1|2|3|...>" The upper limit is num_online_cpus() * 2. v3: Based on Philip Lougher's suggestion, make the following updates: 1. The default configuration is the same as that before the patch installation. 2. Compile the three decompression modes when the new configuration is enabled. 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ fix warning: sparse: incorrect type in initializer (different address spaces) Reported-by: kernel test robot <lkp@intel.com> v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ Xiaoming Ni (2): squashfs: add the mount parameter theads=<single|multi|percpu> squashfs: Allows users to configure the number of decompression threads. fs/squashfs/Kconfig | 51 ++++++++++++++++-- fs/squashfs/decompressor_multi.c | 32 +++++++----- fs/squashfs/decompressor_multi_percpu.c | 39 ++++++++------ fs/squashfs/decompressor_single.c | 23 +++++--- fs/squashfs/squashfs.h | 43 +++++++++++++-- fs/squashfs/squashfs_fs_sb.h | 4 +- fs/squashfs/super.c | 93 ++++++++++++++++++++++++++++++++- 7 files changed, 237 insertions(+), 48 deletions(-) -- 2.12.3 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-02 9:48 ` [PATCH v3 " Xiaoming Ni @ 2022-09-02 9:48 ` Xiaoming Ni 2022-09-09 15:44 ` Phillip Lougher 2022-09-09 15:50 ` Phillip Lougher 2022-09-02 9:48 ` [PATCH v3 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni ` (2 subsequent siblings) 3 siblings, 2 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-02 9:48 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Squashfs supports three decompression concurrency modes: Single-thread mode: concurrent reads are blocked and the memory overhead is small. Multi-thread mode/percpu mode: reduces concurrent read blocking but increases memory overhead. The corresponding schema must be fixed at compile time. During mounting, the concurrent decompression mode cannot be adjusted based on file read blocking. The mount parameter theads=<single|multi|percpu> is added to select the concurrent decompression mode of a single SquashFS file system image. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 40 ++++++++++++++++++++++---- fs/squashfs/decompressor_multi.c | 28 ++++++++++-------- fs/squashfs/decompressor_multi_percpu.c | 39 +++++++++++++++---------- fs/squashfs/decompressor_single.c | 23 +++++++++------ fs/squashfs/squashfs.h | 43 ++++++++++++++++++++++++---- fs/squashfs/squashfs_fs_sb.h | 3 +- fs/squashfs/super.c | 50 ++++++++++++++++++++++++++++++++- 7 files changed, 180 insertions(+), 46 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 916e78fabcaa..9c2827459f40 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -54,9 +54,37 @@ config SQUASHFS_FILE_DIRECT endchoice +config SQUASHFS_DECOMP_SINGLE + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI_PERCPU + depends on SQUASHFS + def_bool n + +config SQUASHFS_CHOICE_DECOMP_BY_MOUNT + bool "Select the parallel decompression mode during mount" + depends on SQUASHFS + default n + select SQUASHFS_DECOMP_SINGLE + select SQUASHFS_DECOMP_MULTI + select SQUASHFS_DECOMP_MULTI_PERCPU + help + Compile all parallel decompression modes and specify the + decompression mode by setting "threads=" during mount. + + threads=<single|multi|percpu> + + default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE + choice - prompt "Decompressor parallelisation options" + prompt "Select decompression parallel mode at compile time" depends on SQUASHFS + depends on !SQUASHFS_CHOICE_DECOMP_BY_MOUNT help Squashfs now supports three parallelisation options for decompression. Each one exhibits various trade-offs between @@ -64,15 +92,17 @@ choice If in doubt, select "Single threaded compression" -config SQUASHFS_DECOMP_SINGLE +config SQUASHFS_COMPILE_DECOMP_SINGLE bool "Single threaded compression" + select SQUASHFS_DECOMP_SINGLE help Traditionally Squashfs has used single-threaded decompression. Only one block (data or metadata) can be decompressed at any one time. This limits CPU and memory usage to a minimum. -config SQUASHFS_DECOMP_MULTI +config SQUASHFS_COMPILE_DECOMP_MULTI bool "Use multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -85,8 +115,9 @@ config SQUASHFS_DECOMP_MULTI decompressors per core. It dynamically allocates decompressors on a demand basis. -config SQUASHFS_DECOMP_MULTI_PERCPU +config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI_PERCPU help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -95,7 +126,6 @@ config SQUASHFS_DECOMP_MULTI_PERCPU This decompressor implementation uses a maximum of one decompressor per core. It uses percpu variables to ensure decompression is load-balanced across the cores. - endchoice config SQUASHFS_XATTR diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index db9f12a3ea05..7b2723b77e75 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -29,13 +29,12 @@ #define MAX_DECOMPRESSOR (num_online_cpus() * 2) -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_multi(void) { return MAX_DECOMPRESSOR; } - -struct squashfs_stream { +struct squashfs_stream_multi { void *comp_opts; struct list_head strm_list; struct mutex mutex; @@ -51,7 +50,7 @@ struct decomp_stream { static void put_decomp_stream(struct decomp_stream *decomp_strm, - struct squashfs_stream *stream) + struct squashfs_stream_multi *stream) { mutex_lock(&stream->mutex); list_add(&decomp_strm->list, &stream->strm_list); @@ -59,10 +58,10 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, wake_up(&stream->wait); } -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_multi(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; + struct squashfs_stream_multi *stream; struct decomp_stream *decomp_strm = NULL; int err = -ENOMEM; @@ -103,9 +102,9 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_multi(struct squashfs_sb_info *msblk) { - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_multi *stream = msblk->stream; if (stream) { struct decomp_stream *decomp_strm; @@ -125,7 +124,7 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, - struct squashfs_stream *stream) + struct squashfs_stream_multi *stream) { struct decomp_stream *decomp_strm; @@ -180,12 +179,12 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_multi(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_multi *stream = msblk->stream; struct decomp_stream *decomp_stream = get_decomp_stream(msblk, stream); res = msblk->decompressor->decompress(msblk, decomp_stream->stream, bio, offset, length, output); @@ -195,3 +194,10 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, msblk->decompressor->name); return res; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { + .create = squashfs_decompressor_create_multi, + .destroy = squashfs_decompressor_destroy_multi, + .decompress = squashfs_decompress_multi, + .max_decompressors = squashfs_max_decompressors_multi, +}; diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c index b881b9283b7f..e24455a7b04d 100644 --- a/fs/squashfs/decompressor_multi_percpu.c +++ b/fs/squashfs/decompressor_multi_percpu.c @@ -20,19 +20,19 @@ * variables, one thread per cpu core. */ -struct squashfs_stream { +struct squashfs_stream_percpu { void *stream; local_lock_t lock; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_percpu(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; - struct squashfs_stream __percpu *percpu; + struct squashfs_stream_percpu *stream; + struct squashfs_stream_percpu __percpu *percpu; int err, cpu; - percpu = alloc_percpu(struct squashfs_stream); + percpu = alloc_percpu(struct squashfs_stream_percpu); if (percpu == NULL) return ERR_PTR(-ENOMEM); @@ -59,11 +59,11 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_percpu(struct squashfs_sb_info *msblk) { - struct squashfs_stream __percpu *percpu = - (struct squashfs_stream __percpu *) msblk->stream; - struct squashfs_stream *stream; + struct squashfs_stream_percpu __percpu *percpu = + (struct squashfs_stream_percpu __percpu *) msblk->stream; + struct squashfs_stream_percpu *stream; int cpu; if (msblk->stream) { @@ -75,19 +75,21 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { - struct squashfs_stream *stream; + struct squashfs_stream_percpu *stream; + struct squashfs_stream_percpu __percpu *percpu = + (struct squashfs_stream_percpu __percpu *) msblk->stream; int res; - local_lock(&msblk->stream->lock); - stream = this_cpu_ptr(msblk->stream); + local_lock(&percpu->lock); + stream = this_cpu_ptr(percpu); res = msblk->decompressor->decompress(msblk, stream->stream, bio, offset, length, output); - local_unlock(&msblk->stream->lock); + local_unlock(&percpu->lock); if (res < 0) ERROR("%s decompression failed, data probably corrupt\n", @@ -96,7 +98,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_percpu(void) { return num_possible_cpus(); } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { + .create = squashfs_decompressor_create_percpu, + .destroy = squashfs_decompressor_destroy_percpu, + .decompress = squashfs_decompress_percpu, + .max_decompressors = squashfs_max_decompressors_percpu, +}; diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c index 4eb3d083d45e..41449de0ea4c 100644 --- a/fs/squashfs/decompressor_single.c +++ b/fs/squashfs/decompressor_single.c @@ -19,15 +19,15 @@ * decompressor framework */ -struct squashfs_stream { +struct squashfs_stream_single { void *stream; struct mutex mutex; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create_single(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream *stream; + struct squashfs_stream_single *stream; int err = -ENOMEM; stream = kmalloc(sizeof(*stream), GFP_KERNEL); @@ -49,9 +49,9 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy_single(struct squashfs_sb_info *msblk) { - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_single *stream = msblk->stream; if (stream) { msblk->decompressor->free(stream->stream); @@ -59,12 +59,12 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress_single(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream *stream = msblk->stream; + struct squashfs_stream_single *stream = msblk->stream; mutex_lock(&stream->mutex); res = msblk->decompressor->decompress(msblk, stream->stream, bio, @@ -78,7 +78,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors_single(void) { return 1; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { + .create = squashfs_decompressor_create_single, + .destroy = squashfs_decompressor_destroy_single, + .decompress = squashfs_decompress_single, + .max_decompressors = squashfs_max_decompressors_single, +}; diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h index 9783e01c8100..9a383ad0dae0 100644 --- a/fs/squashfs/squashfs.h +++ b/fs/squashfs/squashfs.h @@ -38,12 +38,45 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); /* decompressor_xxx.c */ -extern void *squashfs_decompressor_create(struct squashfs_sb_info *, void *); -extern void squashfs_decompressor_destroy(struct squashfs_sb_info *); -extern int squashfs_decompress(struct squashfs_sb_info *, struct bio *, - int, int, struct squashfs_page_actor *); -extern int squashfs_max_decompressors(void); +struct squashfs_decompressor_thread_ops { + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); + void (*destroy)(struct squashfs_sb_info *msblk); + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output); + int (*max_decompressors)(void); +}; + +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; +#endif + +static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) +{ + return msblk->thread_ops->create(msblk, comp_opts); +} + +static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +{ + msblk->thread_ops->destroy(msblk); +} + +static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output) +{ + return msblk->thread_ops->decompress(msblk, bio, offset, length, output); +} + +static inline int squashfs_max_decompressors(struct squashfs_sb_info *msblk) +{ + return msblk->thread_ops->max_decompressors(); +} /* export.c */ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, unsigned int); diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index 1e90c2575f9b..f1e5dad8ae0a 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -53,7 +53,7 @@ struct squashfs_sb_info { __le64 *xattr_id_table; struct mutex meta_index_mutex; struct meta_index *meta_index; - struct squashfs_stream *stream; + void *stream; __le64 *inode_lookup_table; u64 inode_table; u64 directory_table; @@ -66,5 +66,6 @@ struct squashfs_sb_info { int xattr_ids; unsigned int ids; bool panic_on_errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 32565dafa7f3..fd4e70d45f3c 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -47,10 +47,12 @@ enum Opt_errors { enum squashfs_param { Opt_errors, + Opt_threads, }; struct squashfs_mount_opts { enum Opt_errors errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; static const struct constant_table squashfs_param_errors[] = { @@ -61,9 +63,29 @@ static const struct constant_table squashfs_param_errors[] = { static const struct fs_parameter_spec squashfs_fs_parameters[] = { fsparam_enum("errors", Opt_errors, squashfs_param_errors), + fsparam_string("threads", Opt_threads), {} }; +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (strcmp(str, "single") == 0) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } + if (strcmp(str, "multi") == 0) { + opts->thread_ops = &squashfs_decompressor_multi; + return 0; + } + if (strcmp(str, "percpu") == 0) { + opts->thread_ops = &squashfs_decompressor_percpu; + return 0; + } +#endif + return -EINVAL; +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -78,6 +100,10 @@ static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *para case Opt_errors: opts->errors = result.uint_32; break; + case Opt_threads: + if (squashfs_parse_param_threads(param->string, opts) != 0) + return -EINVAL; + break; default: return -EINVAL; } @@ -167,6 +193,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_bdev); goto failed_mount; } + msblk->thread_ops = opts->thread_ops; /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -252,7 +279,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - squashfs_max_decompressors(), msblk->block_size); + squashfs_max_decompressors(msblk), msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -435,6 +462,20 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_multi) { + seq_puts(s, ",threads=multi"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_percpu) { + seq_puts(s, ",threads=percpu"); + return 0; + } +#endif return 0; } @@ -446,6 +487,13 @@ static int squashfs_init_fs_context(struct fs_context *fc) if (!opts) return -ENOMEM; +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + opts->thread_ops = &squashfs_decompressor_single; +#elif CONFIG_SQUASHFS_DECOMP_MULTI + opts->thread_ops = &squashfs_decompressor_multi; +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + opts->thread_ops = &squashfs_decompressor_percpu; +#endif fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-02 9:48 ` [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-09-09 15:44 ` Phillip Lougher 2022-09-13 2:46 ` Xiaoming Ni 2022-09-09 15:50 ` Phillip Lougher 1 sibling, 1 reply; 38+ messages in thread From: Phillip Lougher @ 2022-09-09 15:44 UTC (permalink / raw) To: nixiaoming Cc: chenjianguo3, linux-kernel, phillip, wangbing6, wangle6, yi.zhang, zhongjubin Review comments below. Xiaoming Ni <nixiaoming@huawei.com> wrote: >Squashfs supports three decompression concurrency modes: > Single-thread mode: concurrent reads are blocked and the memory overhead >is small. Please wrap over 80 column line. > Multi-thread mode/percpu mode: reduces concurrent read blocking but >increases memory overhead. > >The corresponding schema must be fixed at compile time. During mounting, >the concurrent decompression mode cannot be adjusted based on file read >blocking. > >The mount parameter theads=<single|multi|percpu> is added to select >the concurrent decompression mode of a single SquashFS file system >image. > >Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> >--- > fs/squashfs/Kconfig | 40 ++++++++++++++++++++++---- > fs/squashfs/decompressor_multi.c | 28 ++++++++++-------- > fs/squashfs/decompressor_multi_percpu.c | 39 +++++++++++++++---------- > fs/squashfs/decompressor_single.c | 23 +++++++++------ > fs/squashfs/squashfs.h | 43 ++++++++++++++++++++++++---- > fs/squashfs/squashfs_fs_sb.h | 3 +- > fs/squashfs/super.c | 50 ++++++++++++++++++++++++++++++++- > 7 files changed, 180 insertions(+), 46 deletions(-) > >diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig >index 916e78fabcaa..9c2827459f40 100644 >--- a/fs/squashfs/Kconfig >+++ b/fs/squashfs/Kconfig >@@ -54,9 +54,37 @@ config SQUASHFS_FILE_DIRECT > > endchoice > >+config SQUASHFS_DECOMP_SINGLE >+ depends on SQUASHFS >+ def_bool n >+ >+config SQUASHFS_DECOMP_MULTI >+ depends on SQUASHFS >+ def_bool n >+ >+config SQUASHFS_DECOMP_MULTI_PERCPU >+ depends on SQUASHFS >+ def_bool n >+ >+config SQUASHFS_CHOICE_DECOMP_BY_MOUNT >+ bool "Select the parallel decompression mode during mount" >+ depends on SQUASHFS >+ default n >+ select SQUASHFS_DECOMP_SINGLE >+ select SQUASHFS_DECOMP_MULTI >+ select SQUASHFS_DECOMP_MULTI_PERCPU >+ help >+ Compile all parallel decompression modes and specify the >+ decompression mode by setting "threads=" during mount. >+ >+ threads=<single|multi|percpu> git am reports "warning: 1 line adds whitespace errors.", which is here. [SNIP] >diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c >index db9f12a3ea05..7b2723b77e75 100644 >--- a/fs/squashfs/decompressor_multi.c >+++ b/fs/squashfs/decompressor_multi.c >@@ -29,13 +29,12 @@ > #define MAX_DECOMPRESSOR (num_online_cpus() * 2) > > >-int squashfs_max_decompressors(void) >+static int squashfs_max_decompressors_multi(void) Changing the name of the function *should* be unnecessary, because you're making it static. > { > return MAX_DECOMPRESSOR; > } > >- >-struct squashfs_stream { >+struct squashfs_stream_multi { Again changing the name of the structure should be unnecessary. This is just extra noise. The current "diffstat" for the 3 decompression threading implementations is: fs/squashfs/decompressor_multi.c | 28 ++++++++++-------- fs/squashfs/decompressor_multi_percpu.c | 39 +++++++++++++++---------- fs/squashfs/decompressor_single.c | 23 +++++++++------ But, once you get rid of this noise, it drops to fs/squashfs/decompressor_multi.c | 16 +++++--- fs/squashfs/decompressor_multi_percpu.c | 23 +++++++---- fs/squashfs/decompressor_single.c | 15 ++++++-- [SNIP] >--- a/fs/squashfs/squashfs.h >+++ b/fs/squashfs/squashfs.h >@@ -38,12 +38,45 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); > extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); > > /* decompressor_xxx.c */ [SNIP] >+ >+static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) >+{ >+ return msblk->thread_ops->create(msblk, comp_opts); >+} >+ >+static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) >+{ >+ msblk->thread_ops->destroy(msblk); >+} >+ >+static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, >+ int offset, int length, struct squashfs_page_actor *output) >+{ >+ return msblk->thread_ops->decompress(msblk, bio, offset, length, output); >+} >+ >+static inline int squashfs_max_decompressors(struct squashfs_sb_info *msblk) >+{ >+ return msblk->thread_ops->max_decompressors(); >+} The above is the reason why you've had to rename the functions in the decompression threading implementations. Because you've put the new accessors into squashfs.h which is also included by the threading implementations which causes a name clash. But, the correct way to avoid this clash, is to put the accessors into a new header file, which isn't included by the decompression threading implementations. The following patch does this cleanup, to simplify the patch here. It also moves the struct squashfs_decompressor_thread_ops definition into decompressor.h which is a better place. Phillip fs/squashfs/block.c | 1 + fs/squashfs/decompressor.c | 1 + fs/squashfs/decompressor.h | 8 +++++ fs/squashfs/decompressor_multi.c | 28 ++++++++--------- fs/squashfs/decompressor_multi_percpu.c | 36 +++++++++++----------- fs/squashfs/decompressor_single.c | 24 +++++++-------- fs/squashfs/squashfs.h | 40 ------------------------- fs/squashfs/super.c | 1 + fs/squashfs/threading.h | 30 +++++++++++++++++++ 9 files changed, 85 insertions(+), 84 deletions(-) create mode 100644 fs/squashfs/threading.h diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 833aca92301f..9bcf7e1b64be 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -26,6 +26,7 @@ #include "squashfs.h" #include "decompressor.h" #include "page_actor.h" +#include "threading.h" /* * Returns the amount of bytes copied to the page actor. diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c index d57bef91ab08..5c355fca0df5 100644 --- a/fs/squashfs/decompressor.c +++ b/fs/squashfs/decompressor.c @@ -18,6 +18,7 @@ #include "decompressor.h" #include "squashfs.h" #include "page_actor.h" +#include "threading.h" /* * This file (and decompressor.h) implements a decompressor framework for diff --git a/fs/squashfs/decompressor.h b/fs/squashfs/decompressor.h index 19ab60834389..5b5ec4828930 100644 --- a/fs/squashfs/decompressor.h +++ b/fs/squashfs/decompressor.h @@ -24,6 +24,14 @@ struct squashfs_decompressor { int supported; }; +struct squashfs_decompressor_thread_ops { + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); + void (*destroy)(struct squashfs_sb_info *msblk); + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output); + int (*max_decompressors)(void); +}; + static inline void *squashfs_comp_opts(struct squashfs_sb_info *msblk, void *buff, int length) { diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index 7b2723b77e75..eb25432bd149 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -29,12 +29,12 @@ #define MAX_DECOMPRESSOR (num_online_cpus() * 2) -static int squashfs_max_decompressors_multi(void) +static int squashfs_max_decompressors(void) { return MAX_DECOMPRESSOR; } -struct squashfs_stream_multi { +struct squashfs_stream { void *comp_opts; struct list_head strm_list; struct mutex mutex; @@ -50,7 +50,7 @@ struct decomp_stream { static void put_decomp_stream(struct decomp_stream *decomp_strm, - struct squashfs_stream_multi *stream) + struct squashfs_stream *stream) { mutex_lock(&stream->mutex); list_add(&decomp_strm->list, &stream->strm_list); @@ -58,10 +58,10 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, wake_up(&stream->wait); } -static void *squashfs_decompressor_create_multi(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream_multi *stream; + struct squashfs_stream *stream; struct decomp_stream *decomp_strm = NULL; int err = -ENOMEM; @@ -102,9 +102,9 @@ static void *squashfs_decompressor_create_multi(struct squashfs_sb_info *msblk, } -static void squashfs_decompressor_destroy_multi(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { - struct squashfs_stream_multi *stream = msblk->stream; + struct squashfs_stream *stream = msblk->stream; if (stream) { struct decomp_stream *decomp_strm; @@ -124,7 +124,7 @@ static void squashfs_decompressor_destroy_multi(struct squashfs_sb_info *msblk) static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, - struct squashfs_stream_multi *stream) + struct squashfs_stream *stream) { struct decomp_stream *decomp_strm; @@ -179,12 +179,12 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } -static int squashfs_decompress_multi(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream_multi *stream = msblk->stream; + struct squashfs_stream *stream = msblk->stream; struct decomp_stream *decomp_stream = get_decomp_stream(msblk, stream); res = msblk->decompressor->decompress(msblk, decomp_stream->stream, bio, offset, length, output); @@ -196,8 +196,8 @@ static int squashfs_decompress_multi(struct squashfs_sb_info *msblk, struct bio } const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { - .create = squashfs_decompressor_create_multi, - .destroy = squashfs_decompressor_destroy_multi, - .decompress = squashfs_decompress_multi, - .max_decompressors = squashfs_max_decompressors_multi, + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, }; diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c index e24455a7b04d..1dfadf76ed9a 100644 --- a/fs/squashfs/decompressor_multi_percpu.c +++ b/fs/squashfs/decompressor_multi_percpu.c @@ -20,19 +20,19 @@ * variables, one thread per cpu core. */ -struct squashfs_stream_percpu { +struct squashfs_stream { void *stream; local_lock_t lock; }; -static void *squashfs_decompressor_create_percpu(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream_percpu *stream; - struct squashfs_stream_percpu __percpu *percpu; + struct squashfs_stream *stream; + struct squashfs_stream __percpu *percpu; int err, cpu; - percpu = alloc_percpu(struct squashfs_stream_percpu); + percpu = alloc_percpu(struct squashfs_stream); if (percpu == NULL) return ERR_PTR(-ENOMEM); @@ -59,11 +59,11 @@ static void *squashfs_decompressor_create_percpu(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -static void squashfs_decompressor_destroy_percpu(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { - struct squashfs_stream_percpu __percpu *percpu = - (struct squashfs_stream_percpu __percpu *) msblk->stream; - struct squashfs_stream_percpu *stream; + struct squashfs_stream __percpu *percpu = + (struct squashfs_stream __percpu *) msblk->stream; + struct squashfs_stream *stream; int cpu; if (msblk->stream) { @@ -75,12 +75,12 @@ static void squashfs_decompressor_destroy_percpu(struct squashfs_sb_info *msblk) } } -static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { - struct squashfs_stream_percpu *stream; - struct squashfs_stream_percpu __percpu *percpu = - (struct squashfs_stream_percpu __percpu *) msblk->stream; + struct squashfs_stream *stream; + struct squashfs_stream __percpu *percpu = + (struct squashfs_stream __percpu *) msblk->stream; int res; local_lock(&percpu->lock); @@ -98,14 +98,14 @@ static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio return res; } -static int squashfs_max_decompressors_percpu(void) +static int squashfs_max_decompressors(void) { return num_possible_cpus(); } const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { - .create = squashfs_decompressor_create_percpu, - .destroy = squashfs_decompressor_destroy_percpu, - .decompress = squashfs_decompress_percpu, - .max_decompressors = squashfs_max_decompressors_percpu, + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, }; diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c index 41449de0ea4c..6f161887710b 100644 --- a/fs/squashfs/decompressor_single.c +++ b/fs/squashfs/decompressor_single.c @@ -19,15 +19,15 @@ * decompressor framework */ -struct squashfs_stream_single { +struct squashfs_stream { void *stream; struct mutex mutex; }; -static void *squashfs_decompressor_create_single(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { - struct squashfs_stream_single *stream; + struct squashfs_stream *stream; int err = -ENOMEM; stream = kmalloc(sizeof(*stream), GFP_KERNEL); @@ -49,9 +49,9 @@ static void *squashfs_decompressor_create_single(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -static void squashfs_decompressor_destroy_single(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { - struct squashfs_stream_single *stream = msblk->stream; + struct squashfs_stream *stream = msblk->stream; if (stream) { msblk->decompressor->free(stream->stream); @@ -59,12 +59,12 @@ static void squashfs_decompressor_destroy_single(struct squashfs_sb_info *msblk) } } -static int squashfs_decompress_single(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { int res; - struct squashfs_stream_single *stream = msblk->stream; + struct squashfs_stream *stream = msblk->stream; mutex_lock(&stream->mutex); res = msblk->decompressor->decompress(msblk, stream->stream, bio, @@ -78,14 +78,14 @@ static int squashfs_decompress_single(struct squashfs_sb_info *msblk, struct bio return res; } -static int squashfs_max_decompressors_single(void) +static int squashfs_max_decompressors(void) { return 1; } const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { - .create = squashfs_decompressor_create_single, - .destroy = squashfs_decompressor_destroy_single, - .decompress = squashfs_decompress_single, - .max_decompressors = squashfs_max_decompressors_single, + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, }; diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h index 9a383ad0dae0..9ba9e95440f8 100644 --- a/fs/squashfs/squashfs.h +++ b/fs/squashfs/squashfs.h @@ -37,46 +37,6 @@ extern void *squashfs_read_table(struct super_block *, u64, int); extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); -/* decompressor_xxx.c */ - -struct squashfs_decompressor_thread_ops { - void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); - void (*destroy)(struct squashfs_sb_info *msblk); - int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, - int offset, int length, struct squashfs_page_actor *output); - int (*max_decompressors)(void); -}; - -#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE -extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; -#endif -#ifdef CONFIG_SQUASHFS_DECOMP_MULTI -extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; -#endif -#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU -extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; -#endif - -static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) -{ - return msblk->thread_ops->create(msblk, comp_opts); -} - -static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) -{ - msblk->thread_ops->destroy(msblk); -} - -static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, - int offset, int length, struct squashfs_page_actor *output) -{ - return msblk->thread_ops->decompress(msblk, bio, offset, length, output); -} - -static inline int squashfs_max_decompressors(struct squashfs_sb_info *msblk) -{ - return msblk->thread_ops->max_decompressors(); -} /* export.c */ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, unsigned int); diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index fd4e70d45f3c..d7dc4b304aa0 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -36,6 +36,7 @@ #include "squashfs.h" #include "decompressor.h" #include "xattr.h" +#include "threading.h" static struct file_system_type squashfs_fs_type; static const struct super_operations squashfs_super_ops; diff --git a/fs/squashfs/threading.h b/fs/squashfs/threading.h new file mode 100644 index 000000000000..bd06519fb8b9 --- /dev/null +++ b/fs/squashfs/threading.h @@ -0,0 +1,30 @@ +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; +#endif + +static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) +{ + return msblk->thread_ops->create(msblk, comp_opts); +} + +static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +{ + msblk->thread_ops->destroy(msblk); +} + +static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output) +{ + return msblk->thread_ops->decompress(msblk, bio, offset, length, output); +} + +static inline int squashfs_max_decompressors(struct squashfs_sb_info *msblk) +{ + return msblk->thread_ops->max_decompressors(); +} -- 2.35.1 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-09 15:44 ` Phillip Lougher @ 2022-09-13 2:46 ` Xiaoming Ni 0 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-13 2:46 UTC (permalink / raw) To: Phillip Lougher Cc: chenjianguo3, linux-kernel, wangbing6, wangle6, yi.zhang, zhongjubin On 2022/9/9 23:44, Phillip Lougher wrote: > Review comments below. > > Xiaoming Ni <nixiaoming@huawei.com> wrote: >> Squashfs supports three decompression concurrency modes: >> Single-thread mode: concurrent reads are blocked and the memory overhead >> is small. > > Please wrap over 80 column line. Thanks for your review, I'll fix it in the next patch version ... >> + Compile all parallel decompression modes and specify the >> + decompression mode by setting "threads=" during mount. >> + >> + threads=<single|multi|percpu> > > git am reports "warning: 1 line adds whitespace errors.", which is here. Thanks for your review, I'll fix it in the next patch version > [SNIP] > >> diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c >> index db9f12a3ea05..7b2723b77e75 100644 >> --- a/fs/squashfs/decompressor_multi.c >> +++ b/fs/squashfs/decompressor_multi.c >> @@ -29,13 +29,12 @@ >> #define MAX_DECOMPRESSOR (num_online_cpus() * 2) >> >> >> -int squashfs_max_decompressors(void) >> +static int squashfs_max_decompressors_multi(void) > > Changing the name of the function *should* be unnecessary, because > you're making it static. > >> { >> return MAX_DECOMPRESSOR; >> } >> >> - >> -struct squashfs_stream { >> +struct squashfs_stream_multi { > > Again changing the name of the structure should be unnecessary. > > This is just extra noise. > > The current "diffstat" for the 3 decompression threading implementations is: > > fs/squashfs/decompressor_multi.c | 28 ++++++++++-------- > fs/squashfs/decompressor_multi_percpu.c | 39 +++++++++++++++---------- > fs/squashfs/decompressor_single.c | 23 +++++++++------ > > But, once you get rid of this noise, it drops to > > fs/squashfs/decompressor_multi.c | 16 +++++--- > fs/squashfs/decompressor_multi_percpu.c | 23 +++++++---- > fs/squashfs/decompressor_single.c | 15 ++++++-- > > [SNIP] Thanks for your comment, I'll use it in the next patch version to reduce the amount of code changes. > >> --- a/fs/squashfs/squashfs.h >> +++ b/fs/squashfs/squashfs.h >> @@ -38,12 +38,45 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); >> extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); >> >> /* decompressor_xxx.c */ > > [SNIP] > >> + >> +static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) >> +{ >> + return msblk->thread_ops->create(msblk, comp_opts); >> +} >> + >> +static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) >> +{ >> + msblk->thread_ops->destroy(msblk); >> +} >> + >> +static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, >> + int offset, int length, struct squashfs_page_actor *output) >> +{ >> + return msblk->thread_ops->decompress(msblk, bio, offset, length, output); >> +} >> + >> +static inline int squashfs_max_decompressors(struct squashfs_sb_info *msblk) >> +{ >> + return msblk->thread_ops->max_decompressors(); >> +} > > The above is the reason why you've had to rename the functions in the > decompression threading implementations. Because you've put the new > accessors into squashfs.h which is also included by the threading > implementations which causes a name clash. > > But, the correct way to avoid this clash, is to put the accessors into > a new header file, which isn't included by the decompression threading > implementations. > > The following patch does this cleanup, to simplify the patch here. It also > moves the struct squashfs_decompressor_thread_ops definition into > decompressor.h which is a better place. > > Phillip > > fs/squashfs/block.c | 1 + > fs/squashfs/decompressor.c | 1 + > fs/squashfs/decompressor.h | 8 +++++ > fs/squashfs/decompressor_multi.c | 28 ++++++++--------- > fs/squashfs/decompressor_multi_percpu.c | 36 +++++++++++----------- > fs/squashfs/decompressor_single.c | 24 +++++++-------- > fs/squashfs/squashfs.h | 40 ------------------------- > fs/squashfs/super.c | 1 + > fs/squashfs/threading.h | 30 +++++++++++++++++++ > 9 files changed, 85 insertions(+), 84 deletions(-) > create mode 100644 fs/squashfs/threading.h I re-checked the use of create(), destory(), decompress(), max_decompressors() and found that the corresponding function had only one or two call points, so do we not need to encapsulate in .h? Squashfs_decompress() is called only once in fs/squashfs/block.c, squashfs_read_data(). Squashfs_decompressor_create() is called only once in fs/squashfs/decompressor.c, squashfs_decompressor_setup(). squashfs_max_decompressors() is called only once in fs/squashfs/super.c, squashfs_fill_super(). squashfs_decompressor_destroy() is called only in fs/squashfs/super.c, squashfs_fill_super() and squashfs_put_super(). Thanks Xiaoming Ni > > diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c > index 833aca92301f..9bcf7e1b64be 100644 > --- a/fs/squashfs/block.c > +++ b/fs/squashfs/block.c > @@ -26,6 +26,7 @@ > #include "squashfs.h" > #include "decompressor.h" > #include "page_actor.h" > +#include "threading.h" > > /* > * Returns the amount of bytes copied to the page actor. > diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c > index d57bef91ab08..5c355fca0df5 100644 > --- a/fs/squashfs/decompressor.c > +++ b/fs/squashfs/decompressor.c > @@ -18,6 +18,7 @@ > #include "decompressor.h" > #include "squashfs.h" > #include "page_actor.h" > +#include "threading.h" > > /* > * This file (and decompressor.h) implements a decompressor framework for > diff --git a/fs/squashfs/decompressor.h b/fs/squashfs/decompressor.h > index 19ab60834389..5b5ec4828930 100644 > --- a/fs/squashfs/decompressor.h > +++ b/fs/squashfs/decompressor.h > @@ -24,6 +24,14 @@ struct squashfs_decompressor { > int supported; > }; > > +struct squashfs_decompressor_thread_ops { > + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); > + void (*destroy)(struct squashfs_sb_info *msblk); > + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, > + int offset, int length, struct squashfs_page_actor *output); > + int (*max_decompressors)(void); > +}; > + > static inline void *squashfs_comp_opts(struct squashfs_sb_info *msblk, > void *buff, int length) > { > diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c > index 7b2723b77e75..eb25432bd149 100644 > --- a/fs/squashfs/decompressor_multi.c > +++ b/fs/squashfs/decompressor_multi.c > @@ -29,12 +29,12 @@ > #define MAX_DECOMPRESSOR (num_online_cpus() * 2) > > > -static int squashfs_max_decompressors_multi(void) > +static int squashfs_max_decompressors(void) > { > return MAX_DECOMPRESSOR; > } > > -struct squashfs_stream_multi { > +struct squashfs_stream { > void *comp_opts; > struct list_head strm_list; > struct mutex mutex; > @@ -50,7 +50,7 @@ struct decomp_stream { > > > static void put_decomp_stream(struct decomp_stream *decomp_strm, > - struct squashfs_stream_multi *stream) > + struct squashfs_stream *stream) > { > mutex_lock(&stream->mutex); > list_add(&decomp_strm->list, &stream->strm_list); > @@ -58,10 +58,10 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, > wake_up(&stream->wait); > } > > -static void *squashfs_decompressor_create_multi(struct squashfs_sb_info *msblk, > +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, > void *comp_opts) > { > - struct squashfs_stream_multi *stream; > + struct squashfs_stream *stream; > struct decomp_stream *decomp_strm = NULL; > int err = -ENOMEM; > > @@ -102,9 +102,9 @@ static void *squashfs_decompressor_create_multi(struct squashfs_sb_info *msblk, > } > > > -static void squashfs_decompressor_destroy_multi(struct squashfs_sb_info *msblk) > +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) > { > - struct squashfs_stream_multi *stream = msblk->stream; > + struct squashfs_stream *stream = msblk->stream; > if (stream) { > struct decomp_stream *decomp_strm; > > @@ -124,7 +124,7 @@ static void squashfs_decompressor_destroy_multi(struct squashfs_sb_info *msblk) > > > static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, > - struct squashfs_stream_multi *stream) > + struct squashfs_stream *stream) > { > struct decomp_stream *decomp_strm; > > @@ -179,12 +179,12 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, > } > > > -static int squashfs_decompress_multi(struct squashfs_sb_info *msblk, struct bio *bio, > +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, > int offset, int length, > struct squashfs_page_actor *output) > { > int res; > - struct squashfs_stream_multi *stream = msblk->stream; > + struct squashfs_stream *stream = msblk->stream; > struct decomp_stream *decomp_stream = get_decomp_stream(msblk, stream); > res = msblk->decompressor->decompress(msblk, decomp_stream->stream, > bio, offset, length, output); > @@ -196,8 +196,8 @@ static int squashfs_decompress_multi(struct squashfs_sb_info *msblk, struct bio > } > > const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { > - .create = squashfs_decompressor_create_multi, > - .destroy = squashfs_decompressor_destroy_multi, > - .decompress = squashfs_decompress_multi, > - .max_decompressors = squashfs_max_decompressors_multi, > + .create = squashfs_decompressor_create, > + .destroy = squashfs_decompressor_destroy, > + .decompress = squashfs_decompress, > + .max_decompressors = squashfs_max_decompressors, > }; > diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c > index e24455a7b04d..1dfadf76ed9a 100644 > --- a/fs/squashfs/decompressor_multi_percpu.c > +++ b/fs/squashfs/decompressor_multi_percpu.c > @@ -20,19 +20,19 @@ > * variables, one thread per cpu core. > */ > > -struct squashfs_stream_percpu { > +struct squashfs_stream { > void *stream; > local_lock_t lock; > }; > > -static void *squashfs_decompressor_create_percpu(struct squashfs_sb_info *msblk, > +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, > void *comp_opts) > { > - struct squashfs_stream_percpu *stream; > - struct squashfs_stream_percpu __percpu *percpu; > + struct squashfs_stream *stream; > + struct squashfs_stream __percpu *percpu; > int err, cpu; > > - percpu = alloc_percpu(struct squashfs_stream_percpu); > + percpu = alloc_percpu(struct squashfs_stream); > if (percpu == NULL) > return ERR_PTR(-ENOMEM); > > @@ -59,11 +59,11 @@ static void *squashfs_decompressor_create_percpu(struct squashfs_sb_info *msblk, > return ERR_PTR(err); > } > > -static void squashfs_decompressor_destroy_percpu(struct squashfs_sb_info *msblk) > +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) > { > - struct squashfs_stream_percpu __percpu *percpu = > - (struct squashfs_stream_percpu __percpu *) msblk->stream; > - struct squashfs_stream_percpu *stream; > + struct squashfs_stream __percpu *percpu = > + (struct squashfs_stream __percpu *) msblk->stream; > + struct squashfs_stream *stream; > int cpu; > > if (msblk->stream) { > @@ -75,12 +75,12 @@ static void squashfs_decompressor_destroy_percpu(struct squashfs_sb_info *msblk) > } > } > > -static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio *bio, > +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, > int offset, int length, struct squashfs_page_actor *output) > { > - struct squashfs_stream_percpu *stream; > - struct squashfs_stream_percpu __percpu *percpu = > - (struct squashfs_stream_percpu __percpu *) msblk->stream; > + struct squashfs_stream *stream; > + struct squashfs_stream __percpu *percpu = > + (struct squashfs_stream __percpu *) msblk->stream; > int res; > > local_lock(&percpu->lock); > @@ -98,14 +98,14 @@ static int squashfs_decompress_percpu(struct squashfs_sb_info *msblk, struct bio > return res; > } > > -static int squashfs_max_decompressors_percpu(void) > +static int squashfs_max_decompressors(void) > { > return num_possible_cpus(); > } > > const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { > - .create = squashfs_decompressor_create_percpu, > - .destroy = squashfs_decompressor_destroy_percpu, > - .decompress = squashfs_decompress_percpu, > - .max_decompressors = squashfs_max_decompressors_percpu, > + .create = squashfs_decompressor_create, > + .destroy = squashfs_decompressor_destroy, > + .decompress = squashfs_decompress, > + .max_decompressors = squashfs_max_decompressors, > }; > diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c > index 41449de0ea4c..6f161887710b 100644 > --- a/fs/squashfs/decompressor_single.c > +++ b/fs/squashfs/decompressor_single.c > @@ -19,15 +19,15 @@ > * decompressor framework > */ > > -struct squashfs_stream_single { > +struct squashfs_stream { > void *stream; > struct mutex mutex; > }; > > -static void *squashfs_decompressor_create_single(struct squashfs_sb_info *msblk, > +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, > void *comp_opts) > { > - struct squashfs_stream_single *stream; > + struct squashfs_stream *stream; > int err = -ENOMEM; > > stream = kmalloc(sizeof(*stream), GFP_KERNEL); > @@ -49,9 +49,9 @@ static void *squashfs_decompressor_create_single(struct squashfs_sb_info *msblk, > return ERR_PTR(err); > } > > -static void squashfs_decompressor_destroy_single(struct squashfs_sb_info *msblk) > +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) > { > - struct squashfs_stream_single *stream = msblk->stream; > + struct squashfs_stream *stream = msblk->stream; > > if (stream) { > msblk->decompressor->free(stream->stream); > @@ -59,12 +59,12 @@ static void squashfs_decompressor_destroy_single(struct squashfs_sb_info *msblk) > } > } > > -static int squashfs_decompress_single(struct squashfs_sb_info *msblk, struct bio *bio, > +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, > int offset, int length, > struct squashfs_page_actor *output) > { > int res; > - struct squashfs_stream_single *stream = msblk->stream; > + struct squashfs_stream *stream = msblk->stream; > > mutex_lock(&stream->mutex); > res = msblk->decompressor->decompress(msblk, stream->stream, bio, > @@ -78,14 +78,14 @@ static int squashfs_decompress_single(struct squashfs_sb_info *msblk, struct bio > return res; > } > > -static int squashfs_max_decompressors_single(void) > +static int squashfs_max_decompressors(void) > { > return 1; > } > > const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { > - .create = squashfs_decompressor_create_single, > - .destroy = squashfs_decompressor_destroy_single, > - .decompress = squashfs_decompress_single, > - .max_decompressors = squashfs_max_decompressors_single, > + .create = squashfs_decompressor_create, > + .destroy = squashfs_decompressor_destroy, > + .decompress = squashfs_decompress, > + .max_decompressors = squashfs_max_decompressors, > }; > diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h > index 9a383ad0dae0..9ba9e95440f8 100644 > --- a/fs/squashfs/squashfs.h > +++ b/fs/squashfs/squashfs.h > @@ -37,46 +37,6 @@ extern void *squashfs_read_table(struct super_block *, u64, int); > extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); > extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); > > -/* decompressor_xxx.c */ > - > -struct squashfs_decompressor_thread_ops { > - void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); > - void (*destroy)(struct squashfs_sb_info *msblk); > - int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, > - int offset, int length, struct squashfs_page_actor *output); > - int (*max_decompressors)(void); > -}; > - > -#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > -extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; > -#endif > -#ifdef CONFIG_SQUASHFS_DECOMP_MULTI > -extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; > -#endif > -#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > -extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; > -#endif > - > -static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) > -{ > - return msblk->thread_ops->create(msblk, comp_opts); > -} > - > -static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) > -{ > - msblk->thread_ops->destroy(msblk); > -} > - > -static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, > - int offset, int length, struct squashfs_page_actor *output) > -{ > - return msblk->thread_ops->decompress(msblk, bio, offset, length, output); > -} > - > -static inline int squashfs_max_decompressors(struct squashfs_sb_info *msblk) > -{ > - return msblk->thread_ops->max_decompressors(); > -} > /* export.c */ > extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, > unsigned int); > diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c > index fd4e70d45f3c..d7dc4b304aa0 100644 > --- a/fs/squashfs/super.c > +++ b/fs/squashfs/super.c > @@ -36,6 +36,7 @@ > #include "squashfs.h" > #include "decompressor.h" > #include "xattr.h" > +#include "threading.h" > > static struct file_system_type squashfs_fs_type; > static const struct super_operations squashfs_super_ops; > diff --git a/fs/squashfs/threading.h b/fs/squashfs/threading.h > new file mode 100644 > index 000000000000..bd06519fb8b9 > --- /dev/null > +++ b/fs/squashfs/threading.h > @@ -0,0 +1,30 @@ > +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; > +#endif > +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI > +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; > +#endif > +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; > +#endif > + > +static inline void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) > +{ > + return msblk->thread_ops->create(msblk, comp_opts); > +} > + > +static inline void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) > +{ > + msblk->thread_ops->destroy(msblk); > +} > + > +static inline int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, > + int offset, int length, struct squashfs_page_actor *output) > +{ > + return msblk->thread_ops->decompress(msblk, bio, offset, length, output); > +} > + > +static inline int squashfs_max_decompressors(struct squashfs_sb_info *msblk) > +{ > + return msblk->thread_ops->max_decompressors(); > +} > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-02 9:48 ` [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-09-09 15:44 ` Phillip Lougher @ 2022-09-09 15:50 ` Phillip Lougher 2022-09-13 2:47 ` Xiaoming Ni 1 sibling, 1 reply; 38+ messages in thread From: Phillip Lougher @ 2022-09-09 15:50 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 02/09/2022 10:48, Xiaoming Ni wrote: > Squashfs supports three decompression concurrency modes: > Single-thread mode: concurrent reads are blocked and the memory overhead > is small. > Multi-thread mode/percpu mode: reduces concurrent read blocking but > increases memory overhead. > > The corresponding schema must be fixed at compile time. During mounting, > the concurrent decompression mode cannot be adjusted based on file read > blocking. > > The mount parameter theads=<single|multi|percpu> is added to select > the concurrent decompression mode of a single SquashFS file system > image. > > Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> Additional review comment ... [SNIP] > diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c > index 32565dafa7f3..fd4e70d45f3c 100644 > --- a/fs/squashfs/super.c > +++ b/fs/squashfs/super.c > @@ -47,10 +47,12 @@ enum Opt_errors { [SNIP] > +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > + opts->thread_ops = &squashfs_decompressor_single; > +#elif CONFIG_SQUASHFS_DECOMP_MULTI this should be #elif defined CONFIG_SQUASHFS_DECOMP_MULTI > + opts->thread_ops = &squashfs_decompressor_multi; > +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU this should be #elif defined CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU Phillip > + opts->thread_ops = &squashfs_decompressor_percpu; > +#endif > fc->fs_private = opts; > fc->ops = &squashfs_context_ops; > return 0; ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-09 15:50 ` Phillip Lougher @ 2022-09-13 2:47 ` Xiaoming Ni 0 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-13 2:47 UTC (permalink / raw) To: Phillip Lougher, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 2022/9/9 23:50, Phillip Lougher wrote: > On 02/09/2022 10:48, Xiaoming Ni wrote: >> Squashfs supports three decompression concurrency modes: >> Single-thread mode: concurrent reads are blocked and the memory >> overhead >> is small. >> Multi-thread mode/percpu mode: reduces concurrent read blocking but >> increases memory overhead. >> >> The corresponding schema must be fixed at compile time. During mounting, >> the concurrent decompression mode cannot be adjusted based on file read >> blocking. >> >> The mount parameter theads=<single|multi|percpu> is added to select >> the concurrent decompression mode of a single SquashFS file system >> image. >> >> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > > Additional review comment ... > > [SNIP] > >> diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c >> index 32565dafa7f3..fd4e70d45f3c 100644 >> --- a/fs/squashfs/super.c >> +++ b/fs/squashfs/super.c >> @@ -47,10 +47,12 @@ enum Opt_errors { > > [SNIP] > >> +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE >> + opts->thread_ops = &squashfs_decompressor_single; > +#elif >> CONFIG_SQUASHFS_DECOMP_MULTI > > this should be > > #elif defined CONFIG_SQUASHFS_DECOMP_MULTI > Thanks for your comment, I'll fix it in the next patch version >> + opts->thread_ops = &squashfs_decompressor_multi; >> +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > > this should be > > #elif defined CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > Thanks for your comment, I'll fix it in the next patch version > Phillip > >> + opts->thread_ops = &squashfs_decompressor_percpu; >> +#endif >> fc->fs_private = opts; >> fc->ops = &squashfs_context_ops; >> return 0; > > > . Thanks Xiaoming Ni ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 2/2] squashfs: Allows users to configure the number of decompression threads. 2022-09-02 9:48 ` [PATCH v3 " Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-09-02 9:48 ` Xiaoming Ni 2022-09-09 15:26 ` [PATCH v3 0/2] squashfs: Add the mount parameter "threads=" Phillip Lougher 2022-09-16 8:36 ` [PATCH v4 " Xiaoming Ni 3 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-02 9:48 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 The maximum number of threads in the decompressor_multi.c file is fixed and cannot be adjusted according to user needs. Therefore, the mount parameter needs to be added to allow users to configure the number of threads as required. The upper limit is num_online_cpus() * 2. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 17 +++++++++-- fs/squashfs/decompressor_multi.c | 4 +-- fs/squashfs/squashfs_fs_sb.h | 1 + fs/squashfs/super.c | 63 +++++++++++++++++++++++++++++++++------- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 9c2827459f40..60fc98bdf421 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -73,12 +73,10 @@ config SQUASHFS_CHOICE_DECOMP_BY_MOUNT select SQUASHFS_DECOMP_SINGLE select SQUASHFS_DECOMP_MULTI select SQUASHFS_DECOMP_MULTI_PERCPU + select SQUASHFS_MOUNT_DECOMP_THREADS help Compile all parallel decompression modes and specify the decompression mode by setting "threads=" during mount. - - threads=<single|multi|percpu> - default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE choice @@ -128,6 +126,19 @@ config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU decompression is load-balanced across the cores. endchoice +config SQUASHFS_MOUNT_DECOMP_THREADS + bool "Add the mount parameter 'threads=' for squashfs" + depends on SQUASHFS + depends on SQUASHFS_DECOMP_MULTI + default n + help + Use threads= to set the decompression parallel mode and the number of threads. + If SQUASHFS_CHOICE_DECOMP_BY_MOUNT=y + threads=<single|multi|percpu|1|2|3|...> + else + threads=<2|3|...> + The upper limit is num_online_cpus() * 2. + config SQUASHFS_XATTR bool "Squashfs XATTR support" depends on SQUASHFS diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index 7b2723b77e75..6d1cea325cca 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -144,7 +144,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, * If there is no available decomp and already full, * let's wait for releasing decomp from other users. */ - if (stream->avail_decomp >= MAX_DECOMPRESSOR) + if (stream->avail_decomp >= msblk->max_thread_num) goto wait; /* Let's allocate new decomp */ @@ -160,7 +160,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } stream->avail_decomp++; - WARN_ON(stream->avail_decomp > MAX_DECOMPRESSOR); + WARN_ON(stream->avail_decomp > msblk->max_thread_num); mutex_unlock(&stream->mutex); break; diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index f1e5dad8ae0a..659082e9e51d 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -67,5 +67,6 @@ struct squashfs_sb_info { unsigned int ids; bool panic_on_errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int max_thread_num; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index fd4e70d45f3c..5705749e7d44 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -53,6 +53,7 @@ enum squashfs_param { struct squashfs_mount_opts { enum Opt_errors errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int thread_num; }; static const struct constant_table squashfs_param_errors[] = { @@ -67,7 +68,8 @@ static const struct fs_parameter_spec squashfs_fs_parameters[] = { {} }; -static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) + +static int squashfs_parse_param_threads_str(const char *str, struct squashfs_mount_opts *opts) { #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT if (strcmp(str, "single") == 0) { @@ -86,6 +88,42 @@ static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_o return -EINVAL; } +static int squashfs_parse_param_threads_num(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + int ret; + unsigned long num; + + ret = kstrtoul(str, 0, &num); + if (ret != 0) + return -EINVAL; + if (num > 1) { + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + return -EINVAL; + opts->thread_num = (int)num; + return 0; + } +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (num == 1) { + opts->thread_ops = &squashfs_decompressor_single; + opts->thread_num = 1; + return 0; + } +#endif +#endif /* !CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS */ + return -EINVAL; +} + +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ + int ret = squashfs_parse_param_threads_str(str, opts); + + if (ret == 0) + return ret; + return squashfs_parse_param_threads_num(str, opts); +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -194,6 +232,11 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) goto failed_mount; } msblk->thread_ops = opts->thread_ops; + if (opts->thread_num == 0) { + msblk->max_thread_num = squashfs_max_decompressors(msblk); + } else { + msblk->max_thread_num = opts->thread_num; + } /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -279,7 +322,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - squashfs_max_decompressors(msblk), msblk->block_size); + msblk->max_thread_num, msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -463,18 +506,17 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) seq_puts(s, ",errors=continue"); #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT - if (msblk->thread_ops == &squashfs_decompressor_single) { - seq_puts(s, ",threads=single"); - return 0; - } - if (msblk->thread_ops == &squashfs_decompressor_multi) { - seq_puts(s, ",threads=multi"); - return 0; - } if (msblk->thread_ops == &squashfs_decompressor_percpu) { seq_puts(s, ",threads=percpu"); return 0; } + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + seq_printf(s, ",threads=%d", msblk->max_thread_num); #endif return 0; } @@ -494,6 +536,7 @@ static int squashfs_init_fs_context(struct fs_context *fc) #elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU opts->thread_ops = &squashfs_decompressor_percpu; #endif + opts->thread_num = 0; fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 0/2] squashfs: Add the mount parameter "threads=" 2022-09-02 9:48 ` [PATCH v3 " Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni @ 2022-09-09 15:26 ` Phillip Lougher 2022-09-16 8:36 ` [PATCH v4 " Xiaoming Ni 3 siblings, 0 replies; 38+ messages in thread From: Phillip Lougher @ 2022-09-09 15:26 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 02/09/2022 10:48, Xiaoming Ni wrote: > Currently, Squashfs supports multiple decompressor parallel modes. However, this > mode can be configured only during kernel building and does not support flexible > selection during runtime. > > In the current patch set, the mount parameter "threads=" is added to allow users > to select the parallel decompressor mode and configure the number of decompressors > when mounting a file system. > > "threads=<single|multi|percpu|1|2|3|...>" > The upper limit is num_online_cpus() * 2. > > > > v3: Based on Philip Lougher's suggestion, make the following updates: > 1. The default configuration is the same as that before the patch installation. > 2. Compile the three decompression modes when the new configuration is enabled. > 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. > Hi, This patch-set looks a lot better IMHO. I only have a couple of relatively minor issues, which will be dealt with as comments on the patches. Phillip > v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ > fix warning: sparse: incorrect type in initializer (different address spaces) > Reported-by: kernel test robot <lkp@intel.com> > > v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ > > Xiaoming Ni (2): > squashfs: add the mount parameter theads=<single|multi|percpu> > squashfs: Allows users to configure the number of decompression > threads. > > fs/squashfs/Kconfig | 51 ++++++++++++++++-- > fs/squashfs/decompressor_multi.c | 32 +++++++----- > fs/squashfs/decompressor_multi_percpu.c | 39 ++++++++------ > fs/squashfs/decompressor_single.c | 23 +++++--- > fs/squashfs/squashfs.h | 43 +++++++++++++-- > fs/squashfs/squashfs_fs_sb.h | 4 +- > fs/squashfs/super.c | 93 ++++++++++++++++++++++++++++++++- > 7 files changed, 237 insertions(+), 48 deletions(-) > ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v4 0/2] squashfs: Add the mount parameter "threads=" 2022-09-02 9:48 ` [PATCH v3 " Xiaoming Ni ` (2 preceding siblings ...) 2022-09-09 15:26 ` [PATCH v3 0/2] squashfs: Add the mount parameter "threads=" Phillip Lougher @ 2022-09-16 8:36 ` Xiaoming Ni 2022-09-16 8:36 ` [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni ` (3 more replies) 3 siblings, 4 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-16 8:36 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Currently, Squashfs supports multiple decompressor parallel modes. However, this mode can be configured only during kernel building and does not support flexible selection during runtime. In the current patch set, the mount parameter "threads=" is added to allow users to select the parallel decompressor mode and configure the number of decompressors when mounting a file system. "threads=<single|multi|percpu|1|2|3|...>" The upper limit is num_online_cpus() * 2. v4: Based on Philip Lougher's suggestion, make the following updates: 1. Use static modifiers to avoid changing symbol names. 2. Fixed some formatting issues v3: https://lore.kernel.org/lkml/20220902094855.22666-1-nixiaoming@huawei.com/ Based on Philip Lougher's suggestion, make the following updates: 1. The default configuration is the same as that before the patch installation. 2. Compile the three decompression modes when the new configuration is enabled. 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ fix warning: sparse: incorrect type in initializer (different address spaces) Reported-by: kernel test robot <lkp@intel.com> v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ Xiaoming Ni (2): squashfs: add the mount parameter theads=<single|multi|percpu> squashfs: Allows users to configure the number of decompression threads fs/squashfs/Kconfig | 51 +++++++++++++++-- fs/squashfs/block.c | 2 +- fs/squashfs/decompressor.c | 2 +- fs/squashfs/decompressor_multi.c | 20 ++++--- fs/squashfs/decompressor_multi_percpu.c | 23 +++++--- fs/squashfs/decompressor_single.c | 15 +++-- fs/squashfs/squashfs.h | 23 ++++++-- fs/squashfs/squashfs_fs_sb.h | 4 +- fs/squashfs/super.c | 97 ++++++++++++++++++++++++++++++++- 9 files changed, 203 insertions(+), 34 deletions(-) -- 2.12.3 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-16 8:36 ` [PATCH v4 " Xiaoming Ni @ 2022-09-16 8:36 ` Xiaoming Ni 2022-09-28 2:20 ` Phillip Lougher 2022-09-16 8:36 ` [PATCH v4 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni ` (2 subsequent siblings) 3 siblings, 1 reply; 38+ messages in thread From: Xiaoming Ni @ 2022-09-16 8:36 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 Squashfs supports three decompression concurrency modes: Single-thread mode: concurrent reads are blocked and the memory overhead is small. Multi-thread mode/percpu mode: reduces concurrent read blocking but increases memory overhead. The corresponding schema must be fixed at compile time. During mounting, the concurrent decompression mode cannot be adjusted based on file read blocking. The mount parameter theads=<single|multi|percpu> is added to select the concurrent decompression mode of a single SquashFS file system image. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 39 +++++++++++++++++++++--- fs/squashfs/block.c | 2 +- fs/squashfs/decompressor.c | 2 +- fs/squashfs/decompressor_multi.c | 16 +++++++--- fs/squashfs/decompressor_multi_percpu.c | 23 +++++++++----- fs/squashfs/decompressor_single.c | 15 ++++++--- fs/squashfs/squashfs.h | 23 +++++++++++--- fs/squashfs/squashfs_fs_sb.h | 3 +- fs/squashfs/super.c | 54 +++++++++++++++++++++++++++++++-- 9 files changed, 145 insertions(+), 32 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 916e78fabcaa..218bacdd4298 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -54,9 +54,36 @@ config SQUASHFS_FILE_DIRECT endchoice +config SQUASHFS_DECOMP_SINGLE + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI_PERCPU + depends on SQUASHFS + def_bool n + +config SQUASHFS_CHOICE_DECOMP_BY_MOUNT + bool "Select the parallel decompression mode during mount" + depends on SQUASHFS + default n + select SQUASHFS_DECOMP_SINGLE + select SQUASHFS_DECOMP_MULTI + select SQUASHFS_DECOMP_MULTI_PERCPU + help + Compile all parallel decompression modes and specify the + decompression mode by setting "threads=" during mount. + threads=<single|multi|percpu> + + default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE + choice - prompt "Decompressor parallelisation options" + prompt "Select decompression parallel mode at compile time" depends on SQUASHFS + depends on !SQUASHFS_CHOICE_DECOMP_BY_MOUNT help Squashfs now supports three parallelisation options for decompression. Each one exhibits various trade-offs between @@ -64,15 +91,17 @@ choice If in doubt, select "Single threaded compression" -config SQUASHFS_DECOMP_SINGLE +config SQUASHFS_COMPILE_DECOMP_SINGLE bool "Single threaded compression" + select SQUASHFS_DECOMP_SINGLE help Traditionally Squashfs has used single-threaded decompression. Only one block (data or metadata) can be decompressed at any one time. This limits CPU and memory usage to a minimum. -config SQUASHFS_DECOMP_MULTI +config SQUASHFS_COMPILE_DECOMP_MULTI bool "Use multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -85,8 +114,9 @@ config SQUASHFS_DECOMP_MULTI decompressors per core. It dynamically allocates decompressors on a demand basis. -config SQUASHFS_DECOMP_MULTI_PERCPU +config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI_PERCPU help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -95,7 +125,6 @@ config SQUASHFS_DECOMP_MULTI_PERCPU This decompressor implementation uses a maximum of one decompressor per core. It uses percpu variables to ensure decompression is load-balanced across the cores. - endchoice config SQUASHFS_XATTR diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 833aca92301f..bed3bb8b27fa 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -216,7 +216,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, res = -EIO; goto out_free_bio; } - res = squashfs_decompress(msblk, bio, offset, length, output); + res = msblk->thread_ops->decompress(msblk, bio, offset, length, output); } else { res = copy_bio_to_actor(bio, output, offset, length); } diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c index d57bef91ab08..8893cb9b4198 100644 --- a/fs/squashfs/decompressor.c +++ b/fs/squashfs/decompressor.c @@ -134,7 +134,7 @@ void *squashfs_decompressor_setup(struct super_block *sb, unsigned short flags) if (IS_ERR(comp_opts)) return comp_opts; - stream = squashfs_decompressor_create(msblk, comp_opts); + stream = msblk->thread_ops->create(msblk, comp_opts); if (IS_ERR(stream)) kfree(comp_opts); diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index db9f12a3ea05..eb25432bd149 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -29,12 +29,11 @@ #define MAX_DECOMPRESSOR (num_online_cpus() * 2) -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return MAX_DECOMPRESSOR; } - struct squashfs_stream { void *comp_opts; struct list_head strm_list; @@ -59,7 +58,7 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, wake_up(&stream->wait); } -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -103,7 +102,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream *stream = msblk->stream; if (stream) { @@ -180,7 +179,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { @@ -195,3 +194,10 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, msblk->decompressor->name); return res; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c index b881b9283b7f..1dfadf76ed9a 100644 --- a/fs/squashfs/decompressor_multi_percpu.c +++ b/fs/squashfs/decompressor_multi_percpu.c @@ -25,7 +25,7 @@ struct squashfs_stream { local_lock_t lock; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -59,7 +59,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream __percpu *percpu = (struct squashfs_stream __percpu *) msblk->stream; @@ -75,19 +75,21 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { struct squashfs_stream *stream; + struct squashfs_stream __percpu *percpu = + (struct squashfs_stream __percpu *) msblk->stream; int res; - local_lock(&msblk->stream->lock); - stream = this_cpu_ptr(msblk->stream); + local_lock(&percpu->lock); + stream = this_cpu_ptr(percpu); res = msblk->decompressor->decompress(msblk, stream->stream, bio, offset, length, output); - local_unlock(&msblk->stream->lock); + local_unlock(&percpu->lock); if (res < 0) ERROR("%s decompression failed, data probably corrupt\n", @@ -96,7 +98,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return num_possible_cpus(); } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c index 4eb3d083d45e..6f161887710b 100644 --- a/fs/squashfs/decompressor_single.c +++ b/fs/squashfs/decompressor_single.c @@ -24,7 +24,7 @@ struct squashfs_stream { struct mutex mutex; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -49,7 +49,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream *stream = msblk->stream; @@ -59,7 +59,7 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { @@ -78,7 +78,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return 1; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h index 9783e01c8100..a6164fdf9435 100644 --- a/fs/squashfs/squashfs.h +++ b/fs/squashfs/squashfs.h @@ -38,11 +38,24 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); /* decompressor_xxx.c */ -extern void *squashfs_decompressor_create(struct squashfs_sb_info *, void *); -extern void squashfs_decompressor_destroy(struct squashfs_sb_info *); -extern int squashfs_decompress(struct squashfs_sb_info *, struct bio *, - int, int, struct squashfs_page_actor *); -extern int squashfs_max_decompressors(void); + +struct squashfs_decompressor_thread_ops { + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); + void (*destroy)(struct squashfs_sb_info *msblk); + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output); + int (*max_decompressors)(void); +}; + +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; +#endif /* export.c */ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index 1e90c2575f9b..f1e5dad8ae0a 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -53,7 +53,7 @@ struct squashfs_sb_info { __le64 *xattr_id_table; struct mutex meta_index_mutex; struct meta_index *meta_index; - struct squashfs_stream *stream; + void *stream; __le64 *inode_lookup_table; u64 inode_table; u64 directory_table; @@ -66,5 +66,6 @@ struct squashfs_sb_info { int xattr_ids; unsigned int ids; bool panic_on_errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 32565dafa7f3..2a33ffe85292 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -47,10 +47,12 @@ enum Opt_errors { enum squashfs_param { Opt_errors, + Opt_threads, }; struct squashfs_mount_opts { enum Opt_errors errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; static const struct constant_table squashfs_param_errors[] = { @@ -61,9 +63,29 @@ static const struct constant_table squashfs_param_errors[] = { static const struct fs_parameter_spec squashfs_fs_parameters[] = { fsparam_enum("errors", Opt_errors, squashfs_param_errors), + fsparam_string("threads", Opt_threads), {} }; +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (strcmp(str, "single") == 0) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } + if (strcmp(str, "multi") == 0) { + opts->thread_ops = &squashfs_decompressor_multi; + return 0; + } + if (strcmp(str, "percpu") == 0) { + opts->thread_ops = &squashfs_decompressor_percpu; + return 0; + } +#endif + return -EINVAL; +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -78,6 +100,10 @@ static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *para case Opt_errors: opts->errors = result.uint_32; break; + case Opt_threads: + if (squashfs_parse_param_threads(param->string, opts) != 0) + return -EINVAL; + break; default: return -EINVAL; } @@ -167,6 +193,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_bdev); goto failed_mount; } + msblk->thread_ops = opts->thread_ops; /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -252,7 +279,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - squashfs_max_decompressors(), msblk->block_size); + msblk->thread_ops->max_decompressors(), msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -383,7 +410,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) squashfs_cache_delete(msblk->block_cache); squashfs_cache_delete(msblk->fragment_cache); squashfs_cache_delete(msblk->read_page); - squashfs_decompressor_destroy(msblk); + msblk->thread_ops->destroy(msblk); kfree(msblk->inode_lookup_table); kfree(msblk->fragment_index); kfree(msblk->id_table); @@ -435,6 +462,20 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_multi) { + seq_puts(s, ",threads=multi"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_percpu) { + seq_puts(s, ",threads=percpu"); + return 0; + } +#endif return 0; } @@ -446,6 +487,13 @@ static int squashfs_init_fs_context(struct fs_context *fc) if (!opts) return -ENOMEM; +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + opts->thread_ops = &squashfs_decompressor_single; +#elif CONFIG_SQUASHFS_DECOMP_MULTI + opts->thread_ops = &squashfs_decompressor_multi; +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + opts->thread_ops = &squashfs_decompressor_percpu; +#endif fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; @@ -478,7 +526,7 @@ static void squashfs_put_super(struct super_block *sb) squashfs_cache_delete(sbi->block_cache); squashfs_cache_delete(sbi->fragment_cache); squashfs_cache_delete(sbi->read_page); - squashfs_decompressor_destroy(sbi); + sbi->thread_ops->destroy(sbi); kfree(sbi->id_table); kfree(sbi->fragment_index); kfree(sbi->meta_index); -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-16 8:36 ` [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-09-28 2:20 ` Phillip Lougher 2022-09-28 3:06 ` Xiaoming Ni 0 siblings, 1 reply; 38+ messages in thread From: Phillip Lougher @ 2022-09-28 2:20 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 16/09/2022 09:36, Xiaoming Ni wrote: > Squashfs supports three decompression concurrency modes: > Single-thread mode: concurrent reads are blocked and the memory > overhead is small. > Multi-thread mode/percpu mode: reduces concurrent read blocking but > increases memory overhead. > > The corresponding schema must be fixed at compile time. During mounting, > the concurrent decompression mode cannot be adjusted based on file read > blocking. > > The mount parameter theads=<single|multi|percpu> is added to select > the concurrent decompression mode of a single SquashFS file system > image. > > Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > > +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE > + opts->thread_ops = &squashfs_decompressor_single; > +#elif CONFIG_SQUASHFS_DECOMP_MULTI > + opts->thread_ops = &squashfs_decompressor_multi; > +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU In my previous review I asked you to fix the above two #elif lines. You have done so in this patch series, but, only in the second patch which seems perverse. The reason why this isn't a good approach. If you *only* apply this patch, with the following Squashfs configuration options phillip@phoenix:/external/stripe/linux$ grep SQUASHFS .config CONFIG_SQUASHFS=y # CONFIG_SQUASHFS_FILE_CACHE is not set CONFIG_SQUASHFS_FILE_DIRECT=y CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU=y # CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set # CONFIG_SQUASHFS_COMPILE_DECOMP_SINGLE is not set # CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI is not set CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU=y CONFIG_SQUASHFS_XATTR=y CONFIG_SQUASHFS_ZLIB=y CONFIG_SQUASHFS_LZ4=y CONFIG_SQUASHFS_LZO=y CONFIG_SQUASHFS_XZ=y CONFIG_SQUASHFS_ZSTD=y # CONFIG_SQUASHFS_4K_DEVBLK_SIZE is not set # CONFIG_SQUASHFS_EMBEDDED is not set CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE=3 You will get the following build warning phillip@phoenix:/external/stripe/linux$ make bzImage SYNC include/config/auto.conf.cmd CALL scripts/checksyscalls.sh CALL scripts/atomic/check-atomics.sh DESCEND objtool CHK include/generated/compile.h UPD kernel/config_data GZIP kernel/config_data.gz CC kernel/configs.o AR kernel/built-in.a CC fs/squashfs/block.o CC fs/squashfs/cache.o CC fs/squashfs/dir.o CC fs/squashfs/export.o CC fs/squashfs/file.o CC fs/squashfs/fragment.o CC fs/squashfs/id.o CC fs/squashfs/inode.o CC fs/squashfs/namei.o CC fs/squashfs/super.o fs/squashfs/super.c: In function ‘squashfs_init_fs_context’: fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is not defined, evaluates to 0 [-Wundef] 492 | #elif CONFIG_SQUASHFS_DECOMP_MULTI | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ This strictly breaks the git bisectability rule. Every patch should be compilable and not break the build or produce warnings. If not it makes git bisect difficult to use to find regressions. This can be avoided by fixing the issue in *this* patch. So please do so. Thanks Phillip > + opts->thread_ops = &squashfs_decompressor_percpu; > +#endif > fc->fs_private = opts; > fc->ops = &squashfs_context_ops; > return 0; > @@ -478,7 +526,7 @@ static void squashfs_put_super(struct super_block *sb) > squashfs_cache_delete(sbi->block_cache); > squashfs_cache_delete(sbi->fragment_cache); > squashfs_cache_delete(sbi->read_page); > - squashfs_decompressor_destroy(sbi); > + sbi->thread_ops->destroy(sbi); > kfree(sbi->id_table); > kfree(sbi->fragment_index); > kfree(sbi->meta_index); ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-28 2:20 ` Phillip Lougher @ 2022-09-28 3:06 ` Xiaoming Ni 0 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-28 3:06 UTC (permalink / raw) To: Phillip Lougher, linux-kernel Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 On 2022/9/28 10:20, Phillip Lougher wrote: > On 16/09/2022 09:36, Xiaoming Ni wrote: >> Squashfs supports three decompression concurrency modes: >> Single-thread mode: concurrent reads are blocked and the memory >> overhead is small. >> Multi-thread mode/percpu mode: reduces concurrent read blocking but >> increases memory overhead. >> >> The corresponding schema must be fixed at compile time. During mounting, >> the concurrent decompression mode cannot be adjusted based on file read >> blocking. >> >> The mount parameter theads=<single|multi|percpu> is added to select >> the concurrent decompression mode of a single SquashFS file system >> image. >> >> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> >> +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE >> + opts->thread_ops = &squashfs_decompressor_single; >> +#elif CONFIG_SQUASHFS_DECOMP_MULTI >> + opts->thread_ops = &squashfs_decompressor_multi; >> +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > > In my previous review I asked you to fix the above two #elif > lines. You have done so in this patch series, but, only in the > second patch which seems perverse. > > The reason why this isn't a good approach. If you *only* apply this > patch, with the following Squashfs configuration options > I'm so sorry. I made a low-level mistake in patching. I will re-check the previous review comments and resend the patch. Thanks > phillip@phoenix:/external/stripe/linux$ grep SQUASHFS .config > CONFIG_SQUASHFS=y > # CONFIG_SQUASHFS_FILE_CACHE is not set > CONFIG_SQUASHFS_FILE_DIRECT=y > CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU=y > # CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set > # CONFIG_SQUASHFS_COMPILE_DECOMP_SINGLE is not set > # CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI is not set > CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU=y > CONFIG_SQUASHFS_XATTR=y > CONFIG_SQUASHFS_ZLIB=y > CONFIG_SQUASHFS_LZ4=y > CONFIG_SQUASHFS_LZO=y > CONFIG_SQUASHFS_XZ=y > CONFIG_SQUASHFS_ZSTD=y > # CONFIG_SQUASHFS_4K_DEVBLK_SIZE is not set > # CONFIG_SQUASHFS_EMBEDDED is not set > CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE=3 > > > You will get the following build warning > > phillip@phoenix:/external/stripe/linux$ make bzImage > SYNC include/config/auto.conf.cmd > CALL scripts/checksyscalls.sh > CALL scripts/atomic/check-atomics.sh > DESCEND objtool > CHK include/generated/compile.h > UPD kernel/config_data > GZIP kernel/config_data.gz > CC kernel/configs.o > AR kernel/built-in.a > CC fs/squashfs/block.o > CC fs/squashfs/cache.o > CC fs/squashfs/dir.o > CC fs/squashfs/export.o > CC fs/squashfs/file.o > CC fs/squashfs/fragment.o > CC fs/squashfs/id.o > CC fs/squashfs/inode.o > CC fs/squashfs/namei.o > CC fs/squashfs/super.o > fs/squashfs/super.c: In function ‘squashfs_init_fs_context’: > fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is > not defined, evaluates to 0 [-Wundef] > 492 | #elif CONFIG_SQUASHFS_DECOMP_MULTI > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > This strictly breaks the git bisectability rule. Every patch should > be compilable and not break the build or produce warnings. If not > it makes git bisect difficult to use to find regressions. > > This can be avoided by fixing the issue in *this* patch. So > please do so. > > Thanks > > Phillip > >> + opts->thread_ops = &squashfs_decompressor_percpu; >> +#endif >> fc->fs_private = opts; >> fc->ops = &squashfs_context_ops; >> return 0; >> @@ -478,7 +526,7 @@ static void squashfs_put_super(struct super_block >> *sb) >> squashfs_cache_delete(sbi->block_cache); >> squashfs_cache_delete(sbi->fragment_cache); >> squashfs_cache_delete(sbi->read_page); >> - squashfs_decompressor_destroy(sbi); >> + sbi->thread_ops->destroy(sbi); >> kfree(sbi->id_table); >> kfree(sbi->fragment_index); >> kfree(sbi->meta_index); > > > . ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v4 2/2] squashfs: Allows users to configure the number of decompression threads 2022-09-16 8:36 ` [PATCH v4 " Xiaoming Ni 2022-09-16 8:36 ` [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-09-16 8:36 ` Xiaoming Ni 2022-09-27 1:05 ` ping //Re: [PATCH v4 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 " Xiaoming Ni 3 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-16 8:36 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 The maximum number of threads in the decompressor_multi.c file is fixed and cannot be adjusted according to user needs. Therefore, the mount parameter needs to be added to allow users to configure the number of threads as required. The upper limit is num_online_cpus() * 2. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 16 ++++++++-- fs/squashfs/decompressor_multi.c | 4 +-- fs/squashfs/squashfs_fs_sb.h | 1 + fs/squashfs/super.c | 67 +++++++++++++++++++++++++++++++++------- 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 218bacdd4298..60fc98bdf421 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -73,11 +73,10 @@ config SQUASHFS_CHOICE_DECOMP_BY_MOUNT select SQUASHFS_DECOMP_SINGLE select SQUASHFS_DECOMP_MULTI select SQUASHFS_DECOMP_MULTI_PERCPU + select SQUASHFS_MOUNT_DECOMP_THREADS help Compile all parallel decompression modes and specify the decompression mode by setting "threads=" during mount. - threads=<single|multi|percpu> - default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE choice @@ -127,6 +126,19 @@ config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU decompression is load-balanced across the cores. endchoice +config SQUASHFS_MOUNT_DECOMP_THREADS + bool "Add the mount parameter 'threads=' for squashfs" + depends on SQUASHFS + depends on SQUASHFS_DECOMP_MULTI + default n + help + Use threads= to set the decompression parallel mode and the number of threads. + If SQUASHFS_CHOICE_DECOMP_BY_MOUNT=y + threads=<single|multi|percpu|1|2|3|...> + else + threads=<2|3|...> + The upper limit is num_online_cpus() * 2. + config SQUASHFS_XATTR bool "Squashfs XATTR support" depends on SQUASHFS diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index eb25432bd149..416c53eedbd1 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -144,7 +144,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, * If there is no available decomp and already full, * let's wait for releasing decomp from other users. */ - if (stream->avail_decomp >= MAX_DECOMPRESSOR) + if (stream->avail_decomp >= msblk->max_thread_num) goto wait; /* Let's allocate new decomp */ @@ -160,7 +160,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } stream->avail_decomp++; - WARN_ON(stream->avail_decomp > MAX_DECOMPRESSOR); + WARN_ON(stream->avail_decomp > msblk->max_thread_num); mutex_unlock(&stream->mutex); break; diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index f1e5dad8ae0a..659082e9e51d 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -67,5 +67,6 @@ struct squashfs_sb_info { unsigned int ids; bool panic_on_errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int max_thread_num; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 2a33ffe85292..cd7b3e530dcf 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -53,6 +53,7 @@ enum squashfs_param { struct squashfs_mount_opts { enum Opt_errors errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int thread_num; }; static const struct constant_table squashfs_param_errors[] = { @@ -67,7 +68,8 @@ static const struct fs_parameter_spec squashfs_fs_parameters[] = { {} }; -static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) + +static int squashfs_parse_param_threads_str(const char *str, struct squashfs_mount_opts *opts) { #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT if (strcmp(str, "single") == 0) { @@ -86,6 +88,42 @@ static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_o return -EINVAL; } +static int squashfs_parse_param_threads_num(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + int ret; + unsigned long num; + + ret = kstrtoul(str, 0, &num); + if (ret != 0) + return -EINVAL; + if (num > 1) { + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + return -EINVAL; + opts->thread_num = (int)num; + return 0; + } +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (num == 1) { + opts->thread_ops = &squashfs_decompressor_single; + opts->thread_num = 1; + return 0; + } +#endif +#endif /* !CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS */ + return -EINVAL; +} + +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ + int ret = squashfs_parse_param_threads_str(str, opts); + + if (ret == 0) + return ret; + return squashfs_parse_param_threads_num(str, opts); +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -194,6 +232,11 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) goto failed_mount; } msblk->thread_ops = opts->thread_ops; + if (opts->thread_num == 0) { + msblk->max_thread_num = msblk->thread_ops->max_decompressors(); + } else { + msblk->max_thread_num = opts->thread_num; + } /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -279,7 +322,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - msblk->thread_ops->max_decompressors(), msblk->block_size); + msblk->max_thread_num, msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -463,18 +506,17 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) seq_puts(s, ",errors=continue"); #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT - if (msblk->thread_ops == &squashfs_decompressor_single) { - seq_puts(s, ",threads=single"); - return 0; - } - if (msblk->thread_ops == &squashfs_decompressor_multi) { - seq_puts(s, ",threads=multi"); - return 0; - } if (msblk->thread_ops == &squashfs_decompressor_percpu) { seq_puts(s, ",threads=percpu"); return 0; } + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + seq_printf(s, ",threads=%d", msblk->max_thread_num); #endif return 0; } @@ -489,11 +531,12 @@ static int squashfs_init_fs_context(struct fs_context *fc) #ifdef CONFIG_SQUASHFS_DECOMP_SINGLE opts->thread_ops = &squashfs_decompressor_single; -#elif CONFIG_SQUASHFS_DECOMP_MULTI +#elif defined(CONFIG_SQUASHFS_DECOMP_MULTI) opts->thread_ops = &squashfs_decompressor_multi; -#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +#elif defined(CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU) opts->thread_ops = &squashfs_decompressor_percpu; #endif + opts->thread_num = 0; fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* ping //Re: [PATCH v4 0/2] squashfs: Add the mount parameter "threads=" 2022-09-16 8:36 ` [PATCH v4 " Xiaoming Ni 2022-09-16 8:36 ` [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-09-16 8:36 ` [PATCH v4 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni @ 2022-09-27 1:05 ` Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 " Xiaoming Ni 3 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-27 1:05 UTC (permalink / raw) To: linux-kernel, phillip Cc: wangle6, yi.zhang, wangbing6, zhongjubin, chenjianguo3 ping On 2022/9/16 16:36, Xiaoming Ni wrote: > Currently, Squashfs supports multiple decompressor parallel modes. However, this > mode can be configured only during kernel building and does not support flexible > selection during runtime. > > In the current patch set, the mount parameter "threads=" is added to allow users > to select the parallel decompressor mode and configure the number of decompressors > when mounting a file system. > > "threads=<single|multi|percpu|1|2|3|...>" > The upper limit is num_online_cpus() * 2. > > > v4: Based on Philip Lougher's suggestion, make the following updates: > 1. Use static modifiers to avoid changing symbol names. > 2. Fixed some formatting issues > > v3: https://lore.kernel.org/lkml/20220902094855.22666-1-nixiaoming@huawei.com/ > Based on Philip Lougher's suggestion, make the following updates: > 1. The default configuration is the same as that before the patch installation. > 2. Compile the three decompression modes when the new configuration is enabled. > 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. > > v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ > fix warning: sparse: incorrect type in initializer (different address spaces) > Reported-by: kernel test robot <lkp@intel.com> > > v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ > > Xiaoming Ni (2): > squashfs: add the mount parameter theads=<single|multi|percpu> > squashfs: Allows users to configure the number of decompression > threads > > fs/squashfs/Kconfig | 51 +++++++++++++++-- > fs/squashfs/block.c | 2 +- > fs/squashfs/decompressor.c | 2 +- > fs/squashfs/decompressor_multi.c | 20 ++++--- > fs/squashfs/decompressor_multi_percpu.c | 23 +++++--- > fs/squashfs/decompressor_single.c | 15 +++-- > fs/squashfs/squashfs.h | 23 ++++++-- > fs/squashfs/squashfs_fs_sb.h | 4 +- > fs/squashfs/super.c | 97 ++++++++++++++++++++++++++++++++- > 9 files changed, 203 insertions(+), 34 deletions(-) > ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" 2022-09-16 8:36 ` [PATCH v4 " Xiaoming Ni ` (2 preceding siblings ...) 2022-09-27 1:05 ` ping //Re: [PATCH v4 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni @ 2022-09-30 9:14 ` Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni ` (3 more replies) 3 siblings, 4 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-30 9:14 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, zhongjubin, chenjianguo3 Currently, Squashfs supports multiple decompressor parallel modes. However, this mode can be configured only during kernel building and does not support flexible selection during runtime. In the current patch set, the mount parameter "threads=" is added to allow users to select the parallel decompressor mode and configure the number of decompressors when mounting a file system. "threads=<single|multi|percpu|1|2|3|...>" The upper limit is num_online_cpus() * 2. v5: fix a low-level mistake in patching: fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is not defined, evaluates to 0 [-Wundef] v4: https://lore.kernel.org/lkml/20220916083604.33408-1-nixiaoming@huawei.com/ Based on Philip Lougher's suggestion, make the following updates: 1. Use static modifiers to avoid changing symbol names. 2. Fixed some formatting issues v3: https://lore.kernel.org/lkml/20220902094855.22666-1-nixiaoming@huawei.com/ Based on Philip Lougher's suggestion, make the following updates: 1. The default configuration is the same as that before the patch installation. 2. Compile the three decompression modes when the new configuration is enabled. 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ fix warning: sparse: incorrect type in initializer (different address spaces) Reported-by: kernel test robot <lkp@intel.com> v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ Xiaoming Ni (2): squashfs: add the mount parameter theads=<single|multi|percpu> squashfs: Allows users to configure the number of decompression threads fs/squashfs/Kconfig | 51 ++++++++++++++++-- fs/squashfs/block.c | 2 +- fs/squashfs/decompressor.c | 2 +- fs/squashfs/decompressor_multi.c | 20 ++++--- fs/squashfs/decompressor_multi_percpu.c | 23 +++++--- fs/squashfs/decompressor_single.c | 15 ++++-- fs/squashfs/squashfs.h | 23 ++++++-- fs/squashfs/squashfs_fs_sb.h | 4 +- fs/squashfs/super.c | 93 +++++++++++++++++++++++++++++++-- 9 files changed, 199 insertions(+), 34 deletions(-) -- 2.12.3 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v5 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-30 9:14 ` [PATCH v5 " Xiaoming Ni @ 2022-09-30 9:14 ` Xiaoming Ni 2022-10-17 21:58 ` Re " Phillip Lougher 2022-09-30 9:14 ` [PATCH v5 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni ` (2 subsequent siblings) 3 siblings, 1 reply; 38+ messages in thread From: Xiaoming Ni @ 2022-09-30 9:14 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, zhongjubin, chenjianguo3 Squashfs supports three decompression concurrency modes: Single-thread mode: concurrent reads are blocked and the memory overhead is small. Multi-thread mode/percpu mode: reduces concurrent read blocking but increases memory overhead. The corresponding schema must be fixed at compile time. During mounting, the concurrent decompression mode cannot be adjusted based on file read blocking. The mount parameter theads=<single|multi|percpu> is added to select the concurrent decompression mode of a single SquashFS file system image. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 39 +++++++++++++++++++++---- fs/squashfs/block.c | 2 +- fs/squashfs/decompressor.c | 2 +- fs/squashfs/decompressor_multi.c | 16 +++++++---- fs/squashfs/decompressor_multi_percpu.c | 23 ++++++++++----- fs/squashfs/decompressor_single.c | 15 +++++++--- fs/squashfs/squashfs.h | 23 +++++++++++---- fs/squashfs/squashfs_fs_sb.h | 3 +- fs/squashfs/super.c | 50 +++++++++++++++++++++++++++++++-- 9 files changed, 141 insertions(+), 32 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 916e78fabcaa..218bacdd4298 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -54,9 +54,36 @@ config SQUASHFS_FILE_DIRECT endchoice +config SQUASHFS_DECOMP_SINGLE + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI_PERCPU + depends on SQUASHFS + def_bool n + +config SQUASHFS_CHOICE_DECOMP_BY_MOUNT + bool "Select the parallel decompression mode during mount" + depends on SQUASHFS + default n + select SQUASHFS_DECOMP_SINGLE + select SQUASHFS_DECOMP_MULTI + select SQUASHFS_DECOMP_MULTI_PERCPU + help + Compile all parallel decompression modes and specify the + decompression mode by setting "threads=" during mount. + threads=<single|multi|percpu> + + default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE + choice - prompt "Decompressor parallelisation options" + prompt "Select decompression parallel mode at compile time" depends on SQUASHFS + depends on !SQUASHFS_CHOICE_DECOMP_BY_MOUNT help Squashfs now supports three parallelisation options for decompression. Each one exhibits various trade-offs between @@ -64,15 +91,17 @@ choice If in doubt, select "Single threaded compression" -config SQUASHFS_DECOMP_SINGLE +config SQUASHFS_COMPILE_DECOMP_SINGLE bool "Single threaded compression" + select SQUASHFS_DECOMP_SINGLE help Traditionally Squashfs has used single-threaded decompression. Only one block (data or metadata) can be decompressed at any one time. This limits CPU and memory usage to a minimum. -config SQUASHFS_DECOMP_MULTI +config SQUASHFS_COMPILE_DECOMP_MULTI bool "Use multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -85,8 +114,9 @@ config SQUASHFS_DECOMP_MULTI decompressors per core. It dynamically allocates decompressors on a demand basis. -config SQUASHFS_DECOMP_MULTI_PERCPU +config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI_PERCPU help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -95,7 +125,6 @@ config SQUASHFS_DECOMP_MULTI_PERCPU This decompressor implementation uses a maximum of one decompressor per core. It uses percpu variables to ensure decompression is load-balanced across the cores. - endchoice config SQUASHFS_XATTR diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 833aca92301f..bed3bb8b27fa 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -216,7 +216,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, res = -EIO; goto out_free_bio; } - res = squashfs_decompress(msblk, bio, offset, length, output); + res = msblk->thread_ops->decompress(msblk, bio, offset, length, output); } else { res = copy_bio_to_actor(bio, output, offset, length); } diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c index d57bef91ab08..8893cb9b4198 100644 --- a/fs/squashfs/decompressor.c +++ b/fs/squashfs/decompressor.c @@ -134,7 +134,7 @@ void *squashfs_decompressor_setup(struct super_block *sb, unsigned short flags) if (IS_ERR(comp_opts)) return comp_opts; - stream = squashfs_decompressor_create(msblk, comp_opts); + stream = msblk->thread_ops->create(msblk, comp_opts); if (IS_ERR(stream)) kfree(comp_opts); diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index db9f12a3ea05..eb25432bd149 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -29,12 +29,11 @@ #define MAX_DECOMPRESSOR (num_online_cpus() * 2) -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return MAX_DECOMPRESSOR; } - struct squashfs_stream { void *comp_opts; struct list_head strm_list; @@ -59,7 +58,7 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, wake_up(&stream->wait); } -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -103,7 +102,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream *stream = msblk->stream; if (stream) { @@ -180,7 +179,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { @@ -195,3 +194,10 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, msblk->decompressor->name); return res; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c index b881b9283b7f..1dfadf76ed9a 100644 --- a/fs/squashfs/decompressor_multi_percpu.c +++ b/fs/squashfs/decompressor_multi_percpu.c @@ -25,7 +25,7 @@ struct squashfs_stream { local_lock_t lock; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -59,7 +59,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream __percpu *percpu = (struct squashfs_stream __percpu *) msblk->stream; @@ -75,19 +75,21 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { struct squashfs_stream *stream; + struct squashfs_stream __percpu *percpu = + (struct squashfs_stream __percpu *) msblk->stream; int res; - local_lock(&msblk->stream->lock); - stream = this_cpu_ptr(msblk->stream); + local_lock(&percpu->lock); + stream = this_cpu_ptr(percpu); res = msblk->decompressor->decompress(msblk, stream->stream, bio, offset, length, output); - local_unlock(&msblk->stream->lock); + local_unlock(&percpu->lock); if (res < 0) ERROR("%s decompression failed, data probably corrupt\n", @@ -96,7 +98,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return num_possible_cpus(); } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c index 4eb3d083d45e..6f161887710b 100644 --- a/fs/squashfs/decompressor_single.c +++ b/fs/squashfs/decompressor_single.c @@ -24,7 +24,7 @@ struct squashfs_stream { struct mutex mutex; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -49,7 +49,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream *stream = msblk->stream; @@ -59,7 +59,7 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { @@ -78,7 +78,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return 1; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h index 9783e01c8100..a6164fdf9435 100644 --- a/fs/squashfs/squashfs.h +++ b/fs/squashfs/squashfs.h @@ -38,11 +38,24 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); /* decompressor_xxx.c */ -extern void *squashfs_decompressor_create(struct squashfs_sb_info *, void *); -extern void squashfs_decompressor_destroy(struct squashfs_sb_info *); -extern int squashfs_decompress(struct squashfs_sb_info *, struct bio *, - int, int, struct squashfs_page_actor *); -extern int squashfs_max_decompressors(void); + +struct squashfs_decompressor_thread_ops { + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); + void (*destroy)(struct squashfs_sb_info *msblk); + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output); + int (*max_decompressors)(void); +}; + +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; +#endif /* export.c */ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index 1e90c2575f9b..f1e5dad8ae0a 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -53,7 +53,7 @@ struct squashfs_sb_info { __le64 *xattr_id_table; struct mutex meta_index_mutex; struct meta_index *meta_index; - struct squashfs_stream *stream; + void *stream; __le64 *inode_lookup_table; u64 inode_table; u64 directory_table; @@ -66,5 +66,6 @@ struct squashfs_sb_info { int xattr_ids; unsigned int ids; bool panic_on_errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 32565dafa7f3..6ba6fb90b391 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -47,10 +47,12 @@ enum Opt_errors { enum squashfs_param { Opt_errors, + Opt_threads, }; struct squashfs_mount_opts { enum Opt_errors errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; static const struct constant_table squashfs_param_errors[] = { @@ -61,9 +63,29 @@ static const struct constant_table squashfs_param_errors[] = { static const struct fs_parameter_spec squashfs_fs_parameters[] = { fsparam_enum("errors", Opt_errors, squashfs_param_errors), + fsparam_string("threads", Opt_threads), {} }; +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (strcmp(str, "single") == 0) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } + if (strcmp(str, "multi") == 0) { + opts->thread_ops = &squashfs_decompressor_multi; + return 0; + } + if (strcmp(str, "percpu") == 0) { + opts->thread_ops = &squashfs_decompressor_percpu; + return 0; + } +#endif + return -EINVAL; +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -78,6 +100,10 @@ static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *para case Opt_errors: opts->errors = result.uint_32; break; + case Opt_threads: + if (squashfs_parse_param_threads(param->string, opts) != 0) + return -EINVAL; + break; default: return -EINVAL; } @@ -167,6 +193,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_bdev); goto failed_mount; } + msblk->thread_ops = opts->thread_ops; /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -252,7 +279,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - squashfs_max_decompressors(), msblk->block_size); + msblk->thread_ops->max_decompressors(), msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -383,7 +410,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) squashfs_cache_delete(msblk->block_cache); squashfs_cache_delete(msblk->fragment_cache); squashfs_cache_delete(msblk->read_page); - squashfs_decompressor_destroy(msblk); + msblk->thread_ops->destroy(msblk); kfree(msblk->inode_lookup_table); kfree(msblk->fragment_index); kfree(msblk->id_table); @@ -435,6 +462,20 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_multi) { + seq_puts(s, ",threads=multi"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_percpu) { + seq_puts(s, ",threads=percpu"); + return 0; + } +#endif return 0; } @@ -446,6 +487,9 @@ static int squashfs_init_fs_context(struct fs_context *fc) if (!opts) return -ENOMEM; +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + opts->thread_ops = &squashfs_decompressor_single; +#endif fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; @@ -478,7 +522,7 @@ static void squashfs_put_super(struct super_block *sb) squashfs_cache_delete(sbi->block_cache); squashfs_cache_delete(sbi->fragment_cache); squashfs_cache_delete(sbi->read_page); - squashfs_decompressor_destroy(sbi); + sbi->thread_ops->destroy(sbi); kfree(sbi->id_table); kfree(sbi->fragment_index); kfree(sbi->meta_index); -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re [PATCH v5 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-09-30 9:14 ` [PATCH v5 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-10-17 21:58 ` Phillip Lougher 0 siblings, 0 replies; 38+ messages in thread From: Phillip Lougher @ 2022-10-17 21:58 UTC (permalink / raw) To: nixiaoming Cc: chenjianguo3, linux-kernel, phillip, wangle6, yi.zhang, zhongjubin On 30 Sep 2022 17:14:05 +0800 Xiaoming Ni wrote: >Squashfs supports three decompression concurrency modes: > Single-thread mode: concurrent reads are blocked and the memory > overhead is small. > Multi-thread mode/percpu mode: reduces concurrent read blocking but > increases memory overhead. > You have made another mistake in fixing this patch. Previously, I asked you to fix patch lines (appearing in V3) which caused a build error. These lines were: @@ -446,6 +487,13 @@ static int squashfs_init_fs_context(struct fs_context *fc) if (!opts) return -ENOMEM; +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + opts->thread_ops = &squashfs_decompressor_single; +#elif CONFIG_SQUASHFS_DECOMP_MULTI + opts->thread_ops = &squashfs_decompressor_multi; +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU + opts->thread_ops = &squashfs_decompressor_percpu; +#endif fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; I expected you to replace the +#elif CONFIG_SQUASHFS_DECOMP_MULTI line with +#elif defined(CONFIG_SQUASHFS_DECOMP_MULTI) and the +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU line with +#elif defined(CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU) If you had done that then the patch set would finally have been OK. Instead you did this in this patch ... > >@@ -446,6 +487,9 @@ static int squashfs_init_fs_context(struct fs_context *fc) > if (!opts) > return -ENOMEM; > >+#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT >+ opts->thread_ops = &squashfs_decompressor_single; >+#endif > fc->fs_private = opts; > fc->ops = &squashfs_context_ops; > return 0; You have removed the build error by removing the offending lines. But, you have rather obviously left opt->thread_ops unassigned if CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set. Given the following configuration CONFIG_SQUASHFS=y # CONFIG_SQUASHFS_FILE_CACHE is not set CONFIG_SQUASHFS_FILE_DIRECT=y CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU=y # CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set # CONFIG_SQUASHFS_COMPILE_DECOMP_SINGLE is not set # CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI is not set CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU=y CONFIG_SQUASHFS_XATTR=y CONFIG_SQUASHFS_ZLIB=y CONFIG_SQUASHFS_LZ4=y CONFIG_SQUASHFS_LZO=y CONFIG_SQUASHFS_XZ=y CONFIG_SQUASHFS_ZSTD=y # CONFIG_SQUASHFS_4K_DEVBLK_SIZE is not set # CONFIG_SQUASHFS_EMBEDDED is not set CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE=3 You will get the following kernel oops with this patch applied. root@logopolis:~# mount -t squashfs /dev/sdb /mnt BUG: kernel NULL pointer dereference, address: 0000000000000018 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 8000000004db0067 P4D 8000000004db0067 PUD 5bf0067 PMD 0 Oops: 0000 [#1] PREEMPT SMP PTI CPU: 3 PID: 1482 Comm: mount Not tainted 6.0.0+ #38 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014 RIP: 0010:squashfs_fill_super+0x2d5/0x700 Code: e0 c9 02 82 e8 2c d0 ff ff 48 89 43 10 48 89 c7 48 85 c0 0f 84 10 03 00 00 8b 93 98 00 00 00 48 8b 83 c0 00 00 00 89 54 24 04 <48> 8b 40 18 e8 12 51 b8 00 8b 54 24 04 48 c7 c7 86 fb 21 82 89 c6 RSP: 0018:ffffc900024e7de0 EFLAGS: 00010286 RAX: 0000000000000000 RBX: ffff88800611b300 RCX: 00000000000003db RDX: 0000000000100000 RSI: 0000000000000cc0 RDI: ffff888004411600 RBP: ffff888004689000 R08: ffff888004411900 R09: 0000000000000000 R10: ffff88800440f450 R11: 0000000000024298 R12: ffff888004ea9540 R13: 000002c12876123d R14: ffff8880044115a0 R15: 00000000000000c0 FS: 00007f99343ea840(0000) GS:ffff88807dd80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000018 CR3: 00000000046ea000 CR4: 00000000000006e0 Call Trace: <TASK> ? squashfs_init_fs_context+0x40/0x40 get_tree_bdev+0x16a/0x260 vfs_get_tree+0x20/0xb0 path_mount+0x2d3/0xa70 __x64_sys_mount+0x194/0x1f0 do_syscall_64+0x43/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7f9933621d8a Code: 48 8b 0d 01 c1 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ce c0 2b 00 f7 d8 64 89 01 48 RSP: 002b:00007ffd3d78d138 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 00007ffd3d78d260 RCX: 00007f9933621d8a RDX: 00000000006191e0 RSI: 00000000006191c0 RDI: 00000000006191a0 RBP: 00000000c0ed0000 R08: 0000000000000000 R09: 0000000000619300 R10: ffffffffc0ed0000 R11: 0000000000000202 R12: 00000000006191c0 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 </TASK> Modules linked in: CR2: 0000000000000018 ---[ end trace 0000000000000000 ]--- RIP: 0010:squashfs_fill_super+0x2d5/0x700 Code: e0 c9 02 82 e8 2c d0 ff ff 48 89 43 10 48 89 c7 48 85 c0 0f 84 10 03 00 00 8b 93 98 00 00 00 48 8b 83 c0 00 00 00 89 54 24 04 <48> 8b 40 18 e8 12 51 b8 00 8b 54 24 04 48 c7 c7 86 fb 21 82 89 c6 RSP: 0018:ffffc900024e7de0 EFLAGS: 00010286 RAX: 0000000000000000 RBX: ffff88800611b300 RCX: 00000000000003db RDX: 0000000000100000 RSI: 0000000000000cc0 RDI: ffff888004411600 RBP: ffff888004689000 R08: ffff888004411900 R09: 0000000000000000 R10: ffff88800440f450 R11: 0000000000024298 R12: ffff888004ea9540 R13: 000002c12876123d R14: ffff8880044115a0 R15: 00000000000000c0 FS: 00007f99343ea840(0000) GS:ffff88807dd80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000018 CR3: 00000000046ea000 CR4: 00000000000006e0 Killed I do not understand why you have not fixed the patch as I have asked, and instead repeatedly sent patches which are obviously broken. Cheers Phillip ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v5 2/2] squashfs: Allows users to configure the number of decompression threads 2022-09-30 9:14 ` [PATCH v5 " Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-09-30 9:14 ` Xiaoming Ni 2022-10-17 0:57 ` ping// Re: [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 " Xiaoming Ni 3 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-09-30 9:14 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, zhongjubin, chenjianguo3 The maximum number of threads in the decompressor_multi.c file is fixed and cannot be adjusted according to user needs. Therefore, the mount parameter needs to be added to allow users to configure the number of threads as required. The upper limit is num_online_cpus() * 2. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 16 ++++++++-- fs/squashfs/decompressor_multi.c | 4 +-- fs/squashfs/squashfs_fs_sb.h | 1 + fs/squashfs/super.c | 63 +++++++++++++++++++++++++++++++++------- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 218bacdd4298..60fc98bdf421 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -73,11 +73,10 @@ config SQUASHFS_CHOICE_DECOMP_BY_MOUNT select SQUASHFS_DECOMP_SINGLE select SQUASHFS_DECOMP_MULTI select SQUASHFS_DECOMP_MULTI_PERCPU + select SQUASHFS_MOUNT_DECOMP_THREADS help Compile all parallel decompression modes and specify the decompression mode by setting "threads=" during mount. - threads=<single|multi|percpu> - default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE choice @@ -127,6 +126,19 @@ config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU decompression is load-balanced across the cores. endchoice +config SQUASHFS_MOUNT_DECOMP_THREADS + bool "Add the mount parameter 'threads=' for squashfs" + depends on SQUASHFS + depends on SQUASHFS_DECOMP_MULTI + default n + help + Use threads= to set the decompression parallel mode and the number of threads. + If SQUASHFS_CHOICE_DECOMP_BY_MOUNT=y + threads=<single|multi|percpu|1|2|3|...> + else + threads=<2|3|...> + The upper limit is num_online_cpus() * 2. + config SQUASHFS_XATTR bool "Squashfs XATTR support" depends on SQUASHFS diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index eb25432bd149..416c53eedbd1 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -144,7 +144,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, * If there is no available decomp and already full, * let's wait for releasing decomp from other users. */ - if (stream->avail_decomp >= MAX_DECOMPRESSOR) + if (stream->avail_decomp >= msblk->max_thread_num) goto wait; /* Let's allocate new decomp */ @@ -160,7 +160,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } stream->avail_decomp++; - WARN_ON(stream->avail_decomp > MAX_DECOMPRESSOR); + WARN_ON(stream->avail_decomp > msblk->max_thread_num); mutex_unlock(&stream->mutex); break; diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index f1e5dad8ae0a..659082e9e51d 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -67,5 +67,6 @@ struct squashfs_sb_info { unsigned int ids; bool panic_on_errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int max_thread_num; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 6ba6fb90b391..920449e1675f 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -53,6 +53,7 @@ enum squashfs_param { struct squashfs_mount_opts { enum Opt_errors errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int thread_num; }; static const struct constant_table squashfs_param_errors[] = { @@ -67,7 +68,8 @@ static const struct fs_parameter_spec squashfs_fs_parameters[] = { {} }; -static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) + +static int squashfs_parse_param_threads_str(const char *str, struct squashfs_mount_opts *opts) { #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT if (strcmp(str, "single") == 0) { @@ -86,6 +88,42 @@ static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_o return -EINVAL; } +static int squashfs_parse_param_threads_num(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + int ret; + unsigned long num; + + ret = kstrtoul(str, 0, &num); + if (ret != 0) + return -EINVAL; + if (num > 1) { + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + return -EINVAL; + opts->thread_num = (int)num; + return 0; + } +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (num == 1) { + opts->thread_ops = &squashfs_decompressor_single; + opts->thread_num = 1; + return 0; + } +#endif +#endif /* !CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS */ + return -EINVAL; +} + +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ + int ret = squashfs_parse_param_threads_str(str, opts); + + if (ret == 0) + return ret; + return squashfs_parse_param_threads_num(str, opts); +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -194,6 +232,11 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) goto failed_mount; } msblk->thread_ops = opts->thread_ops; + if (opts->thread_num == 0) { + msblk->max_thread_num = msblk->thread_ops->max_decompressors(); + } else { + msblk->max_thread_num = opts->thread_num; + } /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -279,7 +322,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - msblk->thread_ops->max_decompressors(), msblk->block_size); + msblk->max_thread_num, msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -463,18 +506,17 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) seq_puts(s, ",errors=continue"); #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT - if (msblk->thread_ops == &squashfs_decompressor_single) { - seq_puts(s, ",threads=single"); - return 0; - } - if (msblk->thread_ops == &squashfs_decompressor_multi) { - seq_puts(s, ",threads=multi"); - return 0; - } if (msblk->thread_ops == &squashfs_decompressor_percpu) { seq_puts(s, ",threads=percpu"); return 0; } + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } +#endif +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + seq_printf(s, ",threads=%d", msblk->max_thread_num); #endif return 0; } @@ -490,6 +532,7 @@ static int squashfs_init_fs_context(struct fs_context *fc) #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT opts->thread_ops = &squashfs_decompressor_single; #endif + opts->thread_num = 0; fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.12.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* ping// Re: [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" 2022-09-30 9:14 ` [PATCH v5 " Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni @ 2022-10-17 0:57 ` Xiaoming Ni 2022-10-17 22:59 ` Phillip Lougher 2022-10-19 3:09 ` [PATCH v6 " Xiaoming Ni 3 siblings, 1 reply; 38+ messages in thread From: Xiaoming Ni @ 2022-10-17 0:57 UTC (permalink / raw) To: linux-kernel, phillip; +Cc: wangle6, yi.zhang, zhongjubin, chenjianguo3 ping On 2022/9/30 17:14, Xiaoming Ni wrote: > Currently, Squashfs supports multiple decompressor parallel modes. However, this > mode can be configured only during kernel building and does not support flexible > selection during runtime. > > In the current patch set, the mount parameter "threads=" is added to allow users > to select the parallel decompressor mode and configure the number of decompressors > when mounting a file system. > > "threads=<single|multi|percpu|1|2|3|...>" > The upper limit is num_online_cpus() * 2. > > v5: fix a low-level mistake in patching: > fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is > not defined, evaluates to 0 [-Wundef] > > v4: https://lore.kernel.org/lkml/20220916083604.33408-1-nixiaoming@huawei.com/ > Based on Philip Lougher's suggestion, make the following updates: > 1. Use static modifiers to avoid changing symbol names. > 2. Fixed some formatting issues > > v3: https://lore.kernel.org/lkml/20220902094855.22666-1-nixiaoming@huawei.com/ > Based on Philip Lougher's suggestion, make the following updates: > 1. The default configuration is the same as that before the patch installation. > 2. Compile the three decompression modes when the new configuration is enabled. > 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. > > v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ > fix warning: sparse: incorrect type in initializer (different address spaces) > Reported-by: kernel test robot <lkp@intel.com> > > v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ > > Xiaoming Ni (2): > squashfs: add the mount parameter theads=<single|multi|percpu> > squashfs: Allows users to configure the number of decompression > threads > > fs/squashfs/Kconfig | 51 ++++++++++++++++-- > fs/squashfs/block.c | 2 +- > fs/squashfs/decompressor.c | 2 +- > fs/squashfs/decompressor_multi.c | 20 ++++--- > fs/squashfs/decompressor_multi_percpu.c | 23 +++++--- > fs/squashfs/decompressor_single.c | 15 ++++-- > fs/squashfs/squashfs.h | 23 ++++++-- > fs/squashfs/squashfs_fs_sb.h | 4 +- > fs/squashfs/super.c | 93 +++++++++++++++++++++++++++++++-- > 9 files changed, 199 insertions(+), 34 deletions(-) > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: ping// Re: [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" 2022-10-17 0:57 ` ping// Re: [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni @ 2022-10-17 22:59 ` Phillip Lougher 2022-10-18 6:24 ` Xiaoming Ni 0 siblings, 1 reply; 38+ messages in thread From: Phillip Lougher @ 2022-10-17 22:59 UTC (permalink / raw) To: Xiaoming Ni Cc: linux-kernel, phillip, wangle6, yi.zhang, zhongjubin, chenjianguo3 On Mon, Oct 17, 2022 at 2:11 AM Xiaoming Ni <nixiaoming@huawei.com> wrote: > > ping I was hoping you'd notice the obvious mistake you made in the patch set, and send an updated version, which would avoid me having to point out such mistakes again. I have replied to patch [1/2] Phillip > > On 2022/9/30 17:14, Xiaoming Ni wrote: > > Currently, Squashfs supports multiple decompressor parallel modes. However, this > > mode can be configured only during kernel building and does not support flexible > > selection during runtime. > > > > In the current patch set, the mount parameter "threads=" is added to allow users > > to select the parallel decompressor mode and configure the number of decompressors > > when mounting a file system. > > > > "threads=<single|multi|percpu|1|2|3|...>" > > The upper limit is num_online_cpus() * 2. > > > > v5: fix a low-level mistake in patching: > > fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is > > not defined, evaluates to 0 [-Wundef] > > > > v4: https://lore.kernel.org/lkml/20220916083604.33408-1-nixiaoming@huawei.com/ > > Based on Philip Lougher's suggestion, make the following updates: > > 1. Use static modifiers to avoid changing symbol names. > > 2. Fixed some formatting issues > > > > v3: https://lore.kernel.org/lkml/20220902094855.22666-1-nixiaoming@huawei.com/ > > Based on Philip Lougher's suggestion, make the following updates: > > 1. The default configuration is the same as that before the patch installation. > > 2. Compile the three decompression modes when the new configuration is enabled. > > 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. > > > > v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ > > fix warning: sparse: incorrect type in initializer (different address spaces) > > Reported-by: kernel test robot <lkp@intel.com> > > > > v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ > > > > Xiaoming Ni (2): > > squashfs: add the mount parameter theads=<single|multi|percpu> > > squashfs: Allows users to configure the number of decompression > > threads > > > > fs/squashfs/Kconfig | 51 ++++++++++++++++-- > > fs/squashfs/block.c | 2 +- > > fs/squashfs/decompressor.c | 2 +- > > fs/squashfs/decompressor_multi.c | 20 ++++--- > > fs/squashfs/decompressor_multi_percpu.c | 23 +++++--- > > fs/squashfs/decompressor_single.c | 15 ++++-- > > fs/squashfs/squashfs.h | 23 ++++++-- > > fs/squashfs/squashfs_fs_sb.h | 4 +- > > fs/squashfs/super.c | 93 +++++++++++++++++++++++++++++++-- > > 9 files changed, 199 insertions(+), 34 deletions(-) > > > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: ping// Re: [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" 2022-10-17 22:59 ` Phillip Lougher @ 2022-10-18 6:24 ` Xiaoming Ni 0 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-10-18 6:24 UTC (permalink / raw) To: Phillip Lougher Cc: linux-kernel, phillip, wangle6, yi.zhang, zhongjubin, chenjianguo3 On 2022/10/18 6:59, Phillip Lougher wrote: > On Mon, Oct 17, 2022 at 2:11 AM Xiaoming Ni <nixiaoming@huawei.com> wrote: >> >> ping > > I was hoping you'd notice the obvious mistake you made in the patch set, > and send an updated version, which would avoid me having to point out > such mistakes again. > > I have replied to patch [1/2] > > Phillip > I'm so sorry. I'm very ashamed of my serial mistakes. I'll fix it again later and resend it, I'm sorry I need to take up your valuable time to help review it again. Thanks Xiaoming Ni ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 0/2] squashfs: Add the mount parameter "threads=" 2022-09-30 9:14 ` [PATCH v5 " Xiaoming Ni ` (2 preceding siblings ...) 2022-10-17 0:57 ` ping// Re: [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni @ 2022-10-19 3:09 ` Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni ` (2 more replies) 3 siblings, 3 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-10-19 3:09 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, zhongjubin, chenjianguo3 Currently, Squashfs supports multiple decompressor parallel modes. However, this mode can be configured only during kernel building and does not support flexible selection during runtime. In the current patch set, the mount parameter "threads=" is added to allow users to select the parallel decompressor mode and configure the number of decompressors when mounting a file system. "threads=<single|multi|percpu|1|2|3|...>" The upper limit is num_online_cpus() * 2. v6: fix opt->thread_ops unassigned if CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set. v5: https://lore.kernel.org/lkml/20220930091406.50869-1-nixiaoming@huawei.com/ fix a low-level mistake in patching: fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is not defined, evaluates to 0 [-Wundef] v4: https://lore.kernel.org/lkml/20220916083604.33408-1-nixiaoming@huawei.com/ Based on Philip Lougher's suggestion, make the following updates: 1. Use static modifiers to avoid changing symbol names. 2. Fixed some formatting issues v3: https://lore.kernel.org/lkml/20220902094855.22666-1-nixiaoming@huawei.com/ Based on Philip Lougher's suggestion, make the following updates: 1. The default configuration is the same as that before the patch installation. 2. Compile the three decompression modes when the new configuration is enabled. 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ fix warning: sparse: incorrect type in initializer (different address spaces) Reported-by: kernel test robot <lkp@intel.com> v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ *** BLURB HERE *** Xiaoming Ni (2): squashfs: add the mount parameter theads=<single|multi|percpu> squashfs: Allows users to configure the number of decompression threads fs/squashfs/Kconfig | 51 +++++++++++-- fs/squashfs/block.c | 2 +- fs/squashfs/decompressor.c | 2 +- fs/squashfs/decompressor_multi.c | 20 +++-- fs/squashfs/decompressor_multi_percpu.c | 23 ++++-- fs/squashfs/decompressor_single.c | 15 +++- fs/squashfs/squashfs.h | 23 ++++-- fs/squashfs/squashfs_fs_sb.h | 4 +- fs/squashfs/super.c | 97 ++++++++++++++++++++++++- 9 files changed, 203 insertions(+), 34 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> 2022-10-19 3:09 ` [PATCH v6 " Xiaoming Ni @ 2022-10-19 3:09 ` Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-10-27 22:44 ` [PATCH v6 0/2] squashfs: Add the mount parameter "threads=" Phillip Lougher 2 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-10-19 3:09 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, zhongjubin, chenjianguo3 Squashfs supports three decompression concurrency modes: Single-thread mode: concurrent reads are blocked and the memory overhead is small. Multi-thread mode/percpu mode: reduces concurrent read blocking but increases memory overhead. The corresponding schema must be fixed at compile time. During mounting, the concurrent decompression mode cannot be adjusted based on file read blocking. The mount parameter theads=<single|multi|percpu> is added to select the concurrent decompression mode of a single SquashFS file system image. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 39 ++++++++++++++--- fs/squashfs/block.c | 2 +- fs/squashfs/decompressor.c | 2 +- fs/squashfs/decompressor_multi.c | 16 ++++--- fs/squashfs/decompressor_multi_percpu.c | 23 ++++++---- fs/squashfs/decompressor_single.c | 15 +++++-- fs/squashfs/squashfs.h | 23 +++++++--- fs/squashfs/squashfs_fs_sb.h | 3 +- fs/squashfs/super.c | 56 +++++++++++++++++++++++-- 9 files changed, 147 insertions(+), 32 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 916e78fabcaa..218bacdd4298 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -54,9 +54,36 @@ config SQUASHFS_FILE_DIRECT endchoice +config SQUASHFS_DECOMP_SINGLE + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI + depends on SQUASHFS + def_bool n + +config SQUASHFS_DECOMP_MULTI_PERCPU + depends on SQUASHFS + def_bool n + +config SQUASHFS_CHOICE_DECOMP_BY_MOUNT + bool "Select the parallel decompression mode during mount" + depends on SQUASHFS + default n + select SQUASHFS_DECOMP_SINGLE + select SQUASHFS_DECOMP_MULTI + select SQUASHFS_DECOMP_MULTI_PERCPU + help + Compile all parallel decompression modes and specify the + decompression mode by setting "threads=" during mount. + threads=<single|multi|percpu> + + default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE + choice - prompt "Decompressor parallelisation options" + prompt "Select decompression parallel mode at compile time" depends on SQUASHFS + depends on !SQUASHFS_CHOICE_DECOMP_BY_MOUNT help Squashfs now supports three parallelisation options for decompression. Each one exhibits various trade-offs between @@ -64,15 +91,17 @@ choice If in doubt, select "Single threaded compression" -config SQUASHFS_DECOMP_SINGLE +config SQUASHFS_COMPILE_DECOMP_SINGLE bool "Single threaded compression" + select SQUASHFS_DECOMP_SINGLE help Traditionally Squashfs has used single-threaded decompression. Only one block (data or metadata) can be decompressed at any one time. This limits CPU and memory usage to a minimum. -config SQUASHFS_DECOMP_MULTI +config SQUASHFS_COMPILE_DECOMP_MULTI bool "Use multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -85,8 +114,9 @@ config SQUASHFS_DECOMP_MULTI decompressors per core. It dynamically allocates decompressors on a demand basis. -config SQUASHFS_DECOMP_MULTI_PERCPU +config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU bool "Use percpu multiple decompressors for parallel I/O" + select SQUASHFS_DECOMP_MULTI_PERCPU help By default Squashfs uses a single decompressor but it gives poor performance on parallel I/O workloads when using multiple CPU @@ -95,7 +125,6 @@ config SQUASHFS_DECOMP_MULTI_PERCPU This decompressor implementation uses a maximum of one decompressor per core. It uses percpu variables to ensure decompression is load-balanced across the cores. - endchoice config SQUASHFS_XATTR diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 833aca92301f..bed3bb8b27fa 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -216,7 +216,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, res = -EIO; goto out_free_bio; } - res = squashfs_decompress(msblk, bio, offset, length, output); + res = msblk->thread_ops->decompress(msblk, bio, offset, length, output); } else { res = copy_bio_to_actor(bio, output, offset, length); } diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c index d57bef91ab08..8893cb9b4198 100644 --- a/fs/squashfs/decompressor.c +++ b/fs/squashfs/decompressor.c @@ -134,7 +134,7 @@ void *squashfs_decompressor_setup(struct super_block *sb, unsigned short flags) if (IS_ERR(comp_opts)) return comp_opts; - stream = squashfs_decompressor_create(msblk, comp_opts); + stream = msblk->thread_ops->create(msblk, comp_opts); if (IS_ERR(stream)) kfree(comp_opts); diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index db9f12a3ea05..eb25432bd149 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -29,12 +29,11 @@ #define MAX_DECOMPRESSOR (num_online_cpus() * 2) -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return MAX_DECOMPRESSOR; } - struct squashfs_stream { void *comp_opts; struct list_head strm_list; @@ -59,7 +58,7 @@ static void put_decomp_stream(struct decomp_stream *decomp_strm, wake_up(&stream->wait); } -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -103,7 +102,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream *stream = msblk->stream; if (stream) { @@ -180,7 +179,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { @@ -195,3 +194,10 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, msblk->decompressor->name); return res; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/decompressor_multi_percpu.c b/fs/squashfs/decompressor_multi_percpu.c index b881b9283b7f..1dfadf76ed9a 100644 --- a/fs/squashfs/decompressor_multi_percpu.c +++ b/fs/squashfs/decompressor_multi_percpu.c @@ -25,7 +25,7 @@ struct squashfs_stream { local_lock_t lock; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -59,7 +59,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream __percpu *percpu = (struct squashfs_stream __percpu *) msblk->stream; @@ -75,19 +75,21 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { struct squashfs_stream *stream; + struct squashfs_stream __percpu *percpu = + (struct squashfs_stream __percpu *) msblk->stream; int res; - local_lock(&msblk->stream->lock); - stream = this_cpu_ptr(msblk->stream); + local_lock(&percpu->lock); + stream = this_cpu_ptr(percpu); res = msblk->decompressor->decompress(msblk, stream->stream, bio, offset, length, output); - local_unlock(&msblk->stream->lock); + local_unlock(&percpu->lock); if (res < 0) ERROR("%s decompression failed, data probably corrupt\n", @@ -96,7 +98,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return num_possible_cpus(); } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/decompressor_single.c b/fs/squashfs/decompressor_single.c index 4eb3d083d45e..6f161887710b 100644 --- a/fs/squashfs/decompressor_single.c +++ b/fs/squashfs/decompressor_single.c @@ -24,7 +24,7 @@ struct squashfs_stream { struct mutex mutex; }; -void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, +static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, void *comp_opts) { struct squashfs_stream *stream; @@ -49,7 +49,7 @@ void *squashfs_decompressor_create(struct squashfs_sb_info *msblk, return ERR_PTR(err); } -void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) +static void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) { struct squashfs_stream *stream = msblk->stream; @@ -59,7 +59,7 @@ void squashfs_decompressor_destroy(struct squashfs_sb_info *msblk) } } -int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, +static int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, int offset, int length, struct squashfs_page_actor *output) { @@ -78,7 +78,14 @@ int squashfs_decompress(struct squashfs_sb_info *msblk, struct bio *bio, return res; } -int squashfs_max_decompressors(void) +static int squashfs_max_decompressors(void) { return 1; } + +const struct squashfs_decompressor_thread_ops squashfs_decompressor_single = { + .create = squashfs_decompressor_create, + .destroy = squashfs_decompressor_destroy, + .decompress = squashfs_decompress, + .max_decompressors = squashfs_max_decompressors, +}; diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h index 9783e01c8100..a6164fdf9435 100644 --- a/fs/squashfs/squashfs.h +++ b/fs/squashfs/squashfs.h @@ -38,11 +38,24 @@ extern const struct squashfs_decompressor *squashfs_lookup_decompressor(int); extern void *squashfs_decompressor_setup(struct super_block *, unsigned short); /* decompressor_xxx.c */ -extern void *squashfs_decompressor_create(struct squashfs_sb_info *, void *); -extern void squashfs_decompressor_destroy(struct squashfs_sb_info *); -extern int squashfs_decompress(struct squashfs_sb_info *, struct bio *, - int, int, struct squashfs_page_actor *); -extern int squashfs_max_decompressors(void); + +struct squashfs_decompressor_thread_ops { + void * (*create)(struct squashfs_sb_info *msblk, void *comp_opts); + void (*destroy)(struct squashfs_sb_info *msblk); + int (*decompress)(struct squashfs_sb_info *msblk, struct bio *bio, + int offset, int length, struct squashfs_page_actor *output); + int (*max_decompressors)(void); +}; + +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_single; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_multi; +#endif +#ifdef CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU +extern const struct squashfs_decompressor_thread_ops squashfs_decompressor_percpu; +#endif /* export.c */ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64, diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index 1e90c2575f9b..f1e5dad8ae0a 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -53,7 +53,7 @@ struct squashfs_sb_info { __le64 *xattr_id_table; struct mutex meta_index_mutex; struct meta_index *meta_index; - struct squashfs_stream *stream; + void *stream; __le64 *inode_lookup_table; u64 inode_table; u64 directory_table; @@ -66,5 +66,6 @@ struct squashfs_sb_info { int xattr_ids; unsigned int ids; bool panic_on_errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index 32565dafa7f3..aac3ea72a9ba 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -47,10 +47,12 @@ enum Opt_errors { enum squashfs_param { Opt_errors, + Opt_threads, }; struct squashfs_mount_opts { enum Opt_errors errors; + const struct squashfs_decompressor_thread_ops *thread_ops; }; static const struct constant_table squashfs_param_errors[] = { @@ -61,9 +63,29 @@ static const struct constant_table squashfs_param_errors[] = { static const struct fs_parameter_spec squashfs_fs_parameters[] = { fsparam_enum("errors", Opt_errors, squashfs_param_errors), + fsparam_string("threads", Opt_threads), {} }; +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (strcmp(str, "single") == 0) { + opts->thread_ops = &squashfs_decompressor_single; + return 0; + } + if (strcmp(str, "multi") == 0) { + opts->thread_ops = &squashfs_decompressor_multi; + return 0; + } + if (strcmp(str, "percpu") == 0) { + opts->thread_ops = &squashfs_decompressor_percpu; + return 0; + } +#endif + return -EINVAL; +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -78,6 +100,10 @@ static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *para case Opt_errors: opts->errors = result.uint_32; break; + case Opt_threads: + if (squashfs_parse_param_threads(param->string, opts) != 0) + return -EINVAL; + break; default: return -EINVAL; } @@ -167,6 +193,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_bdev); goto failed_mount; } + msblk->thread_ops = opts->thread_ops; /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -252,7 +279,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - squashfs_max_decompressors(), msblk->block_size); + msblk->thread_ops->max_decompressors(), msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -383,7 +410,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) squashfs_cache_delete(msblk->block_cache); squashfs_cache_delete(msblk->fragment_cache); squashfs_cache_delete(msblk->read_page); - squashfs_decompressor_destroy(msblk); + msblk->thread_ops->destroy(msblk); kfree(msblk->inode_lookup_table); kfree(msblk->fragment_index); kfree(msblk->id_table); @@ -435,6 +462,20 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) else seq_puts(s, ",errors=continue"); +#ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT + if (msblk->thread_ops == &squashfs_decompressor_single) { + seq_puts(s, ",threads=single"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_multi) { + seq_puts(s, ",threads=multi"); + return 0; + } + if (msblk->thread_ops == &squashfs_decompressor_percpu) { + seq_puts(s, ",threads=percpu"); + return 0; + } +#endif return 0; } @@ -446,6 +487,15 @@ static int squashfs_init_fs_context(struct fs_context *fc) if (!opts) return -ENOMEM; +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + opts->thread_ops = &squashfs_decompressor_single; +#elif defined(CONFIG_SQUASHFS_DECOMP_MULTI) + opts->thread_ops = &squashfs_decompressor_multi; +#elif defined(CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU) + opts->thread_ops = &squashfs_decompressor_percpu; +#else +#error "fail: unknown squashfs decompression thread mode?" +#endif fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; @@ -478,7 +528,7 @@ static void squashfs_put_super(struct super_block *sb) squashfs_cache_delete(sbi->block_cache); squashfs_cache_delete(sbi->fragment_cache); squashfs_cache_delete(sbi->read_page); - squashfs_decompressor_destroy(sbi); + sbi->thread_ops->destroy(sbi); kfree(sbi->id_table); kfree(sbi->fragment_index); kfree(sbi->meta_index); -- 2.27.0 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v6 2/2] squashfs: Allows users to configure the number of decompression threads 2022-10-19 3:09 ` [PATCH v6 " Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni @ 2022-10-19 3:09 ` Xiaoming Ni 2022-10-27 22:44 ` [PATCH v6 0/2] squashfs: Add the mount parameter "threads=" Phillip Lougher 2 siblings, 0 replies; 38+ messages in thread From: Xiaoming Ni @ 2022-10-19 3:09 UTC (permalink / raw) To: linux-kernel, phillip Cc: nixiaoming, wangle6, yi.zhang, zhongjubin, chenjianguo3 The maximum number of threads in the decompressor_multi.c file is fixed and cannot be adjusted according to user needs. Therefore, the mount parameter needs to be added to allow users to configure the number of threads as required. The upper limit is num_online_cpus() * 2. Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- fs/squashfs/Kconfig | 16 ++++++++-- fs/squashfs/decompressor_multi.c | 4 +-- fs/squashfs/squashfs_fs_sb.h | 1 + fs/squashfs/super.c | 55 ++++++++++++++++++++++++++++---- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig index 218bacdd4298..60fc98bdf421 100644 --- a/fs/squashfs/Kconfig +++ b/fs/squashfs/Kconfig @@ -73,11 +73,10 @@ config SQUASHFS_CHOICE_DECOMP_BY_MOUNT select SQUASHFS_DECOMP_SINGLE select SQUASHFS_DECOMP_MULTI select SQUASHFS_DECOMP_MULTI_PERCPU + select SQUASHFS_MOUNT_DECOMP_THREADS help Compile all parallel decompression modes and specify the decompression mode by setting "threads=" during mount. - threads=<single|multi|percpu> - default Decompressor parallelisation is SQUASHFS_DECOMP_SINGLE choice @@ -127,6 +126,19 @@ config SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU decompression is load-balanced across the cores. endchoice +config SQUASHFS_MOUNT_DECOMP_THREADS + bool "Add the mount parameter 'threads=' for squashfs" + depends on SQUASHFS + depends on SQUASHFS_DECOMP_MULTI + default n + help + Use threads= to set the decompression parallel mode and the number of threads. + If SQUASHFS_CHOICE_DECOMP_BY_MOUNT=y + threads=<single|multi|percpu|1|2|3|...> + else + threads=<2|3|...> + The upper limit is num_online_cpus() * 2. + config SQUASHFS_XATTR bool "Squashfs XATTR support" depends on SQUASHFS diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c index eb25432bd149..416c53eedbd1 100644 --- a/fs/squashfs/decompressor_multi.c +++ b/fs/squashfs/decompressor_multi.c @@ -144,7 +144,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, * If there is no available decomp and already full, * let's wait for releasing decomp from other users. */ - if (stream->avail_decomp >= MAX_DECOMPRESSOR) + if (stream->avail_decomp >= msblk->max_thread_num) goto wait; /* Let's allocate new decomp */ @@ -160,7 +160,7 @@ static struct decomp_stream *get_decomp_stream(struct squashfs_sb_info *msblk, } stream->avail_decomp++; - WARN_ON(stream->avail_decomp > MAX_DECOMPRESSOR); + WARN_ON(stream->avail_decomp > msblk->max_thread_num); mutex_unlock(&stream->mutex); break; diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h index f1e5dad8ae0a..659082e9e51d 100644 --- a/fs/squashfs/squashfs_fs_sb.h +++ b/fs/squashfs/squashfs_fs_sb.h @@ -67,5 +67,6 @@ struct squashfs_sb_info { unsigned int ids; bool panic_on_errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int max_thread_num; }; #endif diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index aac3ea72a9ba..1e428ca9414e 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -53,6 +53,7 @@ enum squashfs_param { struct squashfs_mount_opts { enum Opt_errors errors; const struct squashfs_decompressor_thread_ops *thread_ops; + int thread_num; }; static const struct constant_table squashfs_param_errors[] = { @@ -67,7 +68,8 @@ static const struct fs_parameter_spec squashfs_fs_parameters[] = { {} }; -static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) + +static int squashfs_parse_param_threads_str(const char *str, struct squashfs_mount_opts *opts) { #ifdef CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT if (strcmp(str, "single") == 0) { @@ -86,6 +88,42 @@ static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_o return -EINVAL; } +static int squashfs_parse_param_threads_num(const char *str, struct squashfs_mount_opts *opts) +{ +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + int ret; + unsigned long num; + + ret = kstrtoul(str, 0, &num); + if (ret != 0) + return -EINVAL; + if (num > 1) { + opts->thread_ops = &squashfs_decompressor_multi; + if (num > opts->thread_ops->max_decompressors()) + return -EINVAL; + opts->thread_num = (int)num; + return 0; + } +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE + if (num == 1) { + opts->thread_ops = &squashfs_decompressor_single; + opts->thread_num = 1; + return 0; + } +#endif +#endif /* !CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS */ + return -EINVAL; +} + +static int squashfs_parse_param_threads(const char *str, struct squashfs_mount_opts *opts) +{ + int ret = squashfs_parse_param_threads_str(str, opts); + + if (ret == 0) + return ret; + return squashfs_parse_param_threads_num(str, opts); +} + static int squashfs_parse_param(struct fs_context *fc, struct fs_parameter *param) { struct squashfs_mount_opts *opts = fc->fs_private; @@ -194,6 +232,11 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) goto failed_mount; } msblk->thread_ops = opts->thread_ops; + if (opts->thread_num == 0) { + msblk->max_thread_num = msblk->thread_ops->max_decompressors(); + } else { + msblk->max_thread_num = opts->thread_num; + } /* Check the MAJOR & MINOR versions and lookup compression type */ msblk->decompressor = supported_squashfs_filesystem( @@ -279,7 +322,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc) /* Allocate read_page block */ msblk->read_page = squashfs_cache_init("data", - msblk->thread_ops->max_decompressors(), msblk->block_size); + msblk->max_thread_num, msblk->block_size); if (msblk->read_page == NULL) { errorf(fc, "Failed to allocate read_page block"); goto failed_mount; @@ -467,14 +510,13 @@ static int squashfs_show_options(struct seq_file *s, struct dentry *root) seq_puts(s, ",threads=single"); return 0; } - if (msblk->thread_ops == &squashfs_decompressor_multi) { - seq_puts(s, ",threads=multi"); - return 0; - } if (msblk->thread_ops == &squashfs_decompressor_percpu) { seq_puts(s, ",threads=percpu"); return 0; } +#endif +#ifdef CONFIG_SQUASHFS_MOUNT_DECOMP_THREADS + seq_printf(s, ",threads=%d", msblk->max_thread_num); #endif return 0; } @@ -496,6 +538,7 @@ static int squashfs_init_fs_context(struct fs_context *fc) #else #error "fail: unknown squashfs decompression thread mode?" #endif + opts->thread_num = 0; fc->fs_private = opts; fc->ops = &squashfs_context_ops; return 0; -- 2.27.0 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v6 0/2] squashfs: Add the mount parameter "threads=" 2022-10-19 3:09 ` [PATCH v6 " Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni @ 2022-10-27 22:44 ` Phillip Lougher 2 siblings, 0 replies; 38+ messages in thread From: Phillip Lougher @ 2022-10-27 22:44 UTC (permalink / raw) To: Xiaoming Ni, linux-kernel, Andrew Morton Cc: wangle6, yi.zhang, zhongjubin, chenjianguo3 On 19/10/2022 04:09, Xiaoming Ni wrote: > Currently, Squashfs supports multiple decompressor parallel modes. However, this > mode can be configured only during kernel building and does not support flexible > selection during runtime. > > In the current patch set, the mount parameter "threads=" is added to allow users > to select the parallel decompressor mode and configure the number of decompressors > when mounting a file system. > > "threads=<single|multi|percpu|1|2|3|...>" > The upper limit is num_online_cpus() * 2. > > v6: fix opt->thread_ops unassigned if CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set. > This version looks good to me. Thanks. Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk> > v5: https://lore.kernel.org/lkml/20220930091406.50869-1-nixiaoming@huawei.com/ > fix a low-level mistake in patching: > fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is > not defined, evaluates to 0 [-Wundef] > > v4: https://lore.kernel.org/lkml/20220916083604.33408-1-nixiaoming@huawei.com/ > Based on Philip Lougher's suggestion, make the following updates: > 1. Use static modifiers to avoid changing symbol names. > 2. Fixed some formatting issues > > v3: https://lore.kernel.org/lkml/20220902094855.22666-1-nixiaoming@huawei.com/ > Based on Philip Lougher's suggestion, make the following updates: > 1. The default configuration is the same as that before the patch installation. > 2. Compile the three decompression modes when the new configuration is enabled. > 3. "threads=1" supports only the SQUASHFS_DECOMP_SINGLE mode. > > v2: https://lore.kernel.org/lkml/20220816010052.15764-1-nixiaoming@huawei.com/ > fix warning: sparse: incorrect type in initializer (different address spaces) > Reported-by: kernel test robot <lkp@intel.com> > > v1: https://lore.kernel.org/lkml/20220815031100.75243-1-nixiaoming@huawei.com/ > *** BLURB HERE *** > > Xiaoming Ni (2): > squashfs: add the mount parameter theads=<single|multi|percpu> > squashfs: Allows users to configure the number of decompression > threads > > fs/squashfs/Kconfig | 51 +++++++++++-- > fs/squashfs/block.c | 2 +- > fs/squashfs/decompressor.c | 2 +- > fs/squashfs/decompressor_multi.c | 20 +++-- > fs/squashfs/decompressor_multi_percpu.c | 23 ++++-- > fs/squashfs/decompressor_single.c | 15 +++- > fs/squashfs/squashfs.h | 23 ++++-- > fs/squashfs/squashfs_fs_sb.h | 4 +- > fs/squashfs/super.c | 97 ++++++++++++++++++++++++- > 9 files changed, 203 insertions(+), 34 deletions(-) > ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2022-10-27 22:44 UTC | newest] Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-15 3:10 [PATCH 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-08-15 3:10 ` [PATCH 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-08-15 11:19 ` kernel test robot 2022-08-15 3:11 ` [PATCH 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-08-16 1:00 ` [PATCH v2 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-08-26 6:19 ` ping //Re: [PATCH v2 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-08-28 23:18 ` Phillip Lougher 2022-08-30 13:38 ` Xiaoming Ni 2022-08-30 18:08 ` Phillip Lougher 2022-08-30 18:34 ` Phillip Lougher 2022-08-31 1:09 ` Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 " Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-09-09 15:44 ` Phillip Lougher 2022-09-13 2:46 ` Xiaoming Ni 2022-09-09 15:50 ` Phillip Lougher 2022-09-13 2:47 ` Xiaoming Ni 2022-09-02 9:48 ` [PATCH v3 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-09-09 15:26 ` [PATCH v3 0/2] squashfs: Add the mount parameter "threads=" Phillip Lougher 2022-09-16 8:36 ` [PATCH v4 " Xiaoming Ni 2022-09-16 8:36 ` [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-09-28 2:20 ` Phillip Lougher 2022-09-28 3:06 ` Xiaoming Ni 2022-09-16 8:36 ` [PATCH v4 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-09-27 1:05 ` ping //Re: [PATCH v4 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 " Xiaoming Ni 2022-09-30 9:14 ` [PATCH v5 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-10-17 21:58 ` Re " Phillip Lougher 2022-09-30 9:14 ` [PATCH v5 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-10-17 0:57 ` ping// Re: [PATCH v5 0/2] squashfs: Add the mount parameter "threads=" Xiaoming Ni 2022-10-17 22:59 ` Phillip Lougher 2022-10-18 6:24 ` Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 " Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> Xiaoming Ni 2022-10-19 3:09 ` [PATCH v6 2/2] squashfs: Allows users to configure the number of decompression threads Xiaoming Ni 2022-10-27 22:44 ` [PATCH v6 0/2] squashfs: Add the mount parameter "threads=" Phillip Lougher
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.