[f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

[f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

[Chao Yu]

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);
+
+	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

[f2fs-dev] [PATCH 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem

[Chao Yu]

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

[f2fs-dev] [PATCH 3/6] f2fs: compress: fix to check unreleased compressed cluster

[Chao Yu]

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

[f2fs-dev] [PATCH 4/6] f2fs: compress: fix to avoid inconsistent bewteen i_blocks and dnode

[Chao Yu]

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

[f2fs-dev] [PATCH 5/6] f2fs: fix to remove unnecessary f2fs_bug_on() to avoid panic

[Chao Yu]

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

[f2fs-dev] [PATCH 6/6] f2fs: introduce FAULT_INCONSISTENCE

[Chao Yu]

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

Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

[Jaegeuk Kim]

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

Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

[Chao Yu]

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

Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

[Jaegeuk Kim]

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

Re: [f2fs-dev] [PATCH 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

[Chao Yu]

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