2023-04-07 18:38:48

by 李扬韬

[permalink] [raw]
Subject: [PATCH v3] f2fs: add sanity compress level check for compressed file

Commit 3fde13f817e2 ("f2fs: compress: support compress level")
forgot to do basic compress level check, let's add it.

Signed-off-by: Yangtao Li <[email protected]>
---
fs/f2fs/inode.c | 106 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 77 insertions(+), 29 deletions(-)

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index bb5b365a195d..c2460f51bf80 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,79 @@ 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) {
+ f2fs_warn(sbi,
+ "%s: inode (ino=%lx) has unsupported compress algorithm: %u, run fsck to fix",
+ __func__, inode->i_ino, ri->i_compress_algorithm);
+ goto err;
+ }
+ if (le64_to_cpu(ri->i_compr_blocks) >
+ SECTOR_TO_BLOCK(inode->i_blocks)) {
+ 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));
+ goto err;
+ }
+ if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
+ ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
+ 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);
+ goto err;
+ }
+
+ compress_level = le16_to_cpu(ri->i_compress_flag) >>
+ COMPRESS_LEVEL_OFFSET;
+ switch (ri->i_compress_algorithm) {
+ case COMPRESS_LZO:
+#ifdef CONFIG_F2FS_FS_LZO
+ if (compress_level)
+ goto err_level;
+#endif
+ break;
+ case COMPRESS_LZORLE:
+#ifdef CONFIG_F2FS_FS_LZORLE
+ if (compress_level)
+ goto err_level;
+#endif
+ break;
+ case COMPRESS_LZ4:
+#ifdef CONFIG_F2FS_FS_LZ4
+#ifdef CONFIG_F2FS_FS_LZ4HC
+ if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
+ compress_level > LZ4HC_MAX_CLEVEL)
+#else
+ if (compress_level)
+#endif
+ goto err_level;
+#endif
+ break;
+ case COMPRESS_ZSTD:
+#ifdef CONFIG_F2FS_FS_ZSTD
+ if (!compress_level || compress_level > zstd_max_clevel())
+ goto err_level;
+#endif
+ break;
+ default:
+ goto err_level;
+ }
+
+ return true;
+err_level:
+ f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported compress level: %u, run fsck to fix",
+ __func__, inode->i_ino, compress_level);
+err:
+ set_sbi_flag(sbi, SBI_NEED_FSCK);
+ return false;
+}
+
static bool sanity_check_inode(struct inode *inode, struct page *node_page)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
@@ -286,35 +361,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;
- }
- }
+ i_log_cluster_size))
+ return sanity_check_compress_inode(inode, ri);

return true;
}
--
2.35.1


2023-05-06 15:53:24

by 李扬韬

[permalink] [raw]
Subject: Re: [PATCH v3] f2fs: add sanity compress level check for compressed file

ping......

> On 2023/4/8 2:31, Yangtao Li wrote:

2023-05-18 01:51:51

by patchwork-bot+f2fs

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3] f2fs: add sanity compress level check for compressed file

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <[email protected]>:

On Sat, 8 Apr 2023 02:31:47 +0800 you wrote:
> Commit 3fde13f817e2 ("f2fs: compress: support compress level")
> forgot to do basic compress level check, let's add it.
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
> fs/f2fs/inode.c | 106 +++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 77 insertions(+), 29 deletions(-)

Here is the summary with links:
- [f2fs-dev,v3] f2fs: add sanity compress level check for compressed file
https://git.kernel.org/jaegeuk/f2fs/c/8328548780c7

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2023-06-12 18:45:40

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH v3] f2fs: add sanity compress level check for compressed file

On 04/08, 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 <[email protected]>
> ---
> fs/f2fs/inode.c | 106 +++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 77 insertions(+), 29 deletions(-)
>
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index bb5b365a195d..c2460f51bf80 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,79 @@ 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) {
> + f2fs_warn(sbi,
> + "%s: inode (ino=%lx) has unsupported compress algorithm: %u, run fsck to fix",
> + __func__, inode->i_ino, ri->i_compress_algorithm);
> + goto err;
> + }
> + if (le64_to_cpu(ri->i_compr_blocks) >
> + SECTOR_TO_BLOCK(inode->i_blocks)) {
> + 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));
> + goto err;
> + }
> + if (ri->i_log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
> + ri->i_log_cluster_size > MAX_COMPRESS_LOG_SIZE) {
> + 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);
> + goto err;
> + }
> +
> + compress_level = le16_to_cpu(ri->i_compress_flag) >>
> + COMPRESS_LEVEL_OFFSET;
> + switch (ri->i_compress_algorithm) {
> + case COMPRESS_LZO:
> +#ifdef CONFIG_F2FS_FS_LZO
> + if (compress_level)
> + goto err_level;
> +#endif
> + break;
> + case COMPRESS_LZORLE:
> +#ifdef CONFIG_F2FS_FS_LZORLE
> + if (compress_level)
> + goto err_level;
> +#endif
> + break;
> + case COMPRESS_LZ4:
> +#ifdef CONFIG_F2FS_FS_LZ4
> +#ifdef CONFIG_F2FS_FS_LZ4HC
> + if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
> + compress_level > LZ4HC_MAX_CLEVEL)
> +#else
> + if (compress_level)
> +#endif
> + goto err_level;
> +#endif
> + break;
> + case COMPRESS_ZSTD:
> +#ifdef CONFIG_F2FS_FS_ZSTD
> + if (!compress_level || compress_level > zstd_max_clevel())

I think compress_level=0 is wrong, since that'll be changed to the default level later.
I'll modify to assign the correct level, so please revisit this patch later.

> + goto err_level;
> +#endif
> + break;
> + default:
> + goto err_level;
> + }
> +
> + return true;
> +err_level:
> + f2fs_warn(sbi, "%s: inode (ino=%lx) has unsupported compress level: %u, run fsck to fix",
> + __func__, inode->i_ino, compress_level);
> +err:
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + return false;
> +}
> +
> static bool sanity_check_inode(struct inode *inode, struct page *node_page)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> @@ -286,35 +361,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;
> - }
> - }
> + i_log_cluster_size))
> + return sanity_check_compress_inode(inode, ri);
>
> return true;
> }
> --
> 2.35.1