This patch supports to store chksum value with compressed
data, and verify the integrality of compressed data while
reading the data.
The feature can be enabled through specifying mount option
'compress_chksum'.
Signed-off-by: Chao Yu <[email protected]>
---
v3:
- remove incorrect duplicated definition in f2fs_decompress_pages().
Documentation/filesystems/f2fs.rst | 1 +
fs/f2fs/compress.c | 23 +++++++++++++++++++++++
fs/f2fs/f2fs.h | 16 ++++++++++++++--
fs/f2fs/inode.c | 3 +++
fs/f2fs/super.c | 9 +++++++++
include/linux/f2fs_fs.h | 2 +-
6 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index b8ee761c9922..985ae7d35066 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -260,6 +260,7 @@ compress_extension=%s Support adding specified extension, so that f2fs can enab
For other files, we can still enable compression via ioctl.
Note that, there is one reserved special extension '*', it
can be set to enable compression for all files.
+compress_chksum Support verifying chksum of raw data in compressed cluster.
inlinecrypt When possible, encrypt/decrypt the contents of encrypted
files using the blk-crypto framework rather than
filesystem-layer encryption. This allows the use of
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 3957a84a185e..487c4280a7b8 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -607,6 +607,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
f2fs_cops[fi->i_compress_algorithm];
unsigned int max_len, new_nr_cpages;
struct page **new_cpages;
+ u32 chksum = 0;
int i, ret;
trace_f2fs_compress_pages_start(cc->inode, cc->cluster_idx,
@@ -660,6 +661,11 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
cc->cbuf->clen = cpu_to_le32(cc->clen);
+ if (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)
+ chksum = f2fs_crc32(F2FS_I_SB(cc->inode),
+ cc->cbuf->cdata, cc->clen);
+ cc->cbuf->chksum = cpu_to_le32(chksum);
+
for (i = 0; i < COMPRESS_DATA_RESERVED_SIZE; i++)
cc->cbuf->reserved[i] = cpu_to_le32(0);
@@ -795,6 +801,23 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
ret = cops->decompress_pages(dic);
+ if (!ret && fi->i_compress_flag & 1 << COMPRESS_CHKSUM) {
+ u32 provided = le32_to_cpu(dic->cbuf->chksum);
+ u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
+
+ if (provided != calculated) {
+ if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
+ set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
+ printk_ratelimited(
+ "%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
+ KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
+ provided, calculated);
+ }
+ set_sbi_flag(sbi, SBI_NEED_FSCK);
+ WARN_ON_ONCE(1);
+ }
+ }
+
out_vunmap_cbuf:
vm_unmap_ram(dic->cbuf, dic->nr_cpages);
out_vunmap_rbuf:
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 0d38f2135016..7c2e7e4738e5 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -147,7 +147,8 @@ struct f2fs_mount_info {
/* For compression */
unsigned char compress_algorithm; /* algorithm type */
- unsigned compress_log_size; /* cluster log size */
+ unsigned char compress_log_size; /* cluster log size */
+ bool compress_chksum; /* compressed data chksum */
unsigned char compress_ext_cnt; /* extension count */
unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
};
@@ -674,6 +675,7 @@ enum {
FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
FI_VERITY_IN_PROGRESS, /* building fs-verity Merkle tree */
FI_COMPRESSED_FILE, /* indicate file's data can be compressed */
+ FI_COMPRESS_CORRUPT, /* indicate compressed cluster is corrupted */
FI_MMAP_FILE, /* indicate file was mmapped */
FI_MAX, /* max flag, never be used */
};
@@ -731,6 +733,7 @@ struct f2fs_inode_info {
atomic_t i_compr_blocks; /* # of compressed blocks */
unsigned char i_compress_algorithm; /* algorithm type */
unsigned char i_log_cluster_size; /* log of cluster size */
+ unsigned short i_compress_flag; /* compress flag */
unsigned int i_cluster_size; /* cluster size */
};
@@ -1270,9 +1273,15 @@ enum compress_algorithm_type {
COMPRESS_MAX,
};
-#define COMPRESS_DATA_RESERVED_SIZE 5
+enum compress_flag {
+ COMPRESS_CHKSUM,
+ COMPRESS_MAX_FLAG,
+};
+
+#define COMPRESS_DATA_RESERVED_SIZE 4
struct compress_data {
__le32 clen; /* compressed data size */
+ __le32 chksum; /* compressed data chksum */
__le32 reserved[COMPRESS_DATA_RESERVED_SIZE]; /* reserved */
u8 cdata[]; /* compressed data */
};
@@ -3887,6 +3896,9 @@ static inline void set_compress_context(struct inode *inode)
F2FS_OPTION(sbi).compress_algorithm;
F2FS_I(inode)->i_log_cluster_size =
F2FS_OPTION(sbi).compress_log_size;
+ F2FS_I(inode)->i_compress_flag =
+ F2FS_OPTION(sbi).compress_chksum ?
+ 1 << COMPRESS_CHKSUM : 0;
F2FS_I(inode)->i_cluster_size =
1 << F2FS_I(inode)->i_log_cluster_size;
F2FS_I(inode)->i_flags |= F2FS_COMPR_FL;
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 657db2fb6739..349d9cb933ee 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -456,6 +456,7 @@ static int do_read_inode(struct inode *inode)
le64_to_cpu(ri->i_compr_blocks));
fi->i_compress_algorithm = ri->i_compress_algorithm;
fi->i_log_cluster_size = ri->i_log_cluster_size;
+ fi->i_compress_flag = le16_to_cpu(ri->i_compress_flag);
fi->i_cluster_size = 1 << fi->i_log_cluster_size;
set_inode_flag(inode, FI_COMPRESSED_FILE);
}
@@ -634,6 +635,8 @@ void f2fs_update_inode(struct inode *inode, struct page *node_page)
&F2FS_I(inode)->i_compr_blocks));
ri->i_compress_algorithm =
F2FS_I(inode)->i_compress_algorithm;
+ ri->i_compress_flag =
+ cpu_to_le16(F2FS_I(inode)->i_compress_flag);
ri->i_log_cluster_size =
F2FS_I(inode)->i_log_cluster_size;
}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 0ec292d7fcdb..78714bae1c48 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -146,6 +146,7 @@ enum {
Opt_compress_algorithm,
Opt_compress_log_size,
Opt_compress_extension,
+ Opt_compress_chksum,
Opt_atgc,
Opt_err,
};
@@ -214,6 +215,7 @@ static match_table_t f2fs_tokens = {
{Opt_compress_algorithm, "compress_algorithm=%s"},
{Opt_compress_log_size, "compress_log_size=%u"},
{Opt_compress_extension, "compress_extension=%s"},
+ {Opt_compress_chksum, "compress_chksum"},
{Opt_atgc, "atgc"},
{Opt_err, NULL},
};
@@ -934,10 +936,14 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
F2FS_OPTION(sbi).compress_ext_cnt++;
kfree(name);
break;
+ case Opt_compress_chksum:
+ F2FS_OPTION(sbi).compress_chksum = true;
+ break;
#else
case Opt_compress_algorithm:
case Opt_compress_log_size:
case Opt_compress_extension:
+ case Opt_compress_chksum:
f2fs_info(sbi, "compression options not supported");
break;
#endif
@@ -1523,6 +1529,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
seq_printf(seq, ",compress_extension=%s",
F2FS_OPTION(sbi).extensions[i]);
}
+
+ if (F2FS_OPTION(sbi).compress_chksum)
+ seq_puts(seq, ",compress_chksum");
}
static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index a5dbb57a687f..7dc2a06cf19a 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -273,7 +273,7 @@ struct f2fs_inode {
__le64 i_compr_blocks; /* # of compressed blocks */
__u8 i_compress_algorithm; /* compress algorithm */
__u8 i_log_cluster_size; /* log of cluster size */
- __le16 i_padding; /* padding */
+ __le16 i_compress_flag; /* compress flag */
__le32 i_extra_end[0]; /* for attribute size calculation */
} __packed;
__le32 i_addr[DEF_ADDRS_PER_INODE]; /* Pointers to data blocks */
--
2.26.2
On Thu, Nov 26, 2020 at 06:32:09PM +0800, Chao Yu wrote:
> + if (!ret && fi->i_compress_flag & 1 << COMPRESS_CHKSUM) {
This really could use some parentheses. People shouldn't have to look up a
C operator precedence table to understand the code.
> + u32 provided = le32_to_cpu(dic->cbuf->chksum);
> + u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
> +
> + if (provided != calculated) {
> + if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
> + set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
> + printk_ratelimited(
> + "%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
> + KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
> + provided, calculated);
> + }
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + WARN_ON_ONCE(1);
WARN, WARN_ON_ONCE, BUG, BUG_ON, etc. are only for kernel bugs, not for invalid
inputs from disk or userspace.
There is already a log message printed just above, so it seems this WARN_ON_ONCE
should just be removed.
- Eric
On Mon, 2020-12-07 at 12:37 -0800, Eric Biggers wrote:
> On Thu, Nov 26, 2020 at 06:32:09PM +0800, Chao Yu wrote:
> > + if (!ret && fi->i_compress_flag & 1 << COMPRESS_CHKSUM) {
>
> This really could use some parentheses. People shouldn't have to look up a
> C operator precedence table to understand the code.
>
> > + u32 provided = le32_to_cpu(dic->cbuf->chksum);
> > + u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
> > +
> > + if (provided != calculated) {
> > + if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
> > + set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
> > + printk_ratelimited(
> > + "%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
> > + KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
> > + provided, calculated);
> > + }
> > + set_sbi_flag(sbi, SBI_NEED_FSCK);
> > + WARN_ON_ONCE(1);
>
> WARN, WARN_ON_ONCE, BUG, BUG_ON, etc. are only for kernel bugs, not for invalid
> inputs from disk or userspace.
>
> There is already a log message printed just above, so it seems this WARN_ON_ONCE
> should just be removed.
And this should probably be
pr_info_ratelimited("F2FS-fs etc...);
with a terminating newline in the format too.
With the current -next, maybe adding new f2fs_<level>_ratelimited macros
would make more sense.
The logging macro definitions are moved to allow the f2fs_<level>_ratelimited
to work for the one use in f2fs_show_injection_info.
This also adds some missing newline terminations to formats.
---
fs/f2fs/compress.c | 79 +++++++++++++++++++++++++-----------------------------
fs/f2fs/dir.c | 7 +++--
fs/f2fs/f2fs.h | 60 ++++++++++++++++++++++++++++-------------
fs/f2fs/segment.c | 6 ++---
4 files changed, 83 insertions(+), 69 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 08987923513d..587dae6c0947 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -211,8 +211,8 @@ static int lzo_compress_pages(struct compress_ctx *cc)
ret = lzo1x_1_compress(cc->rbuf, cc->rlen, cc->cbuf->cdata,
&cc->clen, cc->private);
if (ret != LZO_E_OK) {
- printk_ratelimited("%sF2FS-fs (%s): lzo compress failed, ret:%d\n",
- KERN_ERR, F2FS_I_SB(cc->inode)->sb->s_id, ret);
+ f2fs_err_ratelimited(F2FS_I_SB(cc->inode),
+ "lzo compress failed, ret:%d\n", ret);
return -EIO;
}
return 0;
@@ -225,17 +225,16 @@ static int lzo_decompress_pages(struct decompress_io_ctx *dic)
ret = lzo1x_decompress_safe(dic->cbuf->cdata, dic->clen,
dic->rbuf, &dic->rlen);
if (ret != LZO_E_OK) {
- printk_ratelimited("%sF2FS-fs (%s): lzo decompress failed, ret:%d\n",
- KERN_ERR, F2FS_I_SB(dic->inode)->sb->s_id, ret);
+ f2fs_err_ratelimited(F2FS_I_SB(dic->inode),
+ "lzo decompress failed, ret:%d\n", ret);
return -EIO;
}
if (dic->rlen != PAGE_SIZE << dic->log_cluster_size) {
- printk_ratelimited("%sF2FS-fs (%s): lzo invalid rlen:%zu, "
- "expected:%lu\n", KERN_ERR,
- F2FS_I_SB(dic->inode)->sb->s_id,
- dic->rlen,
- PAGE_SIZE << dic->log_cluster_size);
+ f2fs_err_ratelimited(F2FS_I_SB(dic->inode),
+ "lzo invalid rlen:%zu, expected:%lu\n",
+ dic->rlen,
+ PAGE_SIZE << dic->log_cluster_size);
return -EIO;
}
return 0;
@@ -292,17 +291,16 @@ static int lz4_decompress_pages(struct decompress_io_ctx *dic)
ret = LZ4_decompress_safe(dic->cbuf->cdata, dic->rbuf,
dic->clen, dic->rlen);
if (ret < 0) {
- printk_ratelimited("%sF2FS-fs (%s): lz4 decompress failed, ret:%d\n",
- KERN_ERR, F2FS_I_SB(dic->inode)->sb->s_id, ret);
+ f2fs_err_ratelimited(F2FS_I_SB(dic->inode),
+ "lz4 decompress failed, ret:%d\n", ret);
return -EIO;
}
if (ret != PAGE_SIZE << dic->log_cluster_size) {
- printk_ratelimited("%sF2FS-fs (%s): lz4 invalid rlen:%zu, "
- "expected:%lu\n", KERN_ERR,
- F2FS_I_SB(dic->inode)->sb->s_id,
- dic->rlen,
- PAGE_SIZE << dic->log_cluster_size);
+ f2fs_err_ratelimited(F2FS_I_SB(dic->inode),
+ "lz4 invalid rlen:%zu, expected:%lu\n",
+ dic->rlen,
+ PAGE_SIZE << dic->log_cluster_size);
return -EIO;
}
return 0;
@@ -336,9 +334,8 @@ static int zstd_init_compress_ctx(struct compress_ctx *cc)
stream = ZSTD_initCStream(params, 0, workspace, workspace_size);
if (!stream) {
- printk_ratelimited("%sF2FS-fs (%s): %s ZSTD_initCStream failed\n",
- KERN_ERR, F2FS_I_SB(cc->inode)->sb->s_id,
- __func__);
+ f2fs_err_ratelimited(F2FS_I_SB(cc->inode),
+ "%s ZSTD_initCStream failed\n", __func__);
kvfree(workspace);
return -EIO;
}
@@ -376,17 +373,17 @@ static int zstd_compress_pages(struct compress_ctx *cc)
ret = ZSTD_compressStream(stream, &outbuf, &inbuf);
if (ZSTD_isError(ret)) {
- printk_ratelimited("%sF2FS-fs (%s): %s ZSTD_compressStream failed, ret: %d\n",
- KERN_ERR, F2FS_I_SB(cc->inode)->sb->s_id,
- __func__, ZSTD_getErrorCode(ret));
+ f2fs_err_ratelimited(F2FS_I_SB(cc->inode),
+ "%s ZSTD_compressStream failed, ret: %d\n",
+ __func__, ZSTD_getErrorCode(ret));
return -EIO;
}
ret = ZSTD_endStream(stream, &outbuf);
if (ZSTD_isError(ret)) {
- printk_ratelimited("%sF2FS-fs (%s): %s ZSTD_endStream returned %d\n",
- KERN_ERR, F2FS_I_SB(cc->inode)->sb->s_id,
- __func__, ZSTD_getErrorCode(ret));
+ f2fs_err_ratelimited(F2FS_I_SB(cc->inode),
+ "%s ZSTD_endStream returned %d\n",
+ __func__, ZSTD_getErrorCode(ret));
return -EIO;
}
@@ -418,9 +415,8 @@ static int zstd_init_decompress_ctx(struct decompress_io_ctx *dic)
stream = ZSTD_initDStream(max_window_size, workspace, workspace_size);
if (!stream) {
- printk_ratelimited("%sF2FS-fs (%s): %s ZSTD_initDStream failed\n",
- KERN_ERR, F2FS_I_SB(dic->inode)->sb->s_id,
- __func__);
+ f2fs_err_ratelimited(F2FS_I_SB(dic->inode),
+ "%s ZSTD_initDStream failed\n", __func__);
kvfree(workspace);
return -EIO;
}
@@ -455,18 +451,17 @@ static int zstd_decompress_pages(struct decompress_io_ctx *dic)
ret = ZSTD_decompressStream(stream, &outbuf, &inbuf);
if (ZSTD_isError(ret)) {
- printk_ratelimited("%sF2FS-fs (%s): %s ZSTD_compressStream failed, ret: %d\n",
- KERN_ERR, F2FS_I_SB(dic->inode)->sb->s_id,
- __func__, ZSTD_getErrorCode(ret));
+ f2fs_err_ratelimited(F2FS_I_SB(dic->inode),
+ "%s ZSTD_compressStream failed, ret: %d\n",
+ __func__, ZSTD_getErrorCode(ret));
return -EIO;
}
if (dic->rlen != outbuf.pos) {
- printk_ratelimited("%sF2FS-fs (%s): %s ZSTD invalid rlen:%zu, "
- "expected:%lu\n", KERN_ERR,
- F2FS_I_SB(dic->inode)->sb->s_id,
- __func__, dic->rlen,
- PAGE_SIZE << dic->log_cluster_size);
+ f2fs_err_ratelimited(F2FS_I_SB(dic->inode),
+ "%s ZSTD invalid rlen:%zu, expected:%lu\n",
+ __func__, dic->rlen,
+ PAGE_SIZE << dic->log_cluster_size);
return -EIO;
}
@@ -492,8 +487,8 @@ static int lzorle_compress_pages(struct compress_ctx *cc)
ret = lzorle1x_1_compress(cc->rbuf, cc->rlen, cc->cbuf->cdata,
&cc->clen, cc->private);
if (ret != LZO_E_OK) {
- printk_ratelimited("%sF2FS-fs (%s): lzo-rle compress failed, ret:%d\n",
- KERN_ERR, F2FS_I_SB(cc->inode)->sb->s_id, ret);
+ f2fs_err_ratelimited(F2FS_I_SB(cc->inode),
+ "lzo-rle compress failed, ret:%d\n", ret);
return -EIO;
}
return 0;
@@ -808,10 +803,10 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
if (provided != calculated) {
if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
- printk_ratelimited(
- "%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
- KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
- provided, calculated);
+ f2fs_info_ratelimited(sbi,
+ "checksum invalid, nid = %lu, %x vs %x\n",
+ dic->inode->i_ino,
+ provided, calculated);
}
set_sbi_flag(sbi, SBI_NEED_FSCK);
WARN_ON_ONCE(1);
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 82b58d1f80eb..184989dfc8a5 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -1008,10 +1008,9 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct f2fs_dentry_ptr *d,
if (de->name_len == 0) {
bit_pos++;
ctx->pos = start_pos + bit_pos;
- printk_ratelimited(
- "%sF2FS-fs (%s): invalid namelen(0), ino:%u, run fsck to fix.",
- KERN_WARNING, sbi->sb->s_id,
- le32_to_cpu(de->ino));
+ f2fs_warn_ratelimited(sbi,
+ "invalid namelen(0), ino:%u, run fsck to fix\n",
+ le32_to_cpu(de->ino));
set_sbi_flag(sbi, SBI_NEED_FSCK);
continue;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5cd1b9f7cc53..c6cff897f886 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1573,12 +1573,48 @@ struct f2fs_private_dio {
bool write;
};
+__printf(2, 3)
+void f2fs_printk(struct f2fs_sb_info *sbi, const char *fmt, ...);
+
+#define f2fs_err(sbi, fmt, ...) \
+ f2fs_printk(sbi, KERN_ERR fmt, ##__VA_ARGS__)
+#define f2fs_warn(sbi, fmt, ...) \
+ f2fs_printk(sbi, KERN_WARNING fmt, ##__VA_ARGS__)
+#define f2fs_notice(sbi, fmt, ...) \
+ f2fs_printk(sbi, KERN_NOTICE fmt, ##__VA_ARGS__)
+#define f2fs_info(sbi, fmt, ...) \
+ f2fs_printk(sbi, KERN_INFO fmt, ##__VA_ARGS__)
+#define f2fs_debug(sbi, fmt, ...) \
+ f2fs_printk(sbi, KERN_DEBUG fmt, ##__VA_ARGS__)
+
+/* Ratelimited variants of the above logging uses*/
+
+#define f2fs_printk_ratelimited(sbi, fmt, ...) \
+({ \
+ static DEFINE_RATELIMIT_STATE(_rs, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ \
+ if (__ratelimit(&_rs)) \
+ f2fs_printk(sbi, fmt, ##__VA_ARGS__); \
+})
+
+#define f2fs_err_ratelimited(sbi, fmt, ...) \
+ f2fs_printk_ratelimited(sbi, KERN_ERR fmt, ##__VA_ARGS__)
+#define f2fs_warn_ratelimited(sbi, fmt, ...) \
+ f2fs_printk_ratelimited(sbi, KERN_WARNING fmt, ##__VA_ARGS__)
+#define f2fs_notice_ratelimited(sbi, fmt, ...) \
+ f2fs_printk_ratelimited(sbi, KERN_NOTICE fmt, ##__VA_ARGS__)
+#define f2fs_info_ratelimited(sbi, fmt, ...) \
+ f2fs_printk_ratelimited(sbi, KERN_INFO fmt, ##__VA_ARGS__)
+#define f2fs_debug_ratelimited(sbi, fmt, ...) \
+ f2fs_printk_ratelimited(sbi, KERN_DEBUG fmt, ##__VA_ARGS__)
+
#ifdef CONFIG_F2FS_FAULT_INJECTION
-#define f2fs_show_injection_info(sbi, type) \
- printk_ratelimited("%sF2FS-fs (%s) : inject %s in %s of %pS\n", \
- KERN_INFO, sbi->sb->s_id, \
- f2fs_fault_name[type], \
- __func__, __builtin_return_address(0))
+#define f2fs_show_injection_info(sbi, type) \
+ f2fs_info_ratelimited(sbi, "inject %s in %s of %pS\n", \
+ f2fs_fault_name[type], \
+ __func__, __builtin_return_address(0))
static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
{
struct f2fs_fault_info *ffi = &F2FS_OPTION(sbi).fault_info;
@@ -2027,20 +2063,6 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
return -ENOSPC;
}
-__printf(2, 3)
-void f2fs_printk(struct f2fs_sb_info *sbi, const char *fmt, ...);
-
-#define f2fs_err(sbi, fmt, ...) \
- f2fs_printk(sbi, KERN_ERR fmt, ##__VA_ARGS__)
-#define f2fs_warn(sbi, fmt, ...) \
- f2fs_printk(sbi, KERN_WARNING fmt, ##__VA_ARGS__)
-#define f2fs_notice(sbi, fmt, ...) \
- f2fs_printk(sbi, KERN_NOTICE fmt, ##__VA_ARGS__)
-#define f2fs_info(sbi, fmt, ...) \
- f2fs_printk(sbi, KERN_INFO fmt, ##__VA_ARGS__)
-#define f2fs_debug(sbi, fmt, ...) \
- f2fs_printk(sbi, KERN_DEBUG fmt, ##__VA_ARGS__)
-
static inline void dec_valid_block_count(struct f2fs_sb_info *sbi,
struct inode *inode,
block_t count)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index deca74cb17df..cf500ce90b95 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1057,10 +1057,8 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
dc->error = 0;
if (dc->error)
- printk_ratelimited(
- "%sF2FS-fs (%s): Issue discard(%u, %u, %u) failed, ret: %d",
- KERN_INFO, sbi->sb->s_id,
- dc->lstart, dc->start, dc->len, dc->error);
+ f2fs_info_ratelimited(sbi, "Issue discard(%u, %u, %u) failed, ret: %d\n",
+ dc->lstart, dc->start, dc->len, dc->error);
__detach_discard_cmd(dcc, dc);
}
On 2020/12/8 4:37, Eric Biggers wrote:
> On Thu, Nov 26, 2020 at 06:32:09PM +0800, Chao Yu wrote:
>> + if (!ret && fi->i_compress_flag & 1 << COMPRESS_CHKSUM) {
>
> This really could use some parentheses. People shouldn't have to look up a
> C operator precedence table to understand the code.
Will add parentheses to avoid misread.
>
>> + u32 provided = le32_to_cpu(dic->cbuf->chksum);
>> + u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
>> +
>> + if (provided != calculated) {
>> + if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
>> + set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
>> + printk_ratelimited(
>> + "%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
>> + KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
>> + provided, calculated);
>> + }
>> + set_sbi_flag(sbi, SBI_NEED_FSCK);
>> + WARN_ON_ONCE(1);
>
> WARN, WARN_ON_ONCE, BUG, BUG_ON, etc. are only for kernel bugs, not for invalid
> inputs from disk or userspace.
>
> There is already a log message printed just above, so it seems this WARN_ON_ONCE
> should just be removed.
Jaegeuk wants to give WARN_ON and marking a FSCK flag without returning EFSCORRUPTED,
Jaegeuk, thoughts?
Thanks,
>
> - Eric
> .
>