Re: [f2fs-dev] [PATCH 1/4] f2fs: fix to check lz4hc compression when CONFIG_F2FS_FS_LZ4HC is not enabled

[Chao Yu <chao@kernel.org>]

On 2023/1/24 23:33, Yangtao Li wrote:
> f2fs supports lz4 compression algorithm and lz4hc compression algorithm,
> which the level parameter needs to be passed in. When CONFIG_F2FS_FS_LZ4HC
> is not enabled, even if there is no problem with the level parameter, add
> the level parameter to the lz4 algorithm will cause the mount to fail.
> 
> Let's change it to be the same as other compression algorithms. When the
> kernel does not enable the algorithm, ignore this parameter and print msg.
> 
> Fixes: 3fde13f817e2 ("f2fs: compress: support compress level")
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/f2fs/super.c | 31 ++++++++++++++-----------------
>   1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d8a65645ee48..ad5df4d5c39a 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -588,19 +588,11 @@ static int f2fs_test_compress_extension(struct f2fs_sb_info *sbi)
>   	return 0;
>   }
>   
> -#ifdef CONFIG_F2FS_FS_LZ4
> +#ifdef CONFIG_F2FS_FS_LZ4HC
>   static int f2fs_set_lz4hc_level(struct f2fs_sb_info *sbi, const char *str)
>   {
> -#ifdef CONFIG_F2FS_FS_LZ4HC
>   	unsigned int level;
> -#endif
>   
> -	if (strlen(str) == 3) {
> -		F2FS_OPTION(sbi).compress_level = 0;
> -		return 0;
> -	}
> -
> -#ifdef CONFIG_F2FS_FS_LZ4HC
>   	str += 3;
>   
>   	if (str[0] != ':') {
> @@ -617,10 +609,6 @@ static int f2fs_set_lz4hc_level(struct f2fs_sb_info *sbi, const char *str)
>   
>   	F2FS_OPTION(sbi).compress_level = level;
>   	return 0;
> -#else
> -	f2fs_info(sbi, "kernel doesn't support lz4hc compression");
> -	return -EINVAL;
> -#endif
>   }
>   #endif
>   
> @@ -1085,10 +1073,19 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>   #endif
>   			} else if (!strncmp(name, "lz4", 3)) {
>   #ifdef CONFIG_F2FS_FS_LZ4
> -				ret = f2fs_set_lz4hc_level(sbi, name);
> -				if (ret) {
> -					kfree(name);
> -					return -EINVAL;
> +				if (strlen(name) == 3) {
> +					F2FS_OPTION(sbi).compress_level = 0;
> +				} else {
> +#ifdef CONFIG_F2FS_FS_LZ4HC
> +					ret = f2fs_set_lz4hc_level(sbi, name);
> +					if (ret) {
> +						kfree(name);
> +						return -EINVAL;
> +					}
> +#else
> +					f2fs_info(sbi, "kernel doesn't support lz4hc compression");

It needs to check <alg_name>:<compr_level> format to make sure user wants to
enable lz4hc w/ specified level, otherwise if parameter is lz4xx, it doesn't
make sense to print:

"kernel doesn't support lz4hc compression"

> +					break;

It will cause memory leak for name.

Thanks,

> +#endif
>   				}
>   				F2FS_OPTION(sbi).compress_algorithm =
>   								COMPRESS_LZ4;


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