Re: [PATCH v4 1/6] f2fs: add helper to check compression level

Jaegeuk Kim
2023-06-12T22:15:00Z

Could you please check this version?

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev-test&id=9c84aad379019a0d86655bb50bd7b4bc92683c4b

On 06/12, Sheng Yong wrote:
> This patch adds a helper function to check if compression level is
> valid.
>
> Signed-off-by: Sheng Yong <shengyong@oppo.com>
> ---
> fs/f2fs/compress.c | 31 +++++++++++++++++++++++++++++++
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/super.c | 4 ++--
> 3 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 1132d3cd8f337..63a496137ebe7 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -55,6 +55,7 @@ struct f2fs_compress_ops {
> int (*init_decompress_ctx)(struct decompress_io_ctx *dic);
> void (*destroy_decompress_ctx)(struct decompress_io_ctx *dic);
> int (*decompress_pages)(struct decompress_io_ctx *dic);
> + bool (*is_level_valid)(int level);
> };
>
> static unsigned int offset_in_cluster(struct compress_ctx *cc, pgoff_t index)
> @@ -308,11 +309,23 @@ static int lz4_decompress_pages(struct decompress_io_ctx *dic)
> return 0;
> }
>
> +static bool lz4_is_level_valid(int lvl)
> +{
> + if (lvl == 0)
> + return true;
> +#ifdef CONFIG_F2FS_FS_LZ4HC
> + if (lvl >= LZ4HC_MIN_CLEVEL && lvl <= LZ4HC_MAX_CLEVEL)
> + return true;
> +#endif
> + return false;
> +}
> +
> static const struct f2fs_compress_ops f2fs_lz4_ops = {
> .init_compress_ctx = lz4_init_compress_ctx,
> .destroy_compress_ctx = lz4_destroy_compress_ctx,
> .compress_pages = lz4_compress_pages,
> .decompress_pages = lz4_decompress_pages,
> + .is_level_valid = lz4_is_level_valid,
> };
> #endif
>
> @@ -477,6 +490,13 @@ static int zstd_decompress_pages(struct decompress_io_ctx *dic)
> return 0;
> }
>
> +static bool zstd_is_level_valid(int lvl)
> +{
> + if (lvl < zstd_min_clevel() || lvl > zstd_max_clevel())
> + return false;
> + return true;
> +}
> +
> static const struct f2fs_compress_ops f2fs_zstd_ops = {
> .init_compress_ctx = zstd_init_compress_ctx,
> .destroy_compress_ctx = zstd_destroy_compress_ctx,
> @@ -484,6 +504,7 @@ static const struct f2fs_compress_ops f2fs_zstd_ops = {
> .init_decompress_ctx = zstd_init_decompress_ctx,
> .destroy_decompress_ctx = zstd_destroy_decompress_ctx,
> .decompress_pages = zstd_decompress_pages,
> + .is_level_valid = zstd_is_level_valid,
> };
> #endif
>
> @@ -542,6 +563,16 @@ bool f2fs_is_compress_backend_ready(struct inode *inode)
> return f2fs_cops[F2FS_I(inode)->i_compress_algorithm];
> }
>
> +bool f2fs_is_compress_level_valid(int alg, int lvl)
> +{
> + const struct f2fs_compress_ops *cops = f2fs_cops[alg];
> +
> + if (cops->is_level_valid)
> + return cops->is_level_valid(lvl);
> +
> + return lvl == 0;
> +}
> +
> static mempool_t *compress_page_pool;
> static int num_compress_pages = 512;
> module_param(num_compress_pages, uint, 0444);
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 80c783215b5a3..1b17bbe7e8656 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4236,6 +4236,7 @@ bool f2fs_compress_write_end(struct inode *inode, void *fsdata,
> int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool lock);
> void f2fs_compress_write_end_io(struct bio *bio, struct page *page);
> bool f2fs_is_compress_backend_ready(struct inode *inode);
> +bool f2fs_is_compress_level_valid(int alg, int lvl);
> int __init f2fs_init_compress_mempool(void);
> void f2fs_destroy_compress_mempool(void);
> void f2fs_decompress_cluster(struct decompress_io_ctx *dic, bool in_task);
> @@ -4300,6 +4301,7 @@ static inline bool f2fs_is_compress_backend_ready(struct inode *inode)
> /* not support compression */
> return false;
> }
> +static inline bool f2fs_is_compress_level_valid(int alg, int lvl) { return false; }
> static inline struct page *f2fs_compress_control_page(struct page *page)
> {
> WARN_ON_ONCE(1);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 8fd23caa1ed99..023981824d240 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -606,7 +606,7 @@ static int f2fs_set_lz4hc_level(struct f2fs_sb_info *sbi, const char *str)
> if (kstrtouint(str + 1, 10, &level))
> return -EINVAL;
>
> - if (level < LZ4HC_MIN_CLEVEL || level > LZ4HC_MAX_CLEVEL) {
> + if (!f2fs_is_compress_level_valid(COMPRESS_LZ4, level)) {
> f2fs_info(sbi, "invalid lz4hc compress level: %d", level);
> return -EINVAL;
> }
> @@ -640,7 +640,7 @@ static int f2fs_set_zstd_level(struct f2fs_sb_info *sbi, const char *str)
> if (kstrtouint(str + 1, 10, &level))
> return -EINVAL;
>
> - if (!level || level > zstd_max_clevel()) {
> + if (!f2fs_is_compress_level_valid(COMPRESS_ZSTD, level)) {
> f2fs_info(sbi, "invalid zstd compress level: %d", level);
> return -EINVAL;
> }
> --
> 2.40.1