[PATCH 1/3] f2fs: adjust display format of segment bit

[PATCH 1/3] f2fs: adjust display format of segment bit

[Chao Yu]

Just adjust segment bit info printed in procfs.

Before:
1008      5|0  |0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
1009      3|183|0 0 61 20 20 0 0 21 80 c0 2 e4 e 54 0 21 21 17 a 44 d0 28 e4 50 40 30 8 0 2d 32 0 5 b0 80 1 43 2 8e f8 7b 2 25 93 bf e0 73 8e 9a 19 44 60 ff e4 cc e6 8e bf f9 ff 5 3d 31 3d 13
1010      3|1  |0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 40 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

After:
1008      5|0  | 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
1009      4|434| ff 7d ff bf d9 3f ff e7 ff bf d7 bf ff bb be ff fb df f7 fb fa bf fb fe bb df dd ff fe ef ff fe ef e2 27 bf ab bf fb df fd bd bf fb db fc ff ff 3f ff ff bf ff 5f db 3f fb fb bf fb bf 4f ff ef
1010      4|422| ff bb fe ff ef d7 ee ff ff fc bf ef 7d eb ec fd fb 3f 97 7f ef ff af ff db ff ff 69 bf ff f6 e7 ff fb f7 7b fb df be ff ff ef f3 fe ff ff df fe f7 fa ff b7 77 be fe fb a9 7f 87 a2 ac c7 ff 75

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e7bb153..6426855 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -954,7 +954,7 @@ static int segment_bits_seq_show(struct seq_file *seq, void *offset)
 		seq_printf(seq, "%d|%-3u|", se->type,
 					get_valid_blocks(sbi, i, 1));
 		for (j = 0; j < SIT_VBLOCK_MAP_SIZE; j++)
-			seq_printf(seq, "%x ", se->cur_valid_map[j]);
+			seq_printf(seq, " %.2x", se->cur_valid_map[j]);
 		seq_putc(seq, '\n');
 	}
 	return 0;
-- 
2.8.2.311.gee88674

[PATCH 1/3] f2fs: adjust display format of segment bit

[Chao Yu]

Just adjust segment bit info printed in procfs.

Before:
1008      5|0  |0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
1009      3|183|0 0 61 20 20 0 0 21 80 c0 2 e4 e 54 0 21 21 17 a 44 d0 28 e4 50 40 30 8 0 2d 32 0 5 b0 80 1 43 2 8e f8 7b 2 25 93 bf e0 73 8e 9a 19 44 60 ff e4 cc e6 8e bf f9 ff 5 3d 31 3d 13
1010      3|1  |0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 40 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

After:
1008      5|0  | 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
1009      4|434| ff 7d ff bf d9 3f ff e7 ff bf d7 bf ff bb be ff fb df f7 fb fa bf fb fe bb df dd ff fe ef ff fe ef e2 27 bf ab bf fb df fd bd bf fb db fc ff ff 3f ff ff bf ff 5f db 3f fb fb bf fb bf 4f ff ef
1010      4|422| ff bb fe ff ef d7 ee ff ff fc bf ef 7d eb ec fd fb 3f 97 7f ef ff af ff db ff ff 69 bf ff f6 e7 ff fb f7 7b fb df be ff ff ef f3 fe ff ff df fe f7 fa ff b7 77 be fe fb a9 7f 87 a2 ac c7 ff 75

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e7bb153..6426855 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -954,7 +954,7 @@ static int segment_bits_seq_show(struct seq_file *seq, void *offset)
 		seq_printf(seq, "%d|%-3u|", se->type,
 					get_valid_blocks(sbi, i, 1));
 		for (j = 0; j < SIT_VBLOCK_MAP_SIZE; j++)
-			seq_printf(seq, "%x ", se->cur_valid_map[j]);
+			seq_printf(seq, " %.2x", se->cur_valid_map[j]);
 		seq_putc(seq, '\n');
 	}
 	return 0;
-- 
2.8.2.311.gee88674


------------------------------------------------------------------------------

[PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

This patch adds to support checkpoint error injection in f2fs for testing
fatal error tolerance.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h  | 7 +++++++
 fs/f2fs/super.c | 1 +
 2 files changed, 8 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e216bc0..3c513fe 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -47,6 +47,7 @@ enum {
 	FAULT_DIR_DEPTH,
 	FAULT_EVICT_INODE,
 	FAULT_IO,
+	FAULT_CHECKPOINT,
 	FAULT_MAX,
 };
 
@@ -80,6 +81,8 @@ static inline bool time_to_inject(int type)
 		return false;
 	else if (type == FAULT_IO && !IS_FAULT_SET(type))
 		return false;
+	else if (type == FAULT_CHECKPOINT && !IS_FAULT_SET(type))
+		return false;
 
 	atomic_inc(&f2fs_fault.inject_ops);
 	if (atomic_read(&f2fs_fault.inject_ops) >= f2fs_fault.inject_rate) {
@@ -1873,6 +1876,10 @@ static inline int f2fs_readonly(struct super_block *sb)
 
 static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
 {
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+	if (time_to_inject(FAULT_CHECKPOINT))
+		return true;
+#endif
 	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
 }
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6426855..3c49419 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -51,6 +51,7 @@ char *fault_name[FAULT_MAX] = {
 	[FAULT_DIR_DEPTH]	= "too big dir depth",
 	[FAULT_EVICT_INODE]	= "evict_inode fail",
 	[FAULT_IO]		= "IO error",
+	[FAULT_CHECKPOINT]	= "checkpoint error",
 };
 
 static void f2fs_build_fault_attr(unsigned int rate)
-- 
2.8.2.311.gee88674

[PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

This patch adds to support checkpoint error injection in f2fs for testing
fatal error tolerance.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h  | 7 +++++++
 fs/f2fs/super.c | 1 +
 2 files changed, 8 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e216bc0..3c513fe 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -47,6 +47,7 @@ enum {
 	FAULT_DIR_DEPTH,
 	FAULT_EVICT_INODE,
 	FAULT_IO,
+	FAULT_CHECKPOINT,
 	FAULT_MAX,
 };
 
@@ -80,6 +81,8 @@ static inline bool time_to_inject(int type)
 		return false;
 	else if (type == FAULT_IO && !IS_FAULT_SET(type))
 		return false;
+	else if (type == FAULT_CHECKPOINT && !IS_FAULT_SET(type))
+		return false;
 
 	atomic_inc(&f2fs_fault.inject_ops);
 	if (atomic_read(&f2fs_fault.inject_ops) >= f2fs_fault.inject_rate) {
@@ -1873,6 +1876,10 @@ static inline int f2fs_readonly(struct super_block *sb)
 
 static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
 {
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+	if (time_to_inject(FAULT_CHECKPOINT))
+		return true;
+#endif
 	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
 }
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6426855..3c49419 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -51,6 +51,7 @@ char *fault_name[FAULT_MAX] = {
 	[FAULT_DIR_DEPTH]	= "too big dir depth",
 	[FAULT_EVICT_INODE]	= "evict_inode fail",
 	[FAULT_IO]		= "IO error",
+	[FAULT_CHECKPOINT]	= "checkpoint error",
 };
 
 static void f2fs_build_fault_attr(unsigned int rate)
-- 
2.8.2.311.gee88674


------------------------------------------------------------------------------

[PATCH 3/3] f2fs: fix potential deadlock when hitting checkpoint error

[Chao Yu]

tests/generic/013 of fstest suit complains us with below dmesg when we
trigger checkpoint error injection in f2fs.

F2FS-fs : inject checkpoint error in sync_node_pages+0x69f/0x6f0 [f2fs]
F2FS-fs (zram0): Cannot recover all fsync data errno=-5
INFO: task mount:97685 blocked for more than 120 seconds.
      Tainted: G           OE   4.8.0-rc4 #11
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
mount           D ffff8801c1bf7960     0 97685  97397 0x00080000
 ffff8801c1bf7960 ffff8801c1bf7930 ffff880175900000 ffff8801c1bf7980
 ffff8801c1bf8000 0000000000000000 7fffffffffffffff ffff88021f7be340
 ffffffff817c8880 ffff8801c1bf7978 ffffffff817c80a5 ffff880214f58fc0
Call Trace:
 [<ffffffff817c8880>] ? bit_wait+0x50/0x50
 [<ffffffff817c80a5>] schedule+0x35/0x80
 [<ffffffff817cb152>] schedule_timeout+0x292/0x3d0
 [<ffffffff81022ab5>] ? xen_clocksource_get_cycles+0x15/0x20
 [<ffffffff810eeb5c>] ? ktime_get+0x3c/0xb0
 [<ffffffff817c8880>] ? bit_wait+0x50/0x50
 [<ffffffff817c7906>] io_schedule_timeout+0xa6/0x110
 [<ffffffff817c889b>] bit_wait_io+0x1b/0x60
 [<ffffffff817c84e4>] __wait_on_bit+0x64/0x90
 [<ffffffff8117dcd4>] wait_on_page_bit+0xc4/0xd0
 [<ffffffff810bc4d0>] ? autoremove_wake_function+0x40/0x40
 [<ffffffff81190a29>] truncate_inode_pages_range+0x409/0x840
 [<ffffffff811a406d>] ? pcpu_free_area+0x13d/0x1a0
 [<ffffffff810bc025>] ? wake_up_bit+0x25/0x30
 [<ffffffff81190ecc>] truncate_inode_pages_final+0x4c/0x60
 [<ffffffffa025e9e8>] f2fs_evict_inode+0x48/0x390 [f2fs]
 [<ffffffff812212f7>] evict+0xc7/0x1a0
 [<ffffffff81221f77>] iput+0x197/0x200
 [<ffffffffa0268242>] f2fs_fill_super+0xab2/0x1130 [f2fs]
 [<ffffffff81209454>] mount_bdev+0x184/0x1c0
 [<ffffffffa0267790>] ? f2fs_commit_super+0x100/0x100 [f2fs]
 [<ffffffffa02646a5>] f2fs_mount+0x15/0x20 [f2fs]
 [<ffffffff81209e19>] mount_fs+0x39/0x160
 [<ffffffff81225e47>] vfs_kern_mount+0x67/0x110
 [<ffffffff812283bb>] do_mount+0x1bb/0xc80
 [<ffffffff81229163>] SyS_mount+0x83/0xd0
 [<ffffffff8100391e>] do_syscall_64+0x6e/0x170
 [<ffffffff817cc325>] entry_SYSCALL64_slow_path+0x25/0x25

The reason is that after we commit at least one page into f2fs private
bio cache, if there occurs checkpoint error, we will lose the chance to
commit private bio, result in deadlock in f2fs_evict_inode when wait
that page being writebacked. So giving a chance to do committing in
sync_node_pages for fixing.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/node.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 55c22a9..c2d953e 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1416,6 +1416,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, struct writeback_control *wbc)
 	struct pagevec pvec;
 	int step = 0;
 	int nwritten = 0;
+	int ret = 0;
 
 	pagevec_init(&pvec, 0);
 
@@ -1436,7 +1437,8 @@ next_step:
 
 			if (unlikely(f2fs_cp_error(sbi))) {
 				pagevec_release(&pvec);
-				return -EIO;
+				ret = -EIO;
+				goto out;
 			}
 
 			/*
@@ -1485,9 +1487,11 @@ continue_unlock:
 			set_fsync_mark(page, 0);
 			set_dentry_mark(page, 0);
 
-			if (NODE_MAPPING(sbi)->a_ops->writepage(page, wbc))
+			if (NODE_MAPPING(sbi)->a_ops->writepage(page, wbc)) {
 				unlock_page(page);
-
+			} else {
+				nwritten++;
+			}
 			if (--wbc->nr_to_write == 0)
 				break;
 		}
@@ -1504,7 +1508,10 @@ continue_unlock:
 		step++;
 		goto next_step;
 	}
-	return nwritten;
+out:
+	if (ret && nwritten)
+		f2fs_submit_merged_bio(sbi, NODE, WRITE);
+	return ret;
 }
 
 int wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, nid_t ino)
-- 
2.8.2.311.gee88674

[PATCH 3/3] f2fs: fix potential deadlock when hitting checkpoint error

[Chao Yu]

tests/generic/013 of fstest suit complains us with below dmesg when we
trigger checkpoint error injection in f2fs.

F2FS-fs : inject checkpoint error in sync_node_pages+0x69f/0x6f0 [f2fs]
F2FS-fs (zram0): Cannot recover all fsync data errno=-5
INFO: task mount:97685 blocked for more than 120 seconds.
      Tainted: G           OE   4.8.0-rc4 #11
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
mount           D ffff8801c1bf7960     0 97685  97397 0x00080000
 ffff8801c1bf7960 ffff8801c1bf7930 ffff880175900000 ffff8801c1bf7980
 ffff8801c1bf8000 0000000000000000 7fffffffffffffff ffff88021f7be340
 ffffffff817c8880 ffff8801c1bf7978 ffffffff817c80a5 ffff880214f58fc0
Call Trace:
 [<ffffffff817c8880>] ? bit_wait+0x50/0x50
 [<ffffffff817c80a5>] schedule+0x35/0x80
 [<ffffffff817cb152>] schedule_timeout+0x292/0x3d0
 [<ffffffff81022ab5>] ? xen_clocksource_get_cycles+0x15/0x20
 [<ffffffff810eeb5c>] ? ktime_get+0x3c/0xb0
 [<ffffffff817c8880>] ? bit_wait+0x50/0x50
 [<ffffffff817c7906>] io_schedule_timeout+0xa6/0x110
 [<ffffffff817c889b>] bit_wait_io+0x1b/0x60
 [<ffffffff817c84e4>] __wait_on_bit+0x64/0x90
 [<ffffffff8117dcd4>] wait_on_page_bit+0xc4/0xd0
 [<ffffffff810bc4d0>] ? autoremove_wake_function+0x40/0x40
 [<ffffffff81190a29>] truncate_inode_pages_range+0x409/0x840
 [<ffffffff811a406d>] ? pcpu_free_area+0x13d/0x1a0
 [<ffffffff810bc025>] ? wake_up_bit+0x25/0x30
 [<ffffffff81190ecc>] truncate_inode_pages_final+0x4c/0x60
 [<ffffffffa025e9e8>] f2fs_evict_inode+0x48/0x390 [f2fs]
 [<ffffffff812212f7>] evict+0xc7/0x1a0
 [<ffffffff81221f77>] iput+0x197/0x200
 [<ffffffffa0268242>] f2fs_fill_super+0xab2/0x1130 [f2fs]
 [<ffffffff81209454>] mount_bdev+0x184/0x1c0
 [<ffffffffa0267790>] ? f2fs_commit_super+0x100/0x100 [f2fs]
 [<ffffffffa02646a5>] f2fs_mount+0x15/0x20 [f2fs]
 [<ffffffff81209e19>] mount_fs+0x39/0x160
 [<ffffffff81225e47>] vfs_kern_mount+0x67/0x110
 [<ffffffff812283bb>] do_mount+0x1bb/0xc80
 [<ffffffff81229163>] SyS_mount+0x83/0xd0
 [<ffffffff8100391e>] do_syscall_64+0x6e/0x170
 [<ffffffff817cc325>] entry_SYSCALL64_slow_path+0x25/0x25

The reason is that after we commit at least one page into f2fs private
bio cache, if there occurs checkpoint error, we will lose the chance to
commit private bio, result in deadlock in f2fs_evict_inode when wait
that page being writebacked. So giving a chance to do committing in
sync_node_pages for fixing.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/node.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 55c22a9..c2d953e 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1416,6 +1416,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, struct writeback_control *wbc)
 	struct pagevec pvec;
 	int step = 0;
 	int nwritten = 0;
+	int ret = 0;
 
 	pagevec_init(&pvec, 0);
 
@@ -1436,7 +1437,8 @@ next_step:
 
 			if (unlikely(f2fs_cp_error(sbi))) {
 				pagevec_release(&pvec);
-				return -EIO;
+				ret = -EIO;
+				goto out;
 			}
 
 			/*
@@ -1485,9 +1487,11 @@ continue_unlock:
 			set_fsync_mark(page, 0);
 			set_dentry_mark(page, 0);
 
-			if (NODE_MAPPING(sbi)->a_ops->writepage(page, wbc))
+			if (NODE_MAPPING(sbi)->a_ops->writepage(page, wbc)) {
 				unlock_page(page);
-
+			} else {
+				nwritten++;
+			}
 			if (--wbc->nr_to_write == 0)
 				break;
 		}
@@ -1504,7 +1508,10 @@ continue_unlock:
 		step++;
 		goto next_step;
 	}
-	return nwritten;
+out:
+	if (ret && nwritten)
+		f2fs_submit_merged_bio(sbi, NODE, WRITE);
+	return ret;
 }
 
 int wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, nid_t ino)
-- 
2.8.2.311.gee88674


------------------------------------------------------------------------------

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Jaegeuk Kim]

Hi Chao,

The basic rule is to stop every operations once CP_ERROR_FLAG is set.
But, this patch simply breaks the rule.
For example, f2fs_write_data_page() currently exits with mapping_set_error().
So this patch incurs missing dentry blocks in a valid checkpoint.

Thanks,

On Fri, Sep 23, 2016 at 01:24:57PM +0800, Chao Yu wrote:
> This patch adds to support checkpoint error injection in f2fs for testing
> fatal error tolerance.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/f2fs.h  | 7 +++++++
>  fs/f2fs/super.c | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index e216bc0..3c513fe 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -47,6 +47,7 @@ enum {
>  	FAULT_DIR_DEPTH,
>  	FAULT_EVICT_INODE,
>  	FAULT_IO,
> +	FAULT_CHECKPOINT,
>  	FAULT_MAX,
>  };
>  
> @@ -80,6 +81,8 @@ static inline bool time_to_inject(int type)
>  		return false;
>  	else if (type == FAULT_IO && !IS_FAULT_SET(type))
>  		return false;
> +	else if (type == FAULT_CHECKPOINT && !IS_FAULT_SET(type))
> +		return false;
>  
>  	atomic_inc(&f2fs_fault.inject_ops);
>  	if (atomic_read(&f2fs_fault.inject_ops) >= f2fs_fault.inject_rate) {
> @@ -1873,6 +1876,10 @@ static inline int f2fs_readonly(struct super_block *sb)
>  
>  static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
>  {
> +#ifdef CONFIG_F2FS_FAULT_INJECTION
> +	if (time_to_inject(FAULT_CHECKPOINT))
> +		return true;
> +#endif
>  	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
>  }
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 6426855..3c49419 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -51,6 +51,7 @@ char *fault_name[FAULT_MAX] = {
>  	[FAULT_DIR_DEPTH]	= "too big dir depth",
>  	[FAULT_EVICT_INODE]	= "evict_inode fail",
>  	[FAULT_IO]		= "IO error",
> +	[FAULT_CHECKPOINT]	= "checkpoint error",
>  };
>  
>  static void f2fs_build_fault_attr(unsigned int rate)
> -- 
> 2.8.2.311.gee88674

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

Hi Jaegeuk,

On 2016/9/24 7:53, Jaegeuk Kim wrote:
> Hi Chao,
> 
> The basic rule is to stop every operations once CP_ERROR_FLAG is set.
> But, this patch simply breaks the rule.
> For example, f2fs_write_data_page() currently exits with mapping_set_error().
> So this patch incurs missing dentry blocks in a valid checkpoint.

Yes, that's right.

How about triggering checkpoint error in f2fs_stop_checkpoint?

>From 7bedfe9a0e97c4deead1c7cdbfc24187f5080268 Mon Sep 17 00:00:00 2001
From: Chao Yu <yuchao0@huawei.com>
Date: Fri, 23 Sep 2016 06:59:04 +0800
Subject: [PATCH] f2fs: support checkpoint error injection

This patch adds to support checkpoint error injection in f2fs for testing
fatal error tolerance.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c | 14 +++++++++++---
 fs/f2fs/data.c       |  7 ++++---
 fs/f2fs/f2fs.h       |  5 ++++-
 fs/f2fs/file.c       |  8 ++++----
 fs/f2fs/inode.c      |  7 +++++--
 fs/f2fs/super.c      |  1 +
 6 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index d1560bb..834c8ec 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -26,8 +26,17 @@
 static struct kmem_cache *ino_entry_slab;
 struct kmem_cache *inode_entry_slab;

-void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
+void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi,
+					bool end_io, bool need_stop)
 {
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+	if (time_to_inject(FAULT_CHECKPOINT))
+		need_stop = true;
+#endif
+
+	if (!need_stop)
+		return;
+
 	set_ckpt_flags(sbi, CP_ERROR_FLAG);
 	sbi->sb->s_flags |= MS_RDONLY;
 	if (!end_io)
@@ -100,8 +109,7 @@ repeat:
 	 * readonly and make sure do not write checkpoint with non-uptodate
 	 * meta page.
 	 */
-	if (unlikely(!PageUptodate(page)))
-		f2fs_stop_checkpoint(sbi, false);
+	f2fs_stop_checkpoint(sbi, false, !PageUptodate(page));
 out:
 	return page;
 }
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a9f7436..1b00d3d 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -74,10 +74,11 @@ static void f2fs_write_end_io(struct bio *bio)

 		fscrypt_pullback_bio_page(&page, true);

-		if (unlikely(bio->bi_error)) {
+		f2fs_stop_checkpoint(sbi, true, bio->bi_error);
+
+		if (unlikely(bio->bi_error))
 			set_bit(AS_EIO, &page->mapping->flags);
-			f2fs_stop_checkpoint(sbi, true);
-		}
+
 		end_page_writeback(page);
 	}
 	if (atomic_dec_and_test(&sbi->nr_wb_bios) &&
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e216bc0..7bc1802 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -47,6 +47,7 @@ enum {
 	FAULT_DIR_DEPTH,
 	FAULT_EVICT_INODE,
 	FAULT_IO,
+	FAULT_CHECKPOINT,
 	FAULT_MAX,
 };

@@ -80,6 +81,8 @@ static inline bool time_to_inject(int type)
 		return false;
 	else if (type == FAULT_IO && !IS_FAULT_SET(type))
 		return false;
+	else if (type == FAULT_CHECKPOINT && !IS_FAULT_SET(type))
+		return false;

 	atomic_inc(&f2fs_fault.inject_ops);
 	if (atomic_read(&f2fs_fault.inject_ops) >= f2fs_fault.inject_rate) {
@@ -2115,7 +2118,7 @@ void destroy_segment_manager_caches(void);
 /*
  * checkpoint.c
  */
-void f2fs_stop_checkpoint(struct f2fs_sb_info *, bool);
+void f2fs_stop_checkpoint(struct f2fs_sb_info *, bool, bool);
 struct page *grab_meta_page(struct f2fs_sb_info *, pgoff_t);
 struct page *get_meta_page(struct f2fs_sb_info *, pgoff_t);
 struct page *get_tmp_page(struct f2fs_sb_info *, pgoff_t);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index d341a0e..57c7a64 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1720,21 +1720,21 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 	case F2FS_GOING_DOWN_FULLSYNC:
 		sb = freeze_bdev(sb->s_bdev);
 		if (sb && !IS_ERR(sb)) {
-			f2fs_stop_checkpoint(sbi, false);
+			f2fs_stop_checkpoint(sbi, false, true);
 			thaw_bdev(sb->s_bdev, sb);
 		}
 		break;
 	case F2FS_GOING_DOWN_METASYNC:
 		/* do checkpoint only */
 		f2fs_sync_fs(sb, 1);
-		f2fs_stop_checkpoint(sbi, false);
+		f2fs_stop_checkpoint(sbi, false, true);
 		break;
 	case F2FS_GOING_DOWN_NOSYNC:
-		f2fs_stop_checkpoint(sbi, false);
+		f2fs_stop_checkpoint(sbi, false, true);
 		break;
 	case F2FS_GOING_DOWN_METAFLUSH:
 		sync_meta_pages(sbi, META, LONG_MAX);
-		f2fs_stop_checkpoint(sbi, false);
+		f2fs_stop_checkpoint(sbi, false, true);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index ac4daa5..bf53cd2 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -306,12 +306,15 @@ retry:
 	node_page = get_node_page(sbi, inode->i_ino);
 	if (IS_ERR(node_page)) {
 		int err = PTR_ERR(node_page);
+
+		f2fs_stop_checkpoint(sbi, false,
+				err != -ENOMEM && err != -ENOENT);
+
 		if (err == -ENOMEM) {
 			cond_resched();
 			goto retry;
-		} else if (err != -ENOENT) {
-			f2fs_stop_checkpoint(sbi, false);
 		}
+
 		f2fs_inode_synced(inode);
 		return 0;
 	}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6426855..3c49419 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -51,6 +51,7 @@ char *fault_name[FAULT_MAX] = {
 	[FAULT_DIR_DEPTH]	= "too big dir depth",
 	[FAULT_EVICT_INODE]	= "evict_inode fail",
 	[FAULT_IO]		= "IO error",
+	[FAULT_CHECKPOINT]	= "checkpoint error",
 };

 static void f2fs_build_fault_attr(unsigned int rate)
-- 
2.8.2.311.gee88674

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

Hi Jaegeuk,

On 2016/9/24 7:53, Jaegeuk Kim wrote:
> Hi Chao,
> 
> The basic rule is to stop every operations once CP_ERROR_FLAG is set.
> But, this patch simply breaks the rule.
> For example, f2fs_write_data_page() currently exits with mapping_set_error().
> So this patch incurs missing dentry blocks in a valid checkpoint.

Yes, that's right.

How about triggering checkpoint error in f2fs_stop_checkpoint?

>From 7bedfe9a0e97c4deead1c7cdbfc24187f5080268 Mon Sep 17 00:00:00 2001
From: Chao Yu <yuchao0@huawei.com>
Date: Fri, 23 Sep 2016 06:59:04 +0800
Subject: [PATCH] f2fs: support checkpoint error injection

This patch adds to support checkpoint error injection in f2fs for testing
fatal error tolerance.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c | 14 +++++++++++---
 fs/f2fs/data.c       |  7 ++++---
 fs/f2fs/f2fs.h       |  5 ++++-
 fs/f2fs/file.c       |  8 ++++----
 fs/f2fs/inode.c      |  7 +++++--
 fs/f2fs/super.c      |  1 +
 6 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index d1560bb..834c8ec 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -26,8 +26,17 @@
 static struct kmem_cache *ino_entry_slab;
 struct kmem_cache *inode_entry_slab;

-void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
+void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi,
+					bool end_io, bool need_stop)
 {
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+	if (time_to_inject(FAULT_CHECKPOINT))
+		need_stop = true;
+#endif
+
+	if (!need_stop)
+		return;
+
 	set_ckpt_flags(sbi, CP_ERROR_FLAG);
 	sbi->sb->s_flags |= MS_RDONLY;
 	if (!end_io)
@@ -100,8 +109,7 @@ repeat:
 	 * readonly and make sure do not write checkpoint with non-uptodate
 	 * meta page.
 	 */
-	if (unlikely(!PageUptodate(page)))
-		f2fs_stop_checkpoint(sbi, false);
+	f2fs_stop_checkpoint(sbi, false, !PageUptodate(page));
 out:
 	return page;
 }
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a9f7436..1b00d3d 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -74,10 +74,11 @@ static void f2fs_write_end_io(struct bio *bio)

 		fscrypt_pullback_bio_page(&page, true);

-		if (unlikely(bio->bi_error)) {
+		f2fs_stop_checkpoint(sbi, true, bio->bi_error);
+
+		if (unlikely(bio->bi_error))
 			set_bit(AS_EIO, &page->mapping->flags);
-			f2fs_stop_checkpoint(sbi, true);
-		}
+
 		end_page_writeback(page);
 	}
 	if (atomic_dec_and_test(&sbi->nr_wb_bios) &&
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e216bc0..7bc1802 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -47,6 +47,7 @@ enum {
 	FAULT_DIR_DEPTH,
 	FAULT_EVICT_INODE,
 	FAULT_IO,
+	FAULT_CHECKPOINT,
 	FAULT_MAX,
 };

@@ -80,6 +81,8 @@ static inline bool time_to_inject(int type)
 		return false;
 	else if (type == FAULT_IO && !IS_FAULT_SET(type))
 		return false;
+	else if (type == FAULT_CHECKPOINT && !IS_FAULT_SET(type))
+		return false;

 	atomic_inc(&f2fs_fault.inject_ops);
 	if (atomic_read(&f2fs_fault.inject_ops) >= f2fs_fault.inject_rate) {
@@ -2115,7 +2118,7 @@ void destroy_segment_manager_caches(void);
 /*
  * checkpoint.c
  */
-void f2fs_stop_checkpoint(struct f2fs_sb_info *, bool);
+void f2fs_stop_checkpoint(struct f2fs_sb_info *, bool, bool);
 struct page *grab_meta_page(struct f2fs_sb_info *, pgoff_t);
 struct page *get_meta_page(struct f2fs_sb_info *, pgoff_t);
 struct page *get_tmp_page(struct f2fs_sb_info *, pgoff_t);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index d341a0e..57c7a64 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1720,21 +1720,21 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 	case F2FS_GOING_DOWN_FULLSYNC:
 		sb = freeze_bdev(sb->s_bdev);
 		if (sb && !IS_ERR(sb)) {
-			f2fs_stop_checkpoint(sbi, false);
+			f2fs_stop_checkpoint(sbi, false, true);
 			thaw_bdev(sb->s_bdev, sb);
 		}
 		break;
 	case F2FS_GOING_DOWN_METASYNC:
 		/* do checkpoint only */
 		f2fs_sync_fs(sb, 1);
-		f2fs_stop_checkpoint(sbi, false);
+		f2fs_stop_checkpoint(sbi, false, true);
 		break;
 	case F2FS_GOING_DOWN_NOSYNC:
-		f2fs_stop_checkpoint(sbi, false);
+		f2fs_stop_checkpoint(sbi, false, true);
 		break;
 	case F2FS_GOING_DOWN_METAFLUSH:
 		sync_meta_pages(sbi, META, LONG_MAX);
-		f2fs_stop_checkpoint(sbi, false);
+		f2fs_stop_checkpoint(sbi, false, true);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index ac4daa5..bf53cd2 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -306,12 +306,15 @@ retry:
 	node_page = get_node_page(sbi, inode->i_ino);
 	if (IS_ERR(node_page)) {
 		int err = PTR_ERR(node_page);
+
+		f2fs_stop_checkpoint(sbi, false,
+				err != -ENOMEM && err != -ENOENT);
+
 		if (err == -ENOMEM) {
 			cond_resched();
 			goto retry;
-		} else if (err != -ENOENT) {
-			f2fs_stop_checkpoint(sbi, false);
 		}
+
 		f2fs_inode_synced(inode);
 		return 0;
 	}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6426855..3c49419 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -51,6 +51,7 @@ char *fault_name[FAULT_MAX] = {
 	[FAULT_DIR_DEPTH]	= "too big dir depth",
 	[FAULT_EVICT_INODE]	= "evict_inode fail",
 	[FAULT_IO]		= "IO error",
+	[FAULT_CHECKPOINT]	= "checkpoint error",
 };

 static void f2fs_build_fault_attr(unsigned int rate)
-- 
2.8.2.311.gee88674



------------------------------------------------------------------------------

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Jaegeuk Kim]

On Sat, Sep 24, 2016 at 08:46:54AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2016/9/24 7:53, Jaegeuk Kim wrote:
> > Hi Chao,
> > 
> > The basic rule is to stop every operations once CP_ERROR_FLAG is set.
> > But, this patch simply breaks the rule.
> > For example, f2fs_write_data_page() currently exits with mapping_set_error().
> > So this patch incurs missing dentry blocks in a valid checkpoint.
> 
> Yes, that's right.
>
> How about triggering checkpoint error in f2fs_stop_checkpoint?

Let's just use src/godown in xfstests, since we don't need to trigger this
multiple times in runtime.

> 
> >From 7bedfe9a0e97c4deead1c7cdbfc24187f5080268 Mon Sep 17 00:00:00 2001
> From: Chao Yu <yuchao0@huawei.com>
> Date: Fri, 23 Sep 2016 06:59:04 +0800
> Subject: [PATCH] f2fs: support checkpoint error injection
> 
> This patch adds to support checkpoint error injection in f2fs for testing
> fatal error tolerance.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/checkpoint.c | 14 +++++++++++---
>  fs/f2fs/data.c       |  7 ++++---
>  fs/f2fs/f2fs.h       |  5 ++++-
>  fs/f2fs/file.c       |  8 ++++----
>  fs/f2fs/inode.c      |  7 +++++--
>  fs/f2fs/super.c      |  1 +
>  6 files changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index d1560bb..834c8ec 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -26,8 +26,17 @@
>  static struct kmem_cache *ino_entry_slab;
>  struct kmem_cache *inode_entry_slab;
> 
> -void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
> +void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi,
> +					bool end_io, bool need_stop)
>  {
> +#ifdef CONFIG_F2FS_FAULT_INJECTION
> +	if (time_to_inject(FAULT_CHECKPOINT))
> +		need_stop = true;
> +#endif
> +
> +	if (!need_stop)
> +		return;
> +
>  	set_ckpt_flags(sbi, CP_ERROR_FLAG);
>  	sbi->sb->s_flags |= MS_RDONLY;
>  	if (!end_io)
> @@ -100,8 +109,7 @@ repeat:
>  	 * readonly and make sure do not write checkpoint with non-uptodate
>  	 * meta page.
>  	 */
> -	if (unlikely(!PageUptodate(page)))
> -		f2fs_stop_checkpoint(sbi, false);
> +	f2fs_stop_checkpoint(sbi, false, !PageUptodate(page));
>  out:
>  	return page;
>  }
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index a9f7436..1b00d3d 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -74,10 +74,11 @@ static void f2fs_write_end_io(struct bio *bio)
> 
>  		fscrypt_pullback_bio_page(&page, true);
> 
> -		if (unlikely(bio->bi_error)) {
> +		f2fs_stop_checkpoint(sbi, true, bio->bi_error);
> +
> +		if (unlikely(bio->bi_error))
>  			set_bit(AS_EIO, &page->mapping->flags);
> -			f2fs_stop_checkpoint(sbi, true);
> -		}
> +
>  		end_page_writeback(page);
>  	}
>  	if (atomic_dec_and_test(&sbi->nr_wb_bios) &&
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index e216bc0..7bc1802 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -47,6 +47,7 @@ enum {
>  	FAULT_DIR_DEPTH,
>  	FAULT_EVICT_INODE,
>  	FAULT_IO,
> +	FAULT_CHECKPOINT,
>  	FAULT_MAX,
>  };
> 
> @@ -80,6 +81,8 @@ static inline bool time_to_inject(int type)
>  		return false;
>  	else if (type == FAULT_IO && !IS_FAULT_SET(type))
>  		return false;
> +	else if (type == FAULT_CHECKPOINT && !IS_FAULT_SET(type))
> +		return false;
> 
>  	atomic_inc(&f2fs_fault.inject_ops);
>  	if (atomic_read(&f2fs_fault.inject_ops) >= f2fs_fault.inject_rate) {
> @@ -2115,7 +2118,7 @@ void destroy_segment_manager_caches(void);
>  /*
>   * checkpoint.c
>   */
> -void f2fs_stop_checkpoint(struct f2fs_sb_info *, bool);
> +void f2fs_stop_checkpoint(struct f2fs_sb_info *, bool, bool);
>  struct page *grab_meta_page(struct f2fs_sb_info *, pgoff_t);
>  struct page *get_meta_page(struct f2fs_sb_info *, pgoff_t);
>  struct page *get_tmp_page(struct f2fs_sb_info *, pgoff_t);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index d341a0e..57c7a64 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1720,21 +1720,21 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
>  	case F2FS_GOING_DOWN_FULLSYNC:
>  		sb = freeze_bdev(sb->s_bdev);
>  		if (sb && !IS_ERR(sb)) {
> -			f2fs_stop_checkpoint(sbi, false);
> +			f2fs_stop_checkpoint(sbi, false, true);
>  			thaw_bdev(sb->s_bdev, sb);
>  		}
>  		break;
>  	case F2FS_GOING_DOWN_METASYNC:
>  		/* do checkpoint only */
>  		f2fs_sync_fs(sb, 1);
> -		f2fs_stop_checkpoint(sbi, false);
> +		f2fs_stop_checkpoint(sbi, false, true);
>  		break;
>  	case F2FS_GOING_DOWN_NOSYNC:
> -		f2fs_stop_checkpoint(sbi, false);
> +		f2fs_stop_checkpoint(sbi, false, true);
>  		break;
>  	case F2FS_GOING_DOWN_METAFLUSH:
>  		sync_meta_pages(sbi, META, LONG_MAX);
> -		f2fs_stop_checkpoint(sbi, false);
> +		f2fs_stop_checkpoint(sbi, false, true);
>  		break;
>  	default:
>  		ret = -EINVAL;
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index ac4daa5..bf53cd2 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -306,12 +306,15 @@ retry:
>  	node_page = get_node_page(sbi, inode->i_ino);
>  	if (IS_ERR(node_page)) {
>  		int err = PTR_ERR(node_page);
> +
> +		f2fs_stop_checkpoint(sbi, false,
> +				err != -ENOMEM && err != -ENOENT);
> +
>  		if (err == -ENOMEM) {
>  			cond_resched();
>  			goto retry;
> -		} else if (err != -ENOENT) {
> -			f2fs_stop_checkpoint(sbi, false);
>  		}
> +
>  		f2fs_inode_synced(inode);
>  		return 0;
>  	}
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 6426855..3c49419 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -51,6 +51,7 @@ char *fault_name[FAULT_MAX] = {
>  	[FAULT_DIR_DEPTH]	= "too big dir depth",
>  	[FAULT_EVICT_INODE]	= "evict_inode fail",
>  	[FAULT_IO]		= "IO error",
> +	[FAULT_CHECKPOINT]	= "checkpoint error",
>  };
> 
>  static void f2fs_build_fault_attr(unsigned int rate)
> -- 
> 2.8.2.311.gee88674
> 

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

On 2016/9/24 8:52, Jaegeuk Kim wrote:
> On Sat, Sep 24, 2016 at 08:46:54AM +0800, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2016/9/24 7:53, Jaegeuk Kim wrote:
>>> Hi Chao,
>>>
>>> The basic rule is to stop every operations once CP_ERROR_FLAG is set.
>>> But, this patch simply breaks the rule.
>>> For example, f2fs_write_data_page() currently exits with mapping_set_error().
>>> So this patch incurs missing dentry blocks in a valid checkpoint.
>>
>> Yes, that's right.
>>
>> How about triggering checkpoint error in f2fs_stop_checkpoint?
> 
> Let's just use src/godown in xfstests, since we don't need to trigger this
> multiple times in runtime.

After we inject checkpoint error into f2fs at first time, all write IOs will be
refused to be writebacked to storage, meanwhile read IOs can continuously go
through f2fs, so with checkpoint error injection being supported, we can support
to trigger random analogously power off by f2fs itself, instead of using tools.
It means it doesn't needs specified test cases where we must use godown ioctl,
but with normal testcases in xfstest/fsstress/lkp, in CP error injection enabled
f2fs, we can test power off cases.

Thanks,

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

On 2016/9/24 8:52, Jaegeuk Kim wrote:
> On Sat, Sep 24, 2016 at 08:46:54AM +0800, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2016/9/24 7:53, Jaegeuk Kim wrote:
>>> Hi Chao,
>>>
>>> The basic rule is to stop every operations once CP_ERROR_FLAG is set.
>>> But, this patch simply breaks the rule.
>>> For example, f2fs_write_data_page() currently exits with mapping_set_error().
>>> So this patch incurs missing dentry blocks in a valid checkpoint.
>>
>> Yes, that's right.
>>
>> How about triggering checkpoint error in f2fs_stop_checkpoint?
> 
> Let's just use src/godown in xfstests, since we don't need to trigger this
> multiple times in runtime.

After we inject checkpoint error into f2fs at first time, all write IOs will be
refused to be writebacked to storage, meanwhile read IOs can continuously go
through f2fs, so with checkpoint error injection being supported, we can support
to trigger random analogously power off by f2fs itself, instead of using tools.
It means it doesn't needs specified test cases where we must use godown ioctl,
but with normal testcases in xfstest/fsstress/lkp, in CP error injection enabled
f2fs, we can test power off cases.

Thanks,


------------------------------------------------------------------------------

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Jaegeuk Kim]

On Sat, Sep 24, 2016 at 11:32:08AM +0800, Chao Yu wrote:
> On 2016/9/24 8:52, Jaegeuk Kim wrote:
> > On Sat, Sep 24, 2016 at 08:46:54AM +0800, Chao Yu wrote:
> >> Hi Jaegeuk,
> >>
> >> On 2016/9/24 7:53, Jaegeuk Kim wrote:
> >>> Hi Chao,
> >>>
> >>> The basic rule is to stop every operations once CP_ERROR_FLAG is set.
> >>> But, this patch simply breaks the rule.
> >>> For example, f2fs_write_data_page() currently exits with mapping_set_error().
> >>> So this patch incurs missing dentry blocks in a valid checkpoint.
> >>
> >> Yes, that's right.
> >>
> >> How about triggering checkpoint error in f2fs_stop_checkpoint?
> > 
> > Let's just use src/godown in xfstests, since we don't need to trigger this
> > multiple times in runtime.
> 
> After we inject checkpoint error into f2fs at first time, all write IOs will be
> refused to be writebacked to storage, meanwhile read IOs can continuously go
> through f2fs, so with checkpoint error injection being supported, we can support
> to trigger random analogously power off by f2fs itself, instead of using tools.
> It means it doesn't needs specified test cases where we must use godown ioctl,
> but with normal testcases in xfstest/fsstress/lkp, in CP error injection enabled
> f2fs, we can test power off cases.

But, in this approach, the test coverage would be quite limited.
In my testcase, I'm randomly injecting godown while fsstress is running, which
mimics really random power failures, as I believe. I'm running this infinitely
with fscking at every run.

Anyway, in order to do this without godown, how about background_gc thread to
trigger f2fs_stop_checkpoint?

> 
> Thanks,

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

On 2016/9/25 2:10, Jaegeuk Kim wrote:
> On Sat, Sep 24, 2016 at 11:32:08AM +0800, Chao Yu wrote:
>> On 2016/9/24 8:52, Jaegeuk Kim wrote:
>>> On Sat, Sep 24, 2016 at 08:46:54AM +0800, Chao Yu wrote:
>>>> Hi Jaegeuk,
>>>>
>>>> On 2016/9/24 7:53, Jaegeuk Kim wrote:
>>>>> Hi Chao,
>>>>>
>>>>> The basic rule is to stop every operations once CP_ERROR_FLAG is set.
>>>>> But, this patch simply breaks the rule.
>>>>> For example, f2fs_write_data_page() currently exits with mapping_set_error().
>>>>> So this patch incurs missing dentry blocks in a valid checkpoint.
>>>>
>>>> Yes, that's right.
>>>>
>>>> How about triggering checkpoint error in f2fs_stop_checkpoint?
>>>
>>> Let's just use src/godown in xfstests, since we don't need to trigger this
>>> multiple times in runtime.
>>
>> After we inject checkpoint error into f2fs at first time, all write IOs will be
>> refused to be writebacked to storage, meanwhile read IOs can continuously go
>> through f2fs, so with checkpoint error injection being supported, we can support
>> to trigger random analogously power off by f2fs itself, instead of using tools.
>> It means it doesn't needs specified test cases where we must use godown ioctl,
>> but with normal testcases in xfstest/fsstress/lkp, in CP error injection enabled
>> f2fs, we can test power off cases.
> 
> But, in this approach, the test coverage would be quite limited.
> In my testcase, I'm randomly injecting godown while fsstress is running, which
> mimics really random power failures, as I believe. I'm running this infinitely
> with fscking at every run.
> 
> Anyway, in order to do this without godown, how about background_gc thread to
> trigger f2fs_stop_checkpoint?

Yeap, better.

What do you think of adding random f2fs_stop_checkpoint in f2fs_balance_fs?
power off can be triggered if gc thread is not running.

Thanks,

> 
>>
>> Thanks,
> 
> .
> 

Re: [PATCH 2/3] f2fs: support checkpoint error injection

[Chao Yu]

On 2016/9/25 2:10, Jaegeuk Kim wrote:
> On Sat, Sep 24, 2016 at 11:32:08AM +0800, Chao Yu wrote:
>> On 2016/9/24 8:52, Jaegeuk Kim wrote:
>>> On Sat, Sep 24, 2016 at 08:46:54AM +0800, Chao Yu wrote:
>>>> Hi Jaegeuk,
>>>>
>>>> On 2016/9/24 7:53, Jaegeuk Kim wrote:
>>>>> Hi Chao,
>>>>>
>>>>> The basic rule is to stop every operations once CP_ERROR_FLAG is set.
>>>>> But, this patch simply breaks the rule.
>>>>> For example, f2fs_write_data_page() currently exits with mapping_set_error().
>>>>> So this patch incurs missing dentry blocks in a valid checkpoint.
>>>>
>>>> Yes, that's right.
>>>>
>>>> How about triggering checkpoint error in f2fs_stop_checkpoint?
>>>
>>> Let's just use src/godown in xfstests, since we don't need to trigger this
>>> multiple times in runtime.
>>
>> After we inject checkpoint error into f2fs at first time, all write IOs will be
>> refused to be writebacked to storage, meanwhile read IOs can continuously go
>> through f2fs, so with checkpoint error injection being supported, we can support
>> to trigger random analogously power off by f2fs itself, instead of using tools.
>> It means it doesn't needs specified test cases where we must use godown ioctl,
>> but with normal testcases in xfstest/fsstress/lkp, in CP error injection enabled
>> f2fs, we can test power off cases.
> 
> But, in this approach, the test coverage would be quite limited.
> In my testcase, I'm randomly injecting godown while fsstress is running, which
> mimics really random power failures, as I believe. I'm running this infinitely
> with fscking at every run.
> 
> Anyway, in order to do this without godown, how about background_gc thread to
> trigger f2fs_stop_checkpoint?

Yeap, better.

What do you think of adding random f2fs_stop_checkpoint in f2fs_balance_fs?
power off can be triggered if gc thread is not running.

Thanks,

> 
>>
>> Thanks,
> 
> .
> 


------------------------------------------------------------------------------