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

[Sheng Yong via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 2023/6/13 6:14, Jaegeuk Kim wrote:
> Could you please check this version?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev-test&id=9c84aad379019a0d86655bb50bd7b4bc92683c4b

Hi, Jaegeuk,

Thanks for cleaning it up. This version looks good to me. I tested it
with my testcases, and all are passed.

thanks,
shengyong
> 
> 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


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel