* [f2fs-dev] [PATCH 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem
2023-12-20 13:59 [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Chao Yu
@ 2023-12-20 13:59 ` Chao Yu
2023-12-20 13:59 ` [f2fs-dev] [PATCH 3/6] f2fs: compress: fix to check unreleased compressed cluster Chao Yu
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2023-12-20 13:59 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
When we overwrite compressed cluster w/ normal cluster, we should
not unlock cp_rwsem during f2fs_write_raw_pages(), otherwise data
will be corrupted if partial blocks were persisted before CP & SPOR,
due to cluster metadata wasn't updated atomically.
Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/compress.c | 20 ++++++++++++++------
fs/f2fs/data.c | 3 ++-
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 1122db8cc0b0..a1426c3eadcc 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1471,7 +1471,8 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
enum iostat_type io_type)
{
struct address_space *mapping = cc->inode->i_mapping;
- int _submitted, compr_blocks, ret, i;
+ struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
+ int _submitted, compr_blocks, ret = 0, i;
compr_blocks = f2fs_compressed_blocks(cc);
@@ -1486,6 +1487,10 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
if (compr_blocks < 0)
return compr_blocks;
+ /* overwrite compressed cluster w/ normal cluster */
+ if (compr_blocks > 0)
+ f2fs_lock_op(sbi);
+
for (i = 0; i < cc->cluster_size; i++) {
if (!cc->rpages[i])
continue;
@@ -1518,26 +1523,29 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
unlock_page(cc->rpages[i]);
ret = 0;
} else if (ret == -EAGAIN) {
+ ret = 0;
/*
* for quota file, just redirty left pages to
* avoid deadlock caused by cluster update race
* from foreground operation.
*/
if (IS_NOQUOTA(cc->inode))
- return 0;
- ret = 0;
+ goto out;
f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
goto retry_write;
}
- return ret;
+ goto out;
}
*submitted += _submitted;
}
- f2fs_balance_fs(F2FS_M_SB(mapping), true);
+out:
+ if (compr_blocks > 0)
+ f2fs_unlock_op(sbi);
- return 0;
+ f2fs_balance_fs(sbi, true);
+ return ret;
}
int f2fs_write_multi_pages(struct compress_ctx *cc,
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 6c72a6e86ba8..ceed5ac6c66b 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2848,7 +2848,7 @@ int f2fs_write_single_data_page(struct page *page, int *submitted,
.encrypted_page = NULL,
.submitted = 0,
.compr_blocks = compr_blocks,
- .need_lock = LOCK_RETRY,
+ .need_lock = compr_blocks ? LOCK_DONE : LOCK_RETRY,
.post_read = f2fs_post_read_required(inode) ? 1 : 0,
.io_type = io_type,
.io_wbc = wbc,
@@ -2929,6 +2929,7 @@ int f2fs_write_single_data_page(struct page *page, int *submitted,
if (err == -EAGAIN) {
err = f2fs_do_write_data_page(&fio);
if (err == -EAGAIN) {
+ f2fs_bug_on(sbi, compr_blocks);
fio.need_lock = LOCK_REQ;
err = f2fs_do_write_data_page(&fio);
}
--
2.40.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] 10+ messages in thread* [f2fs-dev] [PATCH 3/6] f2fs: compress: fix to check unreleased compressed cluster
2023-12-20 13:59 [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Chao Yu
2023-12-20 13:59 ` [f2fs-dev] [PATCH 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem Chao Yu
@ 2023-12-20 13:59 ` Chao Yu
2023-12-20 13:59 ` [f2fs-dev] [PATCH 4/6] f2fs: compress: fix to avoid inconsistent bewteen i_blocks and dnode Chao Yu
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2023-12-20 13:59 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
From: Sheng Yong <shengyong@oppo.com>
Compressed cluster may not be released due to we can fail in
release_compress_blocks(), fix to handle reserved compressed
cluster correctly in reserve_compress_blocks().
Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Sheng Yong <shengyong@oppo.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/file.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index c5e681fc1d58..c200b4c81baf 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3643,6 +3643,15 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
goto next;
}
+ /*
+ * compressed cluster was not released due to
+ * it fails in release_compress_blocks().
+ */
+ if (blkaddr == NEW_ADDR) {
+ compr_blocks++;
+ continue;
+ }
+
if (__is_valid_data_blkaddr(blkaddr)) {
compr_blocks++;
continue;
@@ -3652,6 +3661,9 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
}
reserved = cluster_size - compr_blocks;
+ if (!reserved)
+ goto next;
+
ret = inc_valid_block_count(sbi, dn->inode, &reserved);
if (ret)
return ret;
--
2.40.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] 10+ messages in thread* [f2fs-dev] [PATCH 4/6] f2fs: compress: fix to avoid inconsistent bewteen i_blocks and dnode
2023-12-20 13:59 [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Chao Yu
2023-12-20 13:59 ` [f2fs-dev] [PATCH 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem Chao Yu
2023-12-20 13:59 ` [f2fs-dev] [PATCH 3/6] f2fs: compress: fix to check unreleased compressed cluster Chao Yu
@ 2023-12-20 13:59 ` Chao Yu
2023-12-20 13:59 ` [f2fs-dev] [PATCH 5/6] f2fs: fix to remove unnecessary f2fs_bug_on() to avoid panic Chao Yu
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2023-12-20 13:59 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
In reserve_compress_blocks(), we update blkaddrs of dnode in prior to
inc_valid_block_count(), it may cause inconsistent status bewteen
i_blocks and blkaddrs once inc_valid_block_count() fails.
To fix this issue, it needs to reverse their invoking order.
Fixes: c75488fb4d82 ("f2fs: introduce F2FS_IOC_RESERVE_COMPRESS_BLOCKS")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/data.c | 5 +++--
fs/f2fs/f2fs.h | 7 ++++++-
fs/f2fs/file.c | 26 ++++++++++++++------------
fs/f2fs/segment.c | 2 +-
4 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ceed5ac6c66b..cc4cb9099db6 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1224,7 +1224,8 @@ int f2fs_reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count)
if (unlikely(is_inode_flag_set(dn->inode, FI_NO_ALLOC)))
return -EPERM;
- if (unlikely((err = inc_valid_block_count(sbi, dn->inode, &count))))
+ err = inc_valid_block_count(sbi, dn->inode, &count, true);
+ if (unlikely(err))
return err;
trace_f2fs_reserve_new_blocks(dn->inode, dn->nid,
@@ -1481,7 +1482,7 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
dn->data_blkaddr = f2fs_data_blkaddr(dn);
if (dn->data_blkaddr == NULL_ADDR) {
- err = inc_valid_block_count(sbi, dn->inode, &count);
+ err = inc_valid_block_count(sbi, dn->inode, &count, true);
if (unlikely(err))
return err;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bcb3940ab5ba..541c52fe2872 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2294,7 +2294,7 @@ static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi,
static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool);
static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
- struct inode *inode, blkcnt_t *count)
+ struct inode *inode, blkcnt_t *count, bool partial)
{
blkcnt_t diff = 0, release = 0;
block_t avail_user_block_count;
@@ -2320,6 +2320,11 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
avail_user_block_count = get_available_block_count(sbi, inode, true);
if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
+ if (!partial) {
+ spin_unlock(&sbi->stat_lock);
+ goto enospc;
+ }
+
diff = sbi->total_valid_block_count - avail_user_block_count;
if (diff > *count)
diff = *count;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index c200b4c81baf..4f3ed627f6a1 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3633,14 +3633,16 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
blkcnt_t reserved;
int ret;
- for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
- blkaddr = f2fs_data_blkaddr(dn);
+ for (i = 0; i < cluster_size; i++) {
+ blkaddr = data_blkaddr(dn->inode, dn->node_page,
+ dn->ofs_in_node + i);
if (i == 0) {
- if (blkaddr == COMPRESS_ADDR)
- continue;
- dn->ofs_in_node += cluster_size;
- goto next;
+ if (blkaddr != COMPRESS_ADDR) {
+ dn->ofs_in_node += cluster_size;
+ goto next;
+ }
+ continue;
}
/*
@@ -3656,20 +3658,20 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
compr_blocks++;
continue;
}
-
- f2fs_set_data_blkaddr(dn, NEW_ADDR);
}
reserved = cluster_size - compr_blocks;
if (!reserved)
goto next;
- ret = inc_valid_block_count(sbi, dn->inode, &reserved);
- if (ret)
+ ret = inc_valid_block_count(sbi, dn->inode, &reserved, false);
+ if (unlikely(ret))
return ret;
- if (reserved != cluster_size - compr_blocks)
- return -ENOSPC;
+ for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
+ if (f2fs_data_blkaddr(dn) == NULL_ADDR)
+ f2fs_set_data_blkaddr(dn, NEW_ADDR);
+ }
f2fs_i_compr_blocks_update(dn->inode, compr_blocks, true);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index bafdd2295957..f95a2e914baf 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -248,7 +248,7 @@ static int __replace_atomic_write_block(struct inode *inode, pgoff_t index,
} else {
blkcnt_t count = 1;
- err = inc_valid_block_count(sbi, inode, &count);
+ err = inc_valid_block_count(sbi, inode, &count, true);
if (err) {
f2fs_put_dnode(&dn);
return err;
--
2.40.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] 10+ messages in thread* [f2fs-dev] [PATCH 5/6] f2fs: fix to remove unnecessary f2fs_bug_on() to avoid panic
2023-12-20 13:59 [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Chao Yu
` (2 preceding siblings ...)
2023-12-20 13:59 ` [f2fs-dev] [PATCH 4/6] f2fs: compress: fix to avoid inconsistent bewteen i_blocks and dnode Chao Yu
@ 2023-12-20 13:59 ` Chao Yu
2023-12-20 13:59 ` [f2fs-dev] [PATCH 6/6] f2fs: introduce FAULT_INCONSISTENCE Chao Yu
2023-12-26 21:02 ` [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Jaegeuk Kim
5 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2023-12-20 13:59 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
verify_blkaddr() will trigger panic once we inject fault into
f2fs_is_valid_blkaddr(), fix to remove this unnecessary f2fs_bug_on().
Fixes: 18792e64c86d ("f2fs: support fault injection for f2fs_is_valid_blkaddr()")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/f2fs.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 541c52fe2872..9487581db08a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3489,11 +3489,9 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
static inline void verify_blkaddr(struct f2fs_sb_info *sbi,
block_t blkaddr, int type)
{
- if (!f2fs_is_valid_blkaddr(sbi, blkaddr, type)) {
+ if (!f2fs_is_valid_blkaddr(sbi, blkaddr, type))
f2fs_err(sbi, "invalid blkaddr: %u, type: %d, run fsck to fix.",
blkaddr, type);
- f2fs_bug_on(sbi, 1);
- }
}
static inline bool __is_valid_data_blkaddr(block_t blkaddr)
--
2.40.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] 10+ messages in thread* [f2fs-dev] [PATCH 6/6] f2fs: introduce FAULT_INCONSISTENCE
2023-12-20 13:59 [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Chao Yu
` (3 preceding siblings ...)
2023-12-20 13:59 ` [f2fs-dev] [PATCH 5/6] f2fs: fix to remove unnecessary f2fs_bug_on() to avoid panic Chao Yu
@ 2023-12-20 13:59 ` Chao Yu
2023-12-26 21:02 ` [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Jaegeuk Kim
5 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2023-12-20 13:59 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
We will encounter below inconsistent status when FAULT_BLKADDR type
fault injection is on.
Info: checkpoint state = d6 : nat_bits crc fsck compacted_summary orphan_inodes sudden-power-off
[ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c100 has i_blocks: 000000c0, but has 191 blocks
[FIX] (fsck_chk_inode_blk:1260) --> [0x1c100] i_blocks=0x000000c0 -> 0xbf
[FIX] (fsck_chk_inode_blk:1269) --> [0x1c100] i_compr_blocks=0x00000026 -> 0x27
[ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1cadb has i_blocks: 0000002f, but has 46 blocks
[FIX] (fsck_chk_inode_blk:1260) --> [0x1cadb] i_blocks=0x0000002f -> 0x2e
[FIX] (fsck_chk_inode_blk:1269) --> [0x1cadb] i_compr_blocks=0x00000011 -> 0x12
[ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c62c has i_blocks: 00000002, but has 1 blocks
[FIX] (fsck_chk_inode_blk:1260) --> [0x1c62c] i_blocks=0x00000002 -> 0x1
After we inject fault into f2fs_is_valid_blkaddr() during truncation,
a) it missed to increase @nr_free or @valid_blocks
b) it can cause in blkaddr leak in truncated dnode
Which may cause inconsistent status.
This patch separates FAULT_INCONSISTENCE from FAULT_BLKADDR, so that
we can:
a) use FAULT_INCONSISTENCE in f2fs_truncate_data_blocks_range() to
simulate inconsistent issue independently,
b) FAULT_BLKADDR fault will not cause any inconsistent status, we can
just use it to check error path handling in kernel side.
Signed-off-by: Chao Yu <chao@kernel.org>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 1 +
Documentation/filesystems/f2fs.rst | 1 +
fs/f2fs/checkpoint.c | 19 +++++++++++++++----
fs/f2fs/f2fs.h | 3 +++
fs/f2fs/file.c | 8 ++++++--
fs/f2fs/super.c | 1 +
6 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 4f1d4e636d67..649aabac16c2 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -708,6 +708,7 @@ Description: Support configuring fault injection type, should be
FAULT_DQUOT_INIT 0x000010000
FAULT_LOCK_OP 0x000020000
FAULT_BLKADDR 0x000040000
+ FAULT_INCONSISTENCE 0x000080000
=================== ===========
What: /sys/fs/f2fs/<disk>/discard_io_aware_gran
diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 5a1686cdd6b4..dfc1e73b17d9 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -209,6 +209,7 @@ fault_type=%d Support configuring fault injection type, should be
FAULT_DQUOT_INIT 0x000010000
FAULT_LOCK_OP 0x000020000
FAULT_BLKADDR 0x000040000
+ FAULT_INCONSISTENCE 0x000080000
=================== ===========
mode=%s Control block allocation mode which supports "adaptive"
and "lfs". In "lfs" mode, there should be no random
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index f3f3e98fd6b0..b59db6c1932c 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -170,12 +170,9 @@ static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, block_t blkaddr,
return exist;
}
-bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
+bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
block_t blkaddr, int type)
{
- if (time_to_inject(sbi, FAULT_BLKADDR))
- return false;
-
switch (type) {
case META_NAT:
break;
@@ -230,6 +227,20 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
return true;
}
+bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
+ block_t blkaddr, int type)
+{
+ if (time_to_inject(sbi, FAULT_BLKADDR))
+ return false;
+ return __f2fs_is_valid_blkaddr(sbi, blkaddr, type);
+}
+
+bool f2fs_is_valid_blkaddr_raw(struct f2fs_sb_info *sbi,
+ block_t blkaddr, int type)
+{
+ return __f2fs_is_valid_blkaddr(sbi, blkaddr, type);
+}
+
/*
* Readahead CP/NAT/SIT/SSA/POR pages
*/
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9487581db08a..49e4de324841 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -61,6 +61,7 @@ enum {
FAULT_DQUOT_INIT,
FAULT_LOCK_OP,
FAULT_BLKADDR,
+ FAULT_INCONSISTENCE,
FAULT_MAX,
};
@@ -3787,6 +3788,8 @@ struct page *f2fs_get_meta_page_retry(struct f2fs_sb_info *sbi, pgoff_t index);
struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi, pgoff_t index);
bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
block_t blkaddr, int type);
+bool f2fs_is_valid_blkaddr_raw(struct f2fs_sb_info *sbi,
+ block_t blkaddr, int type);
int f2fs_ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages,
int type, bool sync);
void f2fs_ra_meta_pages_cond(struct f2fs_sb_info *sbi, pgoff_t index,
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 4f3ed627f6a1..6c4c739d73b3 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -593,9 +593,13 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
f2fs_set_data_blkaddr(dn, NULL_ADDR);
if (__is_valid_data_blkaddr(blkaddr)) {
- if (!f2fs_is_valid_blkaddr(sbi, blkaddr,
- DATA_GENERIC_ENHANCE))
+ if (time_to_inject(sbi, FAULT_INCONSISTENCE))
+ continue;
+ if (!f2fs_is_valid_blkaddr_raw(sbi, blkaddr,
+ DATA_GENERIC_ENHANCE)) {
+ f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
continue;
+ }
if (compressed_cluster)
valid_blocks++;
}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 29b9e6e3eb5f..5659166b3918 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -62,6 +62,7 @@ const char *f2fs_fault_name[FAULT_MAX] = {
[FAULT_DQUOT_INIT] = "dquot initialize",
[FAULT_LOCK_OP] = "lock_op",
[FAULT_BLKADDR] = "invalid blkaddr",
+ [FAULT_INCONSISTENCE] = "inconsistence",
};
void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate,
--
2.40.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] 10+ messages in thread* Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP
2023-12-20 13:59 [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Chao Yu
` (4 preceding siblings ...)
2023-12-20 13:59 ` [f2fs-dev] [PATCH 6/6] f2fs: introduce FAULT_INCONSISTENCE Chao Yu
@ 2023-12-26 21:02 ` Jaegeuk Kim
2023-12-27 1:04 ` Chao Yu
5 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2023-12-26 21:02 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 12/20, Chao Yu wrote:
> If data block in compressed cluster is not persisted with metadata
> during checkpoint, after SPOR, the data may be corrupted, let's
> guarantee to write compressed page by checkpoint.
>
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/compress.c | 3 ++-
> fs/f2fs/data.c | 12 +++++++++---
> fs/f2fs/f2fs.h | 3 ++-
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 5b076329e9bf..1122db8cc0b0 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1442,6 +1442,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
> struct f2fs_sb_info *sbi = bio->bi_private;
> struct compress_io_ctx *cic =
> (struct compress_io_ctx *)page_private(page);
> + enum count_type type = WB_DATA_TYPE(page);
> int i;
>
> if (unlikely(bio->bi_status))
> @@ -1449,7 +1450,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
>
> f2fs_compress_free_page(page);
>
> - dec_page_count(sbi, F2FS_WB_DATA);
> + dec_page_count(sbi, type);
>
> if (atomic_dec_return(&cic->pending_pages))
> return;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index d28c97282e68..6c72a6e86ba8 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
> bioset_exit(&f2fs_bioset);
> }
>
> -static bool __is_cp_guaranteed(struct page *page)
> +bool f2fs_is_cp_guaranteed(struct page *page)
> {
> struct address_space *mapping = page->mapping;
> struct inode *inode;
> @@ -66,7 +66,7 @@ static bool __is_cp_guaranteed(struct page *page)
> return true;
>
> if (f2fs_is_compressed_page(page))
> - return false;
> + return true;
> if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
> page_private_gcing(page))
> return true;
> @@ -1007,6 +1007,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
> struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
> struct page *bio_page;
> + enum count_type type;
>
> f2fs_bug_on(sbi, is_read_io(fio->op));
>
> @@ -1046,7 +1047,12 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> /* set submitted = true as a return value */
> fio->submitted = 1;
>
> - inc_page_count(sbi, WB_DATA_TYPE(bio_page));
> + type = WB_DATA_TYPE(bio_page);
> + /* override count type if page is compressed one */
> + if (fio->compressed_page)
> + type = WB_DATA_TYPE(fio->compressed_page);
Doesn't bio_page already point fio->compressed_page?
> +
> + inc_page_count(sbi, type);
>
> if (io->bio &&
> (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 76e9a8682e38..bcb3940ab5ba 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1092,7 +1092,7 @@ struct f2fs_sm_info {
> * f2fs monitors the number of several block types such as on-writeback,
> * dirty dentry blocks, dirty node blocks, and dirty meta blocks.
> */
> -#define WB_DATA_TYPE(p) (__is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
> +#define WB_DATA_TYPE(p) (f2fs_is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
> enum count_type {
> F2FS_DIRTY_DENTS,
> F2FS_DIRTY_DATA,
> @@ -3824,6 +3824,7 @@ void f2fs_init_ckpt_req_control(struct f2fs_sb_info *sbi);
> */
> int __init f2fs_init_bioset(void);
> void f2fs_destroy_bioset(void);
> +bool f2fs_is_cp_guaranteed(struct page *page);
> int f2fs_init_bio_entry_cache(void);
> void f2fs_destroy_bio_entry_cache(void);
> void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
> --
> 2.40.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] 10+ messages in thread* Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP
2023-12-26 21:02 ` [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP Jaegeuk Kim
@ 2023-12-27 1:04 ` Chao Yu
2023-12-27 22:55 ` Jaegeuk Kim
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2023-12-27 1:04 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2023/12/27 5:02, Jaegeuk Kim wrote:
> On 12/20, Chao Yu wrote:
>> If data block in compressed cluster is not persisted with metadata
>> during checkpoint, after SPOR, the data may be corrupted, let's
>> guarantee to write compressed page by checkpoint.
>>
>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> fs/f2fs/compress.c | 3 ++-
>> fs/f2fs/data.c | 12 +++++++++---
>> fs/f2fs/f2fs.h | 3 ++-
>> 3 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index 5b076329e9bf..1122db8cc0b0 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -1442,6 +1442,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
>> struct f2fs_sb_info *sbi = bio->bi_private;
>> struct compress_io_ctx *cic =
>> (struct compress_io_ctx *)page_private(page);
>> + enum count_type type = WB_DATA_TYPE(page);
>> int i;
>>
>> if (unlikely(bio->bi_status))
>> @@ -1449,7 +1450,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
>>
>> f2fs_compress_free_page(page);
>>
>> - dec_page_count(sbi, F2FS_WB_DATA);
>> + dec_page_count(sbi, type);
>>
>> if (atomic_dec_return(&cic->pending_pages))
>> return;
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index d28c97282e68..6c72a6e86ba8 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
>> bioset_exit(&f2fs_bioset);
>> }
>>
>> -static bool __is_cp_guaranteed(struct page *page)
>> +bool f2fs_is_cp_guaranteed(struct page *page)
>> {
>> struct address_space *mapping = page->mapping;
>> struct inode *inode;
>> @@ -66,7 +66,7 @@ static bool __is_cp_guaranteed(struct page *page)
>> return true;
>>
>> if (f2fs_is_compressed_page(page))
>> - return false;
>> + return true;
>> if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
>> page_private_gcing(page))
>> return true;
>> @@ -1007,6 +1007,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>> enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
>> struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
>> struct page *bio_page;
>> + enum count_type type;
>>
>> f2fs_bug_on(sbi, is_read_io(fio->op));
>>
>> @@ -1046,7 +1047,12 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>> /* set submitted = true as a return value */
>> fio->submitted = 1;
>>
>> - inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>> + type = WB_DATA_TYPE(bio_page);
>> + /* override count type if page is compressed one */
>> + if (fio->compressed_page)
>> + type = WB_DATA_TYPE(fio->compressed_page);
>
> Doesn't bio_page already point fio->compressed_page?
Please check below codes, bio_page will point to fio->encrypted_page if
both software encryption feature and compression feature are on, for this
case, we still need to account F2FS_WB_CP_DATA.
if (fio->encrypted_page)
bio_page = fio->encrypted_page;
else if (fio->compressed_page)
bio_page = fio->compressed_page;
else
bio_page = fio->page;
Thanks,
>
>> +
>> + inc_page_count(sbi, type);
>>
>> if (io->bio &&
>> (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 76e9a8682e38..bcb3940ab5ba 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1092,7 +1092,7 @@ struct f2fs_sm_info {
>> * f2fs monitors the number of several block types such as on-writeback,
>> * dirty dentry blocks, dirty node blocks, and dirty meta blocks.
>> */
>> -#define WB_DATA_TYPE(p) (__is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
>> +#define WB_DATA_TYPE(p) (f2fs_is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
>> enum count_type {
>> F2FS_DIRTY_DENTS,
>> F2FS_DIRTY_DATA,
>> @@ -3824,6 +3824,7 @@ void f2fs_init_ckpt_req_control(struct f2fs_sb_info *sbi);
>> */
>> int __init f2fs_init_bioset(void);
>> void f2fs_destroy_bioset(void);
>> +bool f2fs_is_cp_guaranteed(struct page *page);
>> int f2fs_init_bio_entry_cache(void);
>> void f2fs_destroy_bio_entry_cache(void);
>> void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
>> --
>> 2.40.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] 10+ messages in thread* Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP
2023-12-27 1:04 ` Chao Yu
@ 2023-12-27 22:55 ` Jaegeuk Kim
2023-12-28 2:44 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2023-12-27 22:55 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 12/27, Chao Yu wrote:
> On 2023/12/27 5:02, Jaegeuk Kim wrote:
> > On 12/20, Chao Yu wrote:
> > > If data block in compressed cluster is not persisted with metadata
> > > during checkpoint, after SPOR, the data may be corrupted, let's
> > > guarantee to write compressed page by checkpoint.
> > >
> > > Fixes: 4c8ff7095bef ("f2fs: support data compression")
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > ---
> > > fs/f2fs/compress.c | 3 ++-
> > > fs/f2fs/data.c | 12 +++++++++---
> > > fs/f2fs/f2fs.h | 3 ++-
> > > 3 files changed, 13 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > index 5b076329e9bf..1122db8cc0b0 100644
> > > --- a/fs/f2fs/compress.c
> > > +++ b/fs/f2fs/compress.c
> > > @@ -1442,6 +1442,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
> > > struct f2fs_sb_info *sbi = bio->bi_private;
> > > struct compress_io_ctx *cic =
> > > (struct compress_io_ctx *)page_private(page);
> > > + enum count_type type = WB_DATA_TYPE(page);
> > > int i;
> > > if (unlikely(bio->bi_status))
> > > @@ -1449,7 +1450,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
> > > f2fs_compress_free_page(page);
> > > - dec_page_count(sbi, F2FS_WB_DATA);
> > > + dec_page_count(sbi, type);
> > > if (atomic_dec_return(&cic->pending_pages))
> > > return;
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index d28c97282e68..6c72a6e86ba8 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
> > > bioset_exit(&f2fs_bioset);
> > > }
> > > -static bool __is_cp_guaranteed(struct page *page)
> > > +bool f2fs_is_cp_guaranteed(struct page *page)
> > > {
> > > struct address_space *mapping = page->mapping;
> > > struct inode *inode;
> > > @@ -66,7 +66,7 @@ static bool __is_cp_guaranteed(struct page *page)
> > > return true;
> > > if (f2fs_is_compressed_page(page))
> > > - return false;
> > > + return true;
> > > if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
> > > page_private_gcing(page))
> > > return true;
> > > @@ -1007,6 +1007,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > > enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
> > > struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
> > > struct page *bio_page;
> > > + enum count_type type;
> > > f2fs_bug_on(sbi, is_read_io(fio->op));
> > > @@ -1046,7 +1047,12 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > > /* set submitted = true as a return value */
> > > fio->submitted = 1;
> > > - inc_page_count(sbi, WB_DATA_TYPE(bio_page));
> > > + type = WB_DATA_TYPE(bio_page);
> > > + /* override count type if page is compressed one */
> > > + if (fio->compressed_page)
> > > + type = WB_DATA_TYPE(fio->compressed_page);
> >
> > Doesn't bio_page already point fio->compressed_page?
>
> Please check below codes, bio_page will point to fio->encrypted_page if
> both software encryption feature and compression feature are on, for this
> case, we still need to account F2FS_WB_CP_DATA.
So, it seems you want to make F2FS_WB_CP_DATA regardless of conditions. Then,
how about making this explictly instead of implicit condition check of the page?
#define WB_DATA_TYPE(p, f) (f || __is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
inc_page_count(sbi, WB_DATA_TYPE(bio_page, bio_page == fio->compressed_page));
dec_page_count(sbi, WB_DATA_TYPE(page, f2fs_is_compressed_page(page)));
>
> if (fio->encrypted_page)
> bio_page = fio->encrypted_page;
> else if (fio->compressed_page)
> bio_page = fio->compressed_page;
> else
> bio_page = fio->page;
>
> Thanks,
>
> >
> > > +
> > > + inc_page_count(sbi, type);
> > > if (io->bio &&
> > > (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 76e9a8682e38..bcb3940ab5ba 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -1092,7 +1092,7 @@ struct f2fs_sm_info {
> > > * f2fs monitors the number of several block types such as on-writeback,
> > > * dirty dentry blocks, dirty node blocks, and dirty meta blocks.
> > > */
> > > -#define WB_DATA_TYPE(p) (__is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
> > > +#define WB_DATA_TYPE(p) (f2fs_is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
> > > enum count_type {
> > > F2FS_DIRTY_DENTS,
> > > F2FS_DIRTY_DATA,
> > > @@ -3824,6 +3824,7 @@ void f2fs_init_ckpt_req_control(struct f2fs_sb_info *sbi);
> > > */
> > > int __init f2fs_init_bioset(void);
> > > void f2fs_destroy_bioset(void);
> > > +bool f2fs_is_cp_guaranteed(struct page *page);
> > > int f2fs_init_bio_entry_cache(void);
> > > void f2fs_destroy_bio_entry_cache(void);
> > > void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
> > > --
> > > 2.40.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] 10+ messages in thread* Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP
2023-12-27 22:55 ` Jaegeuk Kim
@ 2023-12-28 2:44 ` Chao Yu
0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2023-12-28 2:44 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2023/12/28 6:55, Jaegeuk Kim wrote:
> On 12/27, Chao Yu wrote:
>> On 2023/12/27 5:02, Jaegeuk Kim wrote:
>>> On 12/20, Chao Yu wrote:
>>>> If data block in compressed cluster is not persisted with metadata
>>>> during checkpoint, after SPOR, the data may be corrupted, let's
>>>> guarantee to write compressed page by checkpoint.
>>>>
>>>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>> fs/f2fs/compress.c | 3 ++-
>>>> fs/f2fs/data.c | 12 +++++++++---
>>>> fs/f2fs/f2fs.h | 3 ++-
>>>> 3 files changed, 13 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>>> index 5b076329e9bf..1122db8cc0b0 100644
>>>> --- a/fs/f2fs/compress.c
>>>> +++ b/fs/f2fs/compress.c
>>>> @@ -1442,6 +1442,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
>>>> struct f2fs_sb_info *sbi = bio->bi_private;
>>>> struct compress_io_ctx *cic =
>>>> (struct compress_io_ctx *)page_private(page);
>>>> + enum count_type type = WB_DATA_TYPE(page);
>>>> int i;
>>>> if (unlikely(bio->bi_status))
>>>> @@ -1449,7 +1450,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
>>>> f2fs_compress_free_page(page);
>>>> - dec_page_count(sbi, F2FS_WB_DATA);
>>>> + dec_page_count(sbi, type);
>>>> if (atomic_dec_return(&cic->pending_pages))
>>>> return;
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index d28c97282e68..6c72a6e86ba8 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
>>>> bioset_exit(&f2fs_bioset);
>>>> }
>>>> -static bool __is_cp_guaranteed(struct page *page)
>>>> +bool f2fs_is_cp_guaranteed(struct page *page)
>>>> {
>>>> struct address_space *mapping = page->mapping;
>>>> struct inode *inode;
>>>> @@ -66,7 +66,7 @@ static bool __is_cp_guaranteed(struct page *page)
>>>> return true;
>>>> if (f2fs_is_compressed_page(page))
>>>> - return false;
>>>> + return true;
>>>> if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
>>>> page_private_gcing(page))
>>>> return true;
>>>> @@ -1007,6 +1007,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>> enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
>>>> struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
>>>> struct page *bio_page;
>>>> + enum count_type type;
>>>> f2fs_bug_on(sbi, is_read_io(fio->op));
>>>> @@ -1046,7 +1047,12 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>> /* set submitted = true as a return value */
>>>> fio->submitted = 1;
>>>> - inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>>>> + type = WB_DATA_TYPE(bio_page);
>>>> + /* override count type if page is compressed one */
>>>> + if (fio->compressed_page)
>>>> + type = WB_DATA_TYPE(fio->compressed_page);
>>>
>>> Doesn't bio_page already point fio->compressed_page?
>>
>> Please check below codes, bio_page will point to fio->encrypted_page if
>> both software encryption feature and compression feature are on, for this
>> case, we still need to account F2FS_WB_CP_DATA.
>
> So, it seems you want to make F2FS_WB_CP_DATA regardless of conditions. Then,
> how about making this explictly instead of implicit condition check of the page?
>
> #define WB_DATA_TYPE(p, f) (f || __is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
>
> inc_page_count(sbi, WB_DATA_TYPE(bio_page, bio_page == fio->compressed_page));
Do you mean inc_page_count(sbi, WB_DATA_TYPE(bio_page, fio->compressed_page));?
If we use inc_page_count(sbi, WB_DATA_TYPE(bio_page, bio_page == fio->compressed_page));
if bio_page points to fio->encrypted_page, but fio->compressed_page is valid, then
WB_DATA_TYPE() will return F2FS_WB_DATA, it doesn't as expect.
Or am I missing something?
Thanks,
>
> dec_page_count(sbi, WB_DATA_TYPE(page, f2fs_is_compressed_page(page)));
>
>>
>> if (fio->encrypted_page)
>> bio_page = fio->encrypted_page;
>> else if (fio->compressed_page)
>> bio_page = fio->compressed_page;
>> else
>> bio_page = fio->page;
>>
>> Thanks,
>>
>>>
>>>> +
>>>> + inc_page_count(sbi, type);
>>>> if (io->bio &&
>>>> (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 76e9a8682e38..bcb3940ab5ba 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -1092,7 +1092,7 @@ struct f2fs_sm_info {
>>>> * f2fs monitors the number of several block types such as on-writeback,
>>>> * dirty dentry blocks, dirty node blocks, and dirty meta blocks.
>>>> */
>>>> -#define WB_DATA_TYPE(p) (__is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
>>>> +#define WB_DATA_TYPE(p) (f2fs_is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
>>>> enum count_type {
>>>> F2FS_DIRTY_DENTS,
>>>> F2FS_DIRTY_DATA,
>>>> @@ -3824,6 +3824,7 @@ void f2fs_init_ckpt_req_control(struct f2fs_sb_info *sbi);
>>>> */
>>>> int __init f2fs_init_bioset(void);
>>>> void f2fs_destroy_bioset(void);
>>>> +bool f2fs_is_cp_guaranteed(struct page *page);
>>>> int f2fs_init_bio_entry_cache(void);
>>>> void f2fs_destroy_bio_entry_cache(void);
>>>> void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
>>>> --
>>>> 2.40.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] 10+ messages in thread