* [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file
@ 2023-03-30 16:28 Yangtao Li via Linux-f2fs-devel
2023-04-03 3:46 ` Chao Yu
2023-04-03 13:33 ` Yangtao Li via Linux-f2fs-devel
0 siblings, 2 replies; 5+ messages in thread
From: Yangtao Li via Linux-f2fs-devel @ 2023-03-30 16:28 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu, Nick Terrell
Cc: linux-f2fs-devel, linux-kernel, Yangtao Li
Commit 3fde13f817e2 ("f2fs: compress: support compress level")
forgot to do basic compress level check, let's add it.
Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
fs/f2fs/inode.c | 94 +++++++++++++++++++++++++------------
include/linux/zstd_lib.h | 3 ++
lib/zstd/compress/clevels.h | 4 --
3 files changed, 67 insertions(+), 34 deletions(-)
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index bb5b365a195d..e63f75168700 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -10,6 +10,8 @@
#include <linux/buffer_head.h>
#include <linux/writeback.h>
#include <linux/sched/mm.h>
+#include <linux/lz4.h>
+#include <linux/zstd.h>
#include "f2fs.h"
#include "node.h"
@@ -202,6 +204,66 @@ void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, page));
}
+static bool sanity_check_compress_inode(struct inode *inode,
+ struct f2fs_inode *ri)
+{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ unsigned char compress_level;
+
+ if (ri->i_compress_algorithm >= COMPRESS_MAX) {
+ set_sbi_flag(sbi, SBI_NEED_FSCK);
+ f2fs_warn(sbi,
+ "%s: inode (ino=%lx) has unsupported compress algorithm: %u, run fsck to fix",
+ __func__, inode->i_ino, ri->i_compress_algorithm);
+ return false;
+ }
+ if (le64_to_cpu(ri->i_compr_blocks) >
+ SECTOR_TO_BLOCK(inode->i_blocks)) {
+ set_sbi_flag(sbi, SBI_NEED_FSCK);
+ f2fs_warn(sbi,
+ "%s: inode (ino=%lx) has inconsistent i_compr_blocks:%llu, i_blocks:%llu, run fsck to fix",
+ __func__, inode->i_ino, le64_to_cpu(ri->i_compr_blocks),
+ SECTOR_TO_BLOCK(inode->i_blocks));
+ return false;
+ }
+ if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
+ ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
+ set_sbi_flag(sbi, SBI_NEED_FSCK);
+ f2fs_warn(sbi,
+ "%s: inode (ino=%lx) has unsupported log cluster size: %u, run fsck to fix",
+ __func__, inode->i_ino, ri->i_log_cluster_size);
+ return false;
+ }
+
+ compress_level = le16_to_cpu(ri->i_compress_flag) >> COMPRESS_LEVEL_OFFSET;
+ switch (ri->i_compress_algorithm) {
+ case COMPRESS_LZO:
+ case COMPRESS_LZORLE:
+ if (compress_level)
+ goto err;
+ break;
+ case COMPRESS_LZ4:
+ if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
+ compress_level > LZ4HC_MAX_CLEVEL)
+ goto err;
+ break;
+ case COMPRESS_ZSTD:
+ if (!compress_level || compress_level > ZSTD_MAX_CLEVEL)
+ goto err;
+ break;
+ default:
+ goto err;
+ }
+
+ return true;
+
+err:
+ set_sbi_flag(sbi, SBI_NEED_FSCK);
+ f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported compress level: %u, run fsck to fix",
+ __func__, inode->i_ino, compress_level);
+ return false;
+}
+
static bool sanity_check_inode(struct inode *inode, struct page *node_page)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
@@ -285,36 +347,8 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page)
if (f2fs_has_extra_attr(inode) && f2fs_sb_has_compression(sbi) &&
fi->i_flags & F2FS_COMPR_FL &&
- F2FS_FITS_IN_INODE(ri, fi->i_extra_isize,
- i_log_cluster_size)) {
- if (ri->i_compress_algorithm >= COMPRESS_MAX) {
- set_sbi_flag(sbi, SBI_NEED_FSCK);
- f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported "
- "compress algorithm: %u, run fsck to fix",
- __func__, inode->i_ino,
- ri->i_compress_algorithm);
- return false;
- }
- if (le64_to_cpu(ri->i_compr_blocks) >
- SECTOR_TO_BLOCK(inode->i_blocks)) {
- set_sbi_flag(sbi, SBI_NEED_FSCK);
- f2fs_warn(sbi, "%s: inode (ino=%lx) has inconsistent "
- "i_compr_blocks:%llu, i_blocks:%llu, run fsck to fix",
- __func__, inode->i_ino,
- le64_to_cpu(ri->i_compr_blocks),
- SECTOR_TO_BLOCK(inode->i_blocks));
- return false;
- }
- if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
- ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
- set_sbi_flag(sbi, SBI_NEED_FSCK);
- f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported "
- "log cluster size: %u, run fsck to fix",
- __func__, inode->i_ino,
- ri->i_log_cluster_size);
- return false;
- }
- }
+ F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_log_cluster_size))
+ sanity_check_compress_inode(inode, ri);
return true;
}
diff --git a/include/linux/zstd_lib.h b/include/linux/zstd_lib.h
index 79d55465d5c1..ff55f41c73d3 100644
--- a/include/linux/zstd_lib.h
+++ b/include/linux/zstd_lib.h
@@ -88,6 +88,9 @@ ZSTDLIB_API const char* ZSTD_versionString(void);
# define ZSTD_CLEVEL_DEFAULT 3
#endif
+/*-===== Pre-defined compression levels =====-*/
+#define ZSTD_MAX_CLEVEL 22
+
/* *************************************
* Constants
***************************************/
diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
index d9a76112ec3a..b040d9d29089 100644
--- a/lib/zstd/compress/clevels.h
+++ b/lib/zstd/compress/clevels.h
@@ -14,10 +14,6 @@
#define ZSTD_STATIC_LINKING_ONLY /* ZSTD_compressionParameters */
#include <linux/zstd.h>
-/*-===== Pre-defined compression levels =====-*/
-
-#define ZSTD_MAX_CLEVEL 22
-
__attribute__((__unused__))
static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEVEL+1] = {
--
2.35.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file
2023-03-30 16:28 [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file Yangtao Li via Linux-f2fs-devel
@ 2023-04-03 3:46 ` Chao Yu
2023-04-03 13:01 ` Chao Yu
2023-04-03 13:33 ` Yangtao Li via Linux-f2fs-devel
1 sibling, 1 reply; 5+ messages in thread
From: Chao Yu @ 2023-04-03 3:46 UTC (permalink / raw)
To: Yangtao Li, Jaegeuk Kim, Nick Terrell; +Cc: linux-kernel, linux-f2fs-devel
On 2023/3/31 0:28, Yangtao Li wrote:
> Commit 3fde13f817e2 ("f2fs: compress: support compress level")
> forgot to do basic compress level check, let's add it.
>
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
> fs/f2fs/inode.c | 94 +++++++++++++++++++++++++------------
> include/linux/zstd_lib.h | 3 ++
> lib/zstd/compress/clevels.h | 4 --
> 3 files changed, 67 insertions(+), 34 deletions(-)
>
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index bb5b365a195d..e63f75168700 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -10,6 +10,8 @@
> #include <linux/buffer_head.h>
> #include <linux/writeback.h>
> #include <linux/sched/mm.h>
> +#include <linux/lz4.h>
> +#include <linux/zstd.h>
>
> #include "f2fs.h"
> #include "node.h"
> @@ -202,6 +204,66 @@ void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
> ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, page));
> }
>
> +static bool sanity_check_compress_inode(struct inode *inode,
> + struct f2fs_inode *ri)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + unsigned char compress_level;
> +
> + if (ri->i_compress_algorithm >= COMPRESS_MAX) {
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + f2fs_warn(sbi,
> + "%s: inode (ino=%lx) has unsupported compress algorithm: %u, run fsck to fix",
> + __func__, inode->i_ino, ri->i_compress_algorithm);
> + return false;
> + }
> + if (le64_to_cpu(ri->i_compr_blocks) >
> + SECTOR_TO_BLOCK(inode->i_blocks)) {
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + f2fs_warn(sbi,
> + "%s: inode (ino=%lx) has inconsistent i_compr_blocks:%llu, i_blocks:%llu, run fsck to fix",
> + __func__, inode->i_ino, le64_to_cpu(ri->i_compr_blocks),
> + SECTOR_TO_BLOCK(inode->i_blocks));
> + return false;
> + }
> + if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
> + ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + f2fs_warn(sbi,
> + "%s: inode (ino=%lx) has unsupported log cluster size: %u, run fsck to fix",
> + __func__, inode->i_ino, ri->i_log_cluster_size);
> + return false;
> + }
> +
> + compress_level = le16_to_cpu(ri->i_compress_flag) >> COMPRESS_LEVEL_OFFSET;
Exceed 80 lines.
> + switch (ri->i_compress_algorithm) {
> + case COMPRESS_LZO:
> + case COMPRESS_LZORLE:
> + if (compress_level)
> + goto err;
> + break;
> + case COMPRESS_LZ4:
> + if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
> + compress_level > LZ4HC_MAX_CLEVEL)
> + goto err;
> + break;
> + case COMPRESS_ZSTD:
> + if (!compress_level || compress_level > ZSTD_MAX_CLEVEL)
> + goto err;
> + break;
> + default:
> + goto err;
> + }
> +
> + return true;
> +
> +err:
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported compress level: %u, run fsck to fix",
> + __func__, inode->i_ino, compress_level);
> + return false;
> +}
> +
> static bool sanity_check_inode(struct inode *inode, struct page *node_page)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> @@ -285,36 +347,8 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page)
>
> if (f2fs_has_extra_attr(inode) && f2fs_sb_has_compression(sbi) &&
> fi->i_flags & F2FS_COMPR_FL &&
> - F2FS_FITS_IN_INODE(ri, fi->i_extra_isize,
> - i_log_cluster_size)) {
> - if (ri->i_compress_algorithm >= COMPRESS_MAX) {
> - set_sbi_flag(sbi, SBI_NEED_FSCK);
> - f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported "
> - "compress algorithm: %u, run fsck to fix",
> - __func__, inode->i_ino,
> - ri->i_compress_algorithm);
> - return false;
> - }
> - if (le64_to_cpu(ri->i_compr_blocks) >
> - SECTOR_TO_BLOCK(inode->i_blocks)) {
> - set_sbi_flag(sbi, SBI_NEED_FSCK);
> - f2fs_warn(sbi, "%s: inode (ino=%lx) has inconsistent "
> - "i_compr_blocks:%llu, i_blocks:%llu, run fsck to fix",
> - __func__, inode->i_ino,
> - le64_to_cpu(ri->i_compr_blocks),
> - SECTOR_TO_BLOCK(inode->i_blocks));
> - return false;
> - }
> - if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
> - ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
> - set_sbi_flag(sbi, SBI_NEED_FSCK);
> - f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported "
> - "log cluster size: %u, run fsck to fix",
> - __func__, inode->i_ino,
> - ri->i_log_cluster_size);
> - return false;
> - }
> - }
> + F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_log_cluster_size))
Exceed 80 lines.
> + sanity_check_compress_inode(inode, ri);
Missed to check return value?
>
> return true;
> }
> diff --git a/include/linux/zstd_lib.h b/include/linux/zstd_lib.h
> index 79d55465d5c1..ff55f41c73d3 100644
> --- a/include/linux/zstd_lib.h
> +++ b/include/linux/zstd_lib.h
> @@ -88,6 +88,9 @@ ZSTDLIB_API const char* ZSTD_versionString(void);
> # define ZSTD_CLEVEL_DEFAULT 3
> #endif
>
> +/*-===== Pre-defined compression levels =====-*/
> +#define ZSTD_MAX_CLEVEL 22
> +
> /* *************************************
> * Constants
> ***************************************/
> diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
> index d9a76112ec3a..b040d9d29089 100644
> --- a/lib/zstd/compress/clevels.h
> +++ b/lib/zstd/compress/clevels.h
> @@ -14,10 +14,6 @@
> #define ZSTD_STATIC_LINKING_ONLY /* ZSTD_compressionParameters */
> #include <linux/zstd.h>
>
> -/*-===== Pre-defined compression levels =====-*/
> -
> -#define ZSTD_MAX_CLEVEL 22
Why not zstd_max_clevel()?
Thanks,
> -
> __attribute__((__unused__))
>
> static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEVEL+1] = {
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file
2023-04-03 3:46 ` Chao Yu
@ 2023-04-03 13:01 ` Chao Yu
0 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2023-04-03 13:01 UTC (permalink / raw)
To: Yangtao Li, Jaegeuk Kim, Nick Terrell; +Cc: linux-kernel, linux-f2fs-devel
On 2023/4/3 11:46, Chao Yu wrote:
> On 2023/3/31 0:28, Yangtao Li wrote:
>> Commit 3fde13f817e2 ("f2fs: compress: support compress level")
>> forgot to do basic compress level check, let's add it.
>>
>> Signed-off-by: Yangtao Li <frank.li@vivo.com>
>> ---
>> fs/f2fs/inode.c | 94 +++++++++++++++++++++++++------------
>> include/linux/zstd_lib.h | 3 ++
>> lib/zstd/compress/clevels.h | 4 --
>> 3 files changed, 67 insertions(+), 34 deletions(-)
>>
>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
>> index bb5b365a195d..e63f75168700 100644
>> --- a/fs/f2fs/inode.c
>> +++ b/fs/f2fs/inode.c
>> @@ -10,6 +10,8 @@
>> #include <linux/buffer_head.h>
>> #include <linux/writeback.h>
>> #include <linux/sched/mm.h>
>> +#include <linux/lz4.h>
>> +#include <linux/zstd.h>
>>
>> #include "f2fs.h"
>> #include "node.h"
>> @@ -202,6 +204,66 @@ void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
>> ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, page));
>> }
>>
>> +static bool sanity_check_compress_inode(struct inode *inode,
>> + struct f2fs_inode *ri)
>> +{
>> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> + unsigned char compress_level;
>> +
>> + if (ri->i_compress_algorithm >= COMPRESS_MAX) {
>> + set_sbi_flag(sbi, SBI_NEED_FSCK);
>> + f2fs_warn(sbi,
>> + "%s: inode (ino=%lx) has unsupported compress algorithm: %u, run fsck to fix",
>> + __func__, inode->i_ino, ri->i_compress_algorithm);
>> + return false;
>> + }
>> + if (le64_to_cpu(ri->i_compr_blocks) >
>> + SECTOR_TO_BLOCK(inode->i_blocks)) {
>> + set_sbi_flag(sbi, SBI_NEED_FSCK);
>> + f2fs_warn(sbi,
>> + "%s: inode (ino=%lx) has inconsistent i_compr_blocks:%llu, i_blocks:%llu, run fsck to fix",
>> + __func__, inode->i_ino, le64_to_cpu(ri->i_compr_blocks),
>> + SECTOR_TO_BLOCK(inode->i_blocks));
>> + return false;
>> + }
>> + if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
>> + ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
>> + set_sbi_flag(sbi, SBI_NEED_FSCK);
>> + f2fs_warn(sbi,
>> + "%s: inode (ino=%lx) has unsupported log cluster size: %u, run fsck to fix",
>> + __func__, inode->i_ino, ri->i_log_cluster_size);
>> + return false;
>> + }
>> +
>> + compress_level = le16_to_cpu(ri->i_compress_flag) >> COMPRESS_LEVEL_OFFSET;
>
> Exceed 80 lines.
Sorry, colunms... out of my mind.
>
>> + switch (ri->i_compress_algorithm) {
>> + case COMPRESS_LZO:
>> + case COMPRESS_LZORLE:
>> + if (compress_level)
>> + goto err;
>> + break;
>> + case COMPRESS_LZ4:
>> + if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
>> + compress_level > LZ4HC_MAX_CLEVEL)
>> + goto err;
>> + break;
>> + case COMPRESS_ZSTD:
>> + if (!compress_level || compress_level > ZSTD_MAX_CLEVEL)
>> + goto err;
>> + break;
>> + default:
>> + goto err;
>> + }
>> +
>> + return true;
>> +
>> +err:
>> + set_sbi_flag(sbi, SBI_NEED_FSCK);
>> + f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported compress level: %u, run fsck to fix",
>> + __func__, inode->i_ino, compress_level);
>> + return false;
>> +}
>> +
>> static bool sanity_check_inode(struct inode *inode, struct page *node_page)
>> {
>> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> @@ -285,36 +347,8 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page)
>>
>> if (f2fs_has_extra_attr(inode) && f2fs_sb_has_compression(sbi) &&
>> fi->i_flags & F2FS_COMPR_FL &&
>> - F2FS_FITS_IN_INODE(ri, fi->i_extra_isize,
>> - i_log_cluster_size)) {
>> - if (ri->i_compress_algorithm >= COMPRESS_MAX) {
>> - set_sbi_flag(sbi, SBI_NEED_FSCK);
>> - f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported "
>> - "compress algorithm: %u, run fsck to fix",
>> - __func__, inode->i_ino,
>> - ri->i_compress_algorithm);
>> - return false;
>> - }
>> - if (le64_to_cpu(ri->i_compr_blocks) >
>> - SECTOR_TO_BLOCK(inode->i_blocks)) {
>> - set_sbi_flag(sbi, SBI_NEED_FSCK);
>> - f2fs_warn(sbi, "%s: inode (ino=%lx) has inconsistent "
>> - "i_compr_blocks:%llu, i_blocks:%llu, run fsck to fix",
>> - __func__, inode->i_ino,
>> - le64_to_cpu(ri->i_compr_blocks),
>> - SECTOR_TO_BLOCK(inode->i_blocks));
>> - return false;
>> - }
>> - if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
>> - ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
>> - set_sbi_flag(sbi, SBI_NEED_FSCK);
>> - f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported "
>> - "log cluster size: %u, run fsck to fix",
>> - __func__, inode->i_ino,
>> - ri->i_log_cluster_size);
>> - return false;
>> - }
>> - }
>> + F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_log_cluster_size))
>
> Exceed 80 lines.
Ditto.
Thanks,
>
>> + sanity_check_compress_inode(inode, ri);
>
> Missed to check return value?
>
>>
>> return true;
>> }
>> diff --git a/include/linux/zstd_lib.h b/include/linux/zstd_lib.h
>> index 79d55465d5c1..ff55f41c73d3 100644
>> --- a/include/linux/zstd_lib.h
>> +++ b/include/linux/zstd_lib.h
>> @@ -88,6 +88,9 @@ ZSTDLIB_API const char* ZSTD_versionString(void);
>> # define ZSTD_CLEVEL_DEFAULT 3
>> #endif
>>
>> +/*-===== Pre-defined compression levels =====-*/
>> +#define ZSTD_MAX_CLEVEL 22
>> +
>> /* *************************************
>> * Constants
>> ***************************************/
>> diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
>> index d9a76112ec3a..b040d9d29089 100644
>> --- a/lib/zstd/compress/clevels.h
>> +++ b/lib/zstd/compress/clevels.h
>> @@ -14,10 +14,6 @@
>> #define ZSTD_STATIC_LINKING_ONLY /* ZSTD_compressionParameters */
>> #include <linux/zstd.h>
>>
>> -/*-===== Pre-defined compression levels =====-*/
>> -
>> -#define ZSTD_MAX_CLEVEL 22
>
> Why not zstd_max_clevel()?
>
> Thanks,
>
>> -
>> __attribute__((__unused__))
>>
>> static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEVEL+1] = {
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file
2023-03-30 16:28 [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file Yangtao Li via Linux-f2fs-devel
2023-04-03 3:46 ` Chao Yu
@ 2023-04-03 13:33 ` Yangtao Li via Linux-f2fs-devel
2023-04-04 1:16 ` Chao Yu
1 sibling, 1 reply; 5+ messages in thread
From: Yangtao Li via Linux-f2fs-devel @ 2023-04-03 13:33 UTC (permalink / raw)
To: linux-f2fs-devel; +Cc: jaegeuk, terrelln, linux-kernel, frank.li
Hi Chao,
> Why not zstd_max_clevel()?
zstd_max_clevel() is only defined when CONFIG_F2FS_FS_ZSTD is enabled,
using zstd_max_clevel() will result in compile errors otherwise.
If using the following code,
----------------------------------------------------------------------------
switch (ri->i_compress_algorithm) {
case COMPRESS_LZO:
case COMPRESS_LZORLE:
if (compress_level)
goto err;
break;
case COMPRESS_LZ4:
if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
compress_level > LZ4HC_MAX_CLEVEL)
goto err;
break;
#ifdef CONFIG_F2FS_FS_ZSTD
case COMPRESS_ZSTD:
if (!compress_level || compress_level > zstd_max_clevel())
goto err;
break;
#endif
default:
goto err;
}
----------------------------------------------------------------------------
then we will get this result:
F2FS-fs (loop0): sanity_check_compress_inode: inode (ino=4) has
unsupported compress level: 0, run fsck to fix
Another way is to use the following code, which ignores the check for
level when CONFIG_F2FS_FS_ZSTD is not enabled.
----------------------------------------------------------------------------
switch (ri->i_compress_algorithm) {
case COMPRESS_LZO:
case COMPRESS_LZORLE:
if (compress_level)
goto err;
break;
case COMPRESS_LZ4:
if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
compress_level > LZ4HC_MAX_CLEVEL)
goto err;
break;
case COMPRESS_ZSTD:
#ifdef CONFIG_F2FS_FS_ZSTD
if (!compress_level || compress_level > zstd_max_clevel())
goto err;
break;
#else
return true;
#endif
default:
goto err;
}
----------------------------------------------------------------------------
Perhaps exporting ZSTD_MAX_CLEVEL is a better choice?
Thx,
Yangtao
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file
2023-04-03 13:33 ` Yangtao Li via Linux-f2fs-devel
@ 2023-04-04 1:16 ` Chao Yu
0 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2023-04-04 1:16 UTC (permalink / raw)
To: Yangtao Li, linux-f2fs-devel; +Cc: jaegeuk, terrelln, linux-kernel
On 2023/4/3 21:33, Yangtao Li wrote:
> Hi Chao,
>
>> Why not zstd_max_clevel()?
>
> zstd_max_clevel() is only defined when CONFIG_F2FS_FS_ZSTD is enabled,
> using zstd_max_clevel() will result in compile errors otherwise.
>
> If using the following code,
>
> ----------------------------------------------------------------------------
> switch (ri->i_compress_algorithm) {
> case COMPRESS_LZO:
> case COMPRESS_LZORLE:
> if (compress_level)
> goto err;
> break;
> case COMPRESS_LZ4:
> if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
> compress_level > LZ4HC_MAX_CLEVEL)
> goto err;
> break;
> #ifdef CONFIG_F2FS_FS_ZSTD
> case COMPRESS_ZSTD:
Hi Yangtao,
How about:
#ifdef CONFIG_F2FS_FS_ZSTD
if (!compress_level || compress_level > zstd_max_clevel())
goto err;
#endif
break;
> if (!compress_level || compress_level > zstd_max_clevel())
> goto err;
> break;
> #endif
> default:
> goto err;
> }
> ----------------------------------------------------------------------------
>
> then we will get this result:
>
> F2FS-fs (loop0): sanity_check_compress_inode: inode (ino=4) has
> unsupported compress level: 0, run fsck to fix
>
> Another way is to use the following code, which ignores the check for
> level when CONFIG_F2FS_FS_ZSTD is not enabled.
>
> ----------------------------------------------------------------------------
> switch (ri->i_compress_algorithm) {
> case COMPRESS_LZO:
> case COMPRESS_LZORLE:
> if (compress_level)
> goto err;
> break;
> case COMPRESS_LZ4:
> if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
> compress_level > LZ4HC_MAX_CLEVEL)
> goto err;
> break;
> case COMPRESS_ZSTD:
> #ifdef CONFIG_F2FS_FS_ZSTD
> if (!compress_level || compress_level > zstd_max_clevel())
> goto err;
> break;
> #else
> return true;
> #endif
> default:
> goto err;
> }
> ----------------------------------------------------------------------------
>
> Perhaps exporting ZSTD_MAX_CLEVEL is a better choice?
>
> Thx,
> Yangtao
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-04-04 1:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-30 16:28 [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file Yangtao Li via Linux-f2fs-devel
2023-04-03 3:46 ` Chao Yu
2023-04-03 13:01 ` Chao Yu
2023-04-03 13:33 ` Yangtao Li via Linux-f2fs-devel
2023-04-04 1:16 ` Chao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).