Messages in this thread | | | Subject | Re: [PATCH v4 1/2] squashfs: add the mount parameter theads=<single|multi|percpu> | From | Xiaoming Ni <> | Date | Wed, 28 Sep 2022 11:06:49 +0800 |
| |
On 2022/9/28 10:20, Phillip Lougher wrote: > On 16/09/2022 09:36, Xiaoming Ni wrote: >> Squashfs supports three decompression concurrency modes: >> Single-thread mode: concurrent reads are blocked and the memory >> overhead is small. >> Multi-thread mode/percpu mode: reduces concurrent read blocking but >> increases memory overhead. >> >> The corresponding schema must be fixed at compile time. During mounting, >> the concurrent decompression mode cannot be adjusted based on file read >> blocking. >> >> The mount parameter theads=<single|multi|percpu> is added to select >> the concurrent decompression mode of a single SquashFS file system >> image. >> >> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> >> +#ifdef CONFIG_SQUASHFS_DECOMP_SINGLE >> + opts->thread_ops = &squashfs_decompressor_single; >> +#elif CONFIG_SQUASHFS_DECOMP_MULTI >> + opts->thread_ops = &squashfs_decompressor_multi; >> +#elif CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU > > In my previous review I asked you to fix the above two #elif > lines. You have done so in this patch series, but, only in the > second patch which seems perverse. > > The reason why this isn't a good approach. If you *only* apply this > patch, with the following Squashfs configuration options > I'm so sorry. I made a low-level mistake in patching. I will re-check the previous review comments and resend the patch.
Thanks
> phillip@phoenix:/external/stripe/linux$ grep SQUASHFS .config > CONFIG_SQUASHFS=y > # CONFIG_SQUASHFS_FILE_CACHE is not set > CONFIG_SQUASHFS_FILE_DIRECT=y > CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU=y > # CONFIG_SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set > # CONFIG_SQUASHFS_COMPILE_DECOMP_SINGLE is not set > # CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI is not set > CONFIG_SQUASHFS_COMPILE_DECOMP_MULTI_PERCPU=y > CONFIG_SQUASHFS_XATTR=y > CONFIG_SQUASHFS_ZLIB=y > CONFIG_SQUASHFS_LZ4=y > CONFIG_SQUASHFS_LZO=y > CONFIG_SQUASHFS_XZ=y > CONFIG_SQUASHFS_ZSTD=y > # CONFIG_SQUASHFS_4K_DEVBLK_SIZE is not set > # CONFIG_SQUASHFS_EMBEDDED is not set > CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE=3 > > > You will get the following build warning > > phillip@phoenix:/external/stripe/linux$ make bzImage > SYNC include/config/auto.conf.cmd > CALL scripts/checksyscalls.sh > CALL scripts/atomic/check-atomics.sh > DESCEND objtool > CHK include/generated/compile.h > UPD kernel/config_data > GZIP kernel/config_data.gz > CC kernel/configs.o > AR kernel/built-in.a > CC fs/squashfs/block.o > CC fs/squashfs/cache.o > CC fs/squashfs/dir.o > CC fs/squashfs/export.o > CC fs/squashfs/file.o > CC fs/squashfs/fragment.o > CC fs/squashfs/id.o > CC fs/squashfs/inode.o > CC fs/squashfs/namei.o > CC fs/squashfs/super.o > fs/squashfs/super.c: In function ‘squashfs_init_fs_context’: > fs/squashfs/super.c:492:7: warning: "CONFIG_SQUASHFS_DECOMP_MULTI" is > not defined, evaluates to 0 [-Wundef] > 492 | #elif CONFIG_SQUASHFS_DECOMP_MULTI > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > This strictly breaks the git bisectability rule. Every patch should > be compilable and not break the build or produce warnings. If not > it makes git bisect difficult to use to find regressions. > > This can be avoided by fixing the issue in *this* patch. So > please do so. > > Thanks > > Phillip > >> + opts->thread_ops = &squashfs_decompressor_percpu; >> +#endif >> fc->fs_private = opts; >> fc->ops = &squashfs_context_ops; >> return 0; >> @@ -478,7 +526,7 @@ static void squashfs_put_super(struct super_block >> *sb) >> squashfs_cache_delete(sbi->block_cache); >> squashfs_cache_delete(sbi->fragment_cache); >> squashfs_cache_delete(sbi->read_page); >> - squashfs_decompressor_destroy(sbi); >> + sbi->thread_ops->destroy(sbi); >> kfree(sbi->id_table); >> kfree(sbi->fragment_index); >> kfree(sbi->meta_index); > > > .
| |