Messages in this thread | | | Date | Mon, 3 Apr 2023 21:01:08 +0800 | Subject | Re: [f2fs-dev] [RESEND] f2fs: add sanity compress level check for compressed file | From | Chao Yu <> |
| |
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
| |