Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <44e2c24a-c94c-a991-d029-c0997a1db5d2@oppo.com>
Date:   Tue, 13 Jun 2023 10:28:32 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [PATCH v4 1/6] f2fs: add helper to check compression level
Content-Language: en-US
To:     Jaegeuk Kim <jaegeuk@kernel.org>
Cc:     chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org, ebiggers@kernel.org
References: <20230612030121.2393541-1-shengyong@oppo.com>
 <20230612030121.2393541-2-shengyong@oppo.com> <ZIeY0qNjXgx419NZ@google.com>
From:   Sheng Yong <shengyong@oppo.com>
In-Reply-To: <ZIeY0qNjXgx419NZ@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VnVMNGl0Qzg1bXNQWGRzbkl1Z2dCZjhUYnBGb1JYZkwxRUpxWUFIZnR4UXZ3?=
 =?utf-8?B?eGZhMkxpV2FVbzRud3hCczNVdTBrWTFFSnZvOGF5V28yTGloaFIyTVFlUVVL?=
 =?utf-8?B?TEYzYWJ5TCt6Q0l2SmJnZWtBMUNpWmhDTDd1Y0JDUmpMd0tDOFdadHFFOWxx?=
 =?utf-8?B?UGZMeTl3NXRTVyszTE1FenZIS2JmM2daTkl0bzZyV2xWckwrSVdKdU54OHBw?=
 =?utf-8?B?RTd5dFRaM2locnNZWC9FVzMwVm85dlV4d3JMcExYU0hjeVlzS0FWcVFOaU16?=
 =?utf-8?B?Y1djWVRIS1hDWkNVYWNHL2VqUUF1YWIrTWRWT2ZwSEhYeXh6RlVrMXdzeEc5?=
 =?utf-8?B?cGQwaHRTWG90UGhIYWJYVTlkQTZiUS9HU0Q4YTYraURaZUZSUEF0RGtlbFEx?=
 =?utf-8?B?aVpXYk9UZnZQRjlqdEx6UmE2WGU5Zk53ZTNVc1Fsck9EczFkUElSblQvUDBX?=
 =?utf-8?B?YUkvMU5UOVFIeXZmUWJBSDFXVHVDNDF4MmZRSzNLNUhidHUwTXVRcTNDcXJw?=
 =?utf-8?B?NnlQditUQU0zV2J5SXBHM05CVVhMcTYyaDU0cWF2eHJyQjROZkszcWZOTEdz?=
 =?utf-8?B?VlYyZkxlNnU4aUl4TTcwZElzMHNCNUVpM3RMYkJZTlBELzUyOE9XdStpUzVY?=
 =?utf-8?B?dk1LeDhyM3cvQ0EzUnE3OXh0UDgrVm1Wam5pUVdSbnNrQWUxSXdpY2RBcURE?=
 =?utf-8?B?T1lJV213SUltR09mdHJNciswU0lnSnloSnZ5ckEwdnNjUkFQQWcrV1RyQlpI?=
 =?utf-8?B?VXQ3Zk5oMGdFSWJuam5EV3dhcm5taWtsd2krK1lSa2Nqbm5UWFU2NzR0OGY5?=
 =?utf-8?B?Q2pSNXFpTlNjSFZOTGxKL1RJUGE3VFhqT09wTmkzcUxiejhPT01zWUpmVGhr?=
 =?utf-8?B?NnluRElDWFMzdUNsSVNMMjJhSXErdUtGckhpVzZjbDZyZEJuRFNyNUlvd3Nt?=
 =?utf-8?B?enNNUEdwU2tSQWN0dDFoOHYrbXMwUEF1T09kMHlLbCtDOWxZVXBhUlpuVVpv?=
 =?utf-8?B?SW5oa1dVckJJVTkzM0RsV0hxeitoZzlvYTdLNnZsV2pQcFkxbDNEZ25TNmg2?=
 =?utf-8?B?OEJvellyenl5UDFCWVZud1c3QUNoZjN0WmxKODNwUE1RSy9peHg1ZzU0ZlJ1?=
 =?utf-8?B?VXJVZkdvTml0WTM3Ui9xUFRhWVF1SExXQ3pIemRHZ1RSSEpGWERNRGhxMWJk?=
 =?utf-8?B?VkpKM1p0dDBSN1lwWldWa1o2Nml4cFl1emJWWHh0ZEoxcEdFOStZRmVBeXdI?=
 =?utf-8?B?bXVnQVBnVXZ3WVlvU1ExUVg4M2dSQ2xFMWNMMEJhTm5TWDlpczN2OTNxcXhs?=
 =?utf-8?B?QlBWSTZrNWdkcFdPb1IyYzQwbE9hb0swakFkZ3FrRzRQWngyVHhsQXNtSmpH?=
 =?utf-8?B?dTArWmhmTnVzS0pueEM4STFTSTYrSUNPQURZNWxabStjY1RQajdhbS9UTThv?=
 =?utf-8?B?ZHBqek1mVkhSakkrU3gvQzRMUllVV2dIMHAwY2E5RWdwWDRzaW1VcWZQYjBq?=
 =?utf-8?B?eFUvcUNEMDRrNWF3ZnFOMGdESFl3VlBtVkdJbDJjOUlYdmxXL1JISVZaUHk5?=
 =?utf-8?B?VUtnVnB0QzYzNjdldjVVY3JvanArdEJiOU8xWURTZGhhcWs1bmF3SWRMcXlm?=
 =?utf-8?B?M01hdi9mU3A3bkp4QWJlZTdwL3FyV01RL0ZjK0IxOWZmc0V2eGVwM0g2K3hE?=
 =?utf-8?B?Z0hFaHFSa1FnQ2tnMURZRVhnckwxNHJLVFRXYm9YdnMwSnFhdEYwaWVwMVRu?=
 =?utf-8?B?VHpkRndHZGM5MWZEcTJXUVNqR1hPc0VXSDkwUmhGWVV0c1EvZWg5TUphZ1J5?=
 =?utf-8?B?K1Z0T1MrU3oxVFFKUCtWT3pZVndhZ1A5SXpKY1ZxSHFOMmNQMW1sZlVsVGRL?=
 =?utf-8?B?OXBPMnBydW45R1FnRm01N2FBSDdDUmhpVHRGcXBSWUhiMEIrYmxjejhzRjRs?=
 =?utf-8?B?UVlndFgwSVVGWFRldGVNaXJ5L0RKREJyNEx6K0N3WkRzK3FUT0g3clhkYzBq?=
 =?utf-8?B?eG5zMnBGVlJSZEtoZVhQQ25ZRmxJd2ZLSjduYndVV2VUNHRoT00rZnNOWDY2?=
 =?utf-8?B?YW5aZnN3Q3lkWlNZZVdLaGsxVVBLbUlvcDlWVVdieWlCRE9DRXJqOVJ5RzJl?=
 =?utf-8?Q?NFVzHVCgZuXsOTyCCeHICSfz6?=
X-OriginatorOrg: oppo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ff512804-b03a-45d6-30a4-08db6bb5e3d1
X-MS-Exchange-CrossTenant-AuthSource: SI2PR02MB5148.apcprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jun 2023 02:28:35.9271
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f1905eb1-c353-41c5-9516-62b4a54b5ee6
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: jMEw63aTQIVhaUtcDoE9JVo3Xpp6DB1VtdXKVCAKWF49pSvUdjX3eGruo4+XK1XGsu1itFuCWOeM5q/BeYei4w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SEYPR02MB5871
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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