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
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 <[email protected]>
---
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
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/[email protected]/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 <[email protected]>
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
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 <[email protected]>
v1: https://lore.kernel.org/lkml/[email protected]/
----
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
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 <[email protected]>
>
> v1: https://lore.kernel.org/lkml/[email protected]/
> ----
>
> 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(-)
>
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 <[email protected]>
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/[email protected]/
>>
>> ----
>>
>> 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(-)
>>
>
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 <[email protected]>
>
> 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/[email protected]/
>>>
>>> ----
>>>
>>> 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(-)
>>>
>>
>
>
> .
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 <[email protected]>
>>
>> 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.
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
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
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/[email protected]/
fix warning: sparse: incorrect type in initializer (different address spaces)
Reported-by: kernel test robot <[email protected]>
v1: https://lore.kernel.org/lkml/[email protected]/
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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/[email protected]/
> fix warning: sparse: incorrect type in initializer (different address spaces)
> Reported-by: kernel test robot <[email protected]>
>
> v1: https://lore.kernel.org/lkml/[email protected]/
>
> 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(-)
>
Review comments below.
Xiaoming Ni <[email protected]> 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 <[email protected]>
>---
> 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
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 <[email protected]>
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;
On 2022/9/9 23:44, Phillip Lougher wrote:
> Review comments below.
>
> Xiaoming Ni <[email protected]> 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();
> +}
>
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 <[email protected]>
>
> 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
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/[email protected]/
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/[email protected]/
fix warning: sparse: incorrect type in initializer (different address spaces)
Reported-by: kernel test robot <[email protected]>
v1: https://lore.kernel.org/lkml/[email protected]/
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
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 <[email protected]>
---
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
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/[email protected]/
> 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/[email protected]/
> fix warning: sparse: incorrect type in initializer (different address spaces)
> Reported-by: kernel test robot <[email protected]>
>
> v1: https://lore.kernel.org/lkml/[email protected]/
>
> 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(-)
>
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/[email protected]/
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/[email protected]/
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/[email protected]/
fix warning: sparse: incorrect type in initializer (different address spaces)
Reported-by: kernel test robot <[email protected]>
v1: https://lore.kernel.org/lkml/[email protected]/
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
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 <[email protected]>
---
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
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/[email protected]/
> 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/[email protected]/
> 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/[email protected]/
> fix warning: sparse: incorrect type in initializer (different address spaces)
> Reported-by: kernel test robot <[email protected]>
>
> v1: https://lore.kernel.org/lkml/[email protected]/
>
> 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(-)
>
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
On Mon, Oct 17, 2022 at 2:11 AM Xiaoming Ni <[email protected]> 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/[email protected]/
> > 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/[email protected]/
> > 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/[email protected]/
> > fix warning: sparse: incorrect type in initializer (different address spaces)
> > Reported-by: kernel test robot <[email protected]>
> >
> > v1: https://lore.kernel.org/lkml/[email protected]/
> >
> > 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(-)
> >
>
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/[email protected]/
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/[email protected]/
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/[email protected]/
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/[email protected]/
fix warning: sparse: incorrect type in initializer (different address spaces)
Reported-by: kernel test robot <[email protected]>
v1: https://lore.kernel.org/lkml/[email protected]/
*** 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
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 <[email protected]>
---
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
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 <[email protected]>
> v5: https://lore.kernel.org/lkml/[email protected]/
> 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/[email protected]/
> 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/[email protected]/
> 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/[email protected]/
> fix warning: sparse: incorrect type in initializer (different address spaces)
> Reported-by: kernel test robot <[email protected]>
>
> v1: https://lore.kernel.org/lkml/[email protected]/
> *** 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(-)
>