[PATCH 1/7] f2fs: run discard jobs when put_super

[PATCH 1/7] f2fs: run discard jobs when put_super

[Jaegeuk Kim]

When we umount f2fs, we need to avoid long delay due to discard commands, which
is actually taking tens of seconds, if storage is very slow on UNMAP. So, this
patch introduces timeout-based work on it.

By default, let me give 5 seconds for discard.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 Documentation/ABI/testing/sysfs-fs-f2fs |  7 +++++++
 fs/f2fs/f2fs.h                          |  5 ++++-
 fs/f2fs/segment.c                       | 11 ++++++++++-
 fs/f2fs/super.c                         |  4 +++-
 fs/f2fs/sysfs.c                         |  3 +++
 5 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index a7ce33199457..91822ce25831 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -86,6 +86,13 @@ Description:
 		The unit size is one block, now only support configuring in range
 		of [1, 512].
 
+What:          /sys/fs/f2fs/<disk>/umount_discard_timeout
+Date:          January 2019
+Contact:       "Jaegeuk Kim" <jaegeuk@kernel.org>
+Description:
+		Set timeout to issue discard commands during umount.
+		Default: 5 secs
+
 What:		/sys/fs/f2fs/<disk>/max_victim_search
 Date:		January 2014
 Contact:	"Jaegeuk Kim" <jaegeuk.kim@samsung.com>
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 0f564883e078..6b6ec5600089 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -191,6 +191,7 @@ enum {
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
 #define DEF_DISABLE_INTERVAL		5	/* 5 secs */
+#define DEF_UMOUNT_DISCARD_TIMEOUT	5	/* 5 secs */
 
 struct cp_control {
 	int reason;
@@ -310,6 +311,7 @@ struct discard_policy {
 	bool sync;			/* submit discard with REQ_SYNC flag */
 	bool ordered;			/* issue discard by lba order */
 	unsigned int granularity;	/* discard granularity */
+	int timeout;			/* discard timeout for put_super */
 };
 
 struct discard_cmd_control {
@@ -1110,6 +1112,7 @@ enum {
 	DISCARD_TIME,
 	GC_TIME,
 	DISABLE_TIME,
+	UMOUNT_DISCARD_TIMEOUT,
 	MAX_TIME,
 };
 
@@ -3006,7 +3009,7 @@ void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr);
 bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr);
 void f2fs_drop_discard_cmd(struct f2fs_sb_info *sbi);
 void f2fs_stop_discard_thread(struct f2fs_sb_info *sbi);
-bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi);
+bool f2fs_issue_discard_timeout(struct f2fs_sb_info *sbi);
 void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
 					struct cp_control *cpc);
 void f2fs_dirty_to_prefree(struct f2fs_sb_info *sbi);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9b79056d705d..5b2b9be6f28d 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1037,6 +1037,7 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
 
 	dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
 	dpolicy->io_aware_gran = MAX_PLIST_NUM;
+	dpolicy->timeout = 0;
 
 	if (discard_type == DPOLICY_BG) {
 		dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
@@ -1424,7 +1425,14 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
 	int i, issued = 0;
 	bool io_interrupted = false;
 
+	if (dpolicy->timeout != 0)
+		f2fs_update_time(sbi, dpolicy->timeout);
+
 	for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
+		if (dpolicy->timeout != 0 &&
+				f2fs_time_over(sbi, dpolicy->timeout))
+			break;
+
 		if (i + 1 < dpolicy->granularity)
 			break;
 
@@ -1611,7 +1619,7 @@ void f2fs_stop_discard_thread(struct f2fs_sb_info *sbi)
 }
 
 /* This comes from f2fs_put_super */
-bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi)
+bool f2fs_issue_discard_timeout(struct f2fs_sb_info *sbi)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	struct discard_policy dpolicy;
@@ -1619,6 +1627,7 @@ bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi)
 
 	__init_discard_policy(sbi, &dpolicy, DPOLICY_UMOUNT,
 					dcc->discard_granularity);
+	dpolicy.timeout = UMOUNT_DISCARD_TIMEOUT;
 	__issue_discard_cmd(sbi, &dpolicy);
 	dropped = __drop_discard_cmd(sbi);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index ea514acede36..24efd76ca151 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1048,7 +1048,7 @@ static void f2fs_put_super(struct super_block *sb)
 	}
 
 	/* be sure to wait for any on-going discard commands */
-	dropped = f2fs_wait_discard_bios(sbi);
+	dropped = f2fs_issue_discard_timeout(sbi);
 
 	if ((f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) &&
 					!sbi->discard_blks && !dropped) {
@@ -2706,6 +2706,8 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 	sbi->interval_time[DISCARD_TIME] = DEF_IDLE_INTERVAL;
 	sbi->interval_time[GC_TIME] = DEF_IDLE_INTERVAL;
 	sbi->interval_time[DISABLE_TIME] = DEF_DISABLE_INTERVAL;
+	sbi->interval_time[UMOUNT_DISCARD_TIMEOUT] =
+				DEF_UMOUNT_DISCARD_TIMEOUT;
 	clear_sbi_flag(sbi, SBI_NEED_FSCK);
 
 	for (i = 0; i < NR_COUNT_TYPE; i++)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 5b83e1d66cb3..18c627e8fc90 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -426,6 +426,8 @@ F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, idle_interval, interval_time[REQ_TIME]);
 F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, discard_idle_interval,
 					interval_time[DISCARD_TIME]);
 F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_idle_interval, interval_time[GC_TIME]);
+F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info,
+		umount_discard_timeout, interval_time[UMOUNT_DISCARD_TIMEOUT]);
 F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, iostat_enable, iostat_enable);
 F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, readdir_ra, readdir_ra);
 F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_pin_file_thresh, gc_pin_file_threshold);
@@ -483,6 +485,7 @@ static struct attribute *f2fs_attrs[] = {
 	ATTR_LIST(idle_interval),
 	ATTR_LIST(discard_idle_interval),
 	ATTR_LIST(gc_idle_interval),
+	ATTR_LIST(umount_discard_timeout),
 	ATTR_LIST(iostat_enable),
 	ATTR_LIST(readdir_ra),
 	ATTR_LIST(gc_pin_file_thresh),
-- 
2.19.0.605.g01d371f741-goog

[PATCH 2/7] f2fs: add quick mode of checkpoint=disable for QA

[Jaegeuk Kim]

This mode returns mount() quickly with EAGAIN. We can trigger this by
shutdown(F2FS_GOING_DOWN_NEED_FSCK).

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c    | 5 +++++
 fs/f2fs/f2fs.h          | 2 ++
 fs/f2fs/file.c          | 6 +++---
 fs/f2fs/segment.c       | 3 +++
 fs/f2fs/super.c         | 5 +++++
 include/linux/f2fs_fs.h | 1 +
 6 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index f955cd3e0677..622dca707752 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1259,6 +1259,11 @@ static void update_ckpt_flags(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	else
 		__clear_ckpt_flags(ckpt, CP_DISABLED_FLAG);
 
+	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK))
+		__set_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
+	else
+		__clear_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
+
 	if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH))
 		__set_ckpt_flags(ckpt, CP_QUOTA_NEED_FSCK_FLAG);
 	else
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 6b6ec5600089..fe95abb05d40 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -191,6 +191,7 @@ enum {
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
 #define DEF_DISABLE_INTERVAL		5	/* 5 secs */
+#define DEF_DISABLE_QUICK_INTERVAL	1	/* 1 secs */
 #define DEF_UMOUNT_DISCARD_TIMEOUT	5	/* 5 secs */
 
 struct cp_control {
@@ -1101,6 +1102,7 @@ enum {
 	SBI_IS_SHUTDOWN,			/* shutdown by ioctl */
 	SBI_IS_RECOVERED,			/* recovered orphan/data */
 	SBI_CP_DISABLED,			/* CP was disabled last mount */
+	SBI_CP_DISABLED_QUICK,			/* CP was disabled quickly */
 	SBI_QUOTA_NEED_FLUSH,			/* need to flush quota info in CP */
 	SBI_QUOTA_SKIP_FLUSH,			/* skip flushing quota in current CP */
 	SBI_QUOTA_NEED_REPAIR,			/* quota file may be corrupted */
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 0d461321edfc..fe6f92fbba38 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1972,11 +1972,11 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		break;
 	case F2FS_GOING_DOWN_NEED_FSCK:
 		set_sbi_flag(sbi, SBI_NEED_FSCK);
+		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
+		set_sbi_flag(sbi, SBI_IS_DIRTY);
 		/* do checkpoint only */
 		ret = f2fs_sync_fs(sb, 1);
-		if (ret)
-			goto out;
-		break;
+		goto out;
 	default:
 		ret = -EINVAL;
 		goto out;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 5b2b9be6f28d..342b720fb4db 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -868,6 +868,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi)
 
 	if (holes[DATA] > ovp || holes[NODE] > ovp)
 		return -EAGAIN;
+	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
+		dirty_segments(sbi) > overprovision_segments(sbi))
+		return -EAGAIN;
 	return 0;
 }
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 24efd76ca151..5e1f8573a17f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3205,6 +3205,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 
 	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_QUOTA_NEED_FSCK_FLAG))
 		set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
+	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_DISABLED_QUICK_FLAG)) {
+		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
+		sbi->interval_time[DISABLE_TIME] = DEF_DISABLE_QUICK_INTERVAL;
+	}
 
 	/* Initialize device list */
 	err = f2fs_scan_devices(sbi);
@@ -3392,6 +3396,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 				cur_cp_version(F2FS_CKPT(sbi)));
 	f2fs_update_time(sbi, CP_TIME);
 	f2fs_update_time(sbi, REQ_TIME);
+	clear_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
 	return 0;
 
 free_meta:
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index d7711048ef93..d6befe1f9dc7 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -116,6 +116,7 @@ struct f2fs_super_block {
 /*
  * For checkpoint
  */
+#define CP_DISABLED_QUICK_FLAG		0x00002000
 #define CP_DISABLED_FLAG		0x00001000
 #define CP_QUOTA_NEED_FSCK_FLAG		0x00000800
 #define CP_LARGE_NAT_BITMAP_FLAG	0x00000400
-- 
2.19.0.605.g01d371f741-goog

[PATCH 3/7] f2fs: try to keep CP_TRIMMED_FLAG after successful umount

[Jaegeuk Kim]

If every discard were issued successfully, we can avoid further discard.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/segment.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 342b720fb4db..fdd8cd21522f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1063,6 +1063,8 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
 	} else if (discard_type == DPOLICY_UMOUNT) {
 		dpolicy->max_requests = UINT_MAX;
 		dpolicy->io_aware = false;
+		/* we need to issue all to keep CP_TRIMMED_FLAG */
+		dpolicy->granularity = 1;
 	}
 }
 
-- 
2.19.0.605.g01d371f741-goog

[PATCH 4/7] f2fs: don't wake up too frequently, if there is lots of IOs

[Jaegeuk Kim]

Otherwise, it wakes up discard thread which will sleep again by busy IOs
in a loop.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/segment.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index a77f76f528b6..5c7ed0442d6e 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -865,7 +865,7 @@ static inline void wake_up_discard_thread(struct f2fs_sb_info *sbi, bool force)
 		}
 	}
 	mutex_unlock(&dcc->cmd_lock);
-	if (!wakeup)
+	if (!wakeup || !is_idle(sbi, DISCARD_TIME))
 		return;
 wake_up:
 	dcc->discard_wake = 1;
-- 
2.19.0.605.g01d371f741-goog

[PATCH 5/7] f2fs: avoid null pointer exception in dcc_info

[Jaegeuk Kim]

If dcc_info is not set yet, we can get null pointer panic.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index fe95abb05d40..8c928cd72b61 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2161,10 +2161,17 @@ static inline bool is_idle(struct f2fs_sb_info *sbi, int type)
 		get_pages(sbi, F2FS_RD_META) || get_pages(sbi, F2FS_WB_DATA) ||
 		get_pages(sbi, F2FS_WB_CP_DATA) ||
 		get_pages(sbi, F2FS_DIO_READ) ||
-		get_pages(sbi, F2FS_DIO_WRITE) ||
-		atomic_read(&SM_I(sbi)->dcc_info->queued_discard) ||
-		atomic_read(&SM_I(sbi)->fcc_info->queued_flush))
+		get_pages(sbi, F2FS_DIO_WRITE))
 		return false;
+
+	if (SM_I(sbi) && SM_I(sbi)->dcc_info &&
+			atomic_read(&SM_I(sbi)->dcc_info->queued_discard))
+		return false;
+
+	if (SM_I(sbi) && SM_I(sbi)->fcc_info &&
+			atomic_read(&SM_I(sbi)->fcc_info->queued_flush))
+		return false;
+
 	return f2fs_time_over(sbi, type);
 }
 
-- 
2.19.0.605.g01d371f741-goog

[PATCH 6/7] f2fs: flush quota blocks after turnning it off

[Jaegeuk Kim]

After quota_off, we'll get some dirty blocks. If put_super don't have a chance
to flush them by checkpoint, it causes NULL pointer exception in end_io after
iput(node_inode). (e.g., by checkpoint=disable)

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/super.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 5e1f8573a17f..7694cb350734 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2026,6 +2026,12 @@ void f2fs_quota_off_umount(struct super_block *sb)
 			set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
 		}
 	}
+	/*
+	 * In case of checkpoint=disable, we must flush quota blocks.
+	 * This can cause NULL exception for node_inode in end_io, since
+	 * put_super already dropped it.
+	 */
+	sync_filesystem(sb);
 }
 
 static void f2fs_truncate_quota_inode_pages(struct super_block *sb)
-- 
2.19.0.605.g01d371f741-goog

[PATCH 7/7] f2fs: sync filesystem after roll-forward recovery

[Jaegeuk Kim]

Some works after roll-forward recovery can get an error which will release
all the data structures. Let's flush them in order to make it clean.

One possible corruption came from:

[   90.400500] list_del corruption. prev->next should be ffffffed1f566208, but was (null)
[   90.675349] Call trace:
[   90.677869]  __list_del_entry_valid+0x94/0xb4
[   90.682351]  remove_dirty_inode+0xac/0x114
[   90.686563]  __f2fs_write_data_pages+0x6a8/0x6c8
[   90.691302]  f2fs_write_data_pages+0x40/0x4c
[   90.695695]  do_writepages+0x80/0xf0
[   90.699372]  __writeback_single_inode+0xdc/0x4ac
[   90.704113]  writeback_sb_inodes+0x280/0x440
[   90.708501]  wb_writeback+0x1b8/0x3d0
[   90.712267]  wb_workfn+0x1a8/0x4d4
[   90.715765]  process_one_work+0x1c0/0x3d4
[   90.719883]  worker_thread+0x224/0x344
[   90.723739]  kthread+0x120/0x130
[   90.727055]  ret_from_fork+0x10/0x18

Reported-by: Sahitya Tummala <stummala@codeaurora.org>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c |  5 +++--
 fs/f2fs/node.c       |  4 +++-
 fs/f2fs/super.c      | 44 +++++++++++++++++++++++++++++++++-----------
 3 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 622dca707752..03fea4efd64b 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -306,8 +306,9 @@ static int f2fs_write_meta_pages(struct address_space *mapping,
 		goto skip_write;
 
 	/* collect a number of dirty meta pages and write together */
-	if (wbc->for_kupdate ||
-		get_pages(sbi, F2FS_DIRTY_META) < nr_pages_to_skip(sbi, META))
+	if (wbc->sync_mode != WB_SYNC_ALL &&
+			get_pages(sbi, F2FS_DIRTY_META) <
+					nr_pages_to_skip(sbi, META))
 		goto skip_write;
 
 	/* if locked failed, cp will flush dirty pages instead */
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 4f450e573312..f6ff84e29749 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1920,7 +1920,9 @@ static int f2fs_write_node_pages(struct address_space *mapping,
 	f2fs_balance_fs_bg(sbi);
 
 	/* collect a number of dirty node pages and write together */
-	if (get_pages(sbi, F2FS_DIRTY_NODES) < nr_pages_to_skip(sbi, NODE))
+	if (wbc->sync_mode != WB_SYNC_ALL &&
+			get_pages(sbi, F2FS_DIRTY_NODES) <
+					nr_pages_to_skip(sbi, NODE))
 		goto skip_write;
 
 	if (wbc->sync_mode == WB_SYNC_ALL)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 7694cb350734..384d1c248857 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1458,9 +1458,16 @@ static int f2fs_enable_quotas(struct super_block *sb);
 
 static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 {
+	unsigned int s_flags = sbi->sb->s_flags;
 	struct cp_control cpc;
-	int err;
+	int err = 0;
+	int ret;
 
+	if (s_flags & SB_RDONLY) {
+		f2fs_msg(sbi->sb, KERN_ERR,
+				"checkpoint=disable on readonly fs");
+		return -EINVAL;
+	}
 	sbi->sb->s_flags |= SB_ACTIVE;
 
 	f2fs_update_time(sbi, DISABLE_TIME);
@@ -1468,18 +1475,24 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 	while (!f2fs_time_over(sbi, DISABLE_TIME)) {
 		mutex_lock(&sbi->gc_mutex);
 		err = f2fs_gc(sbi, true, false, NULL_SEGNO);
-		if (err == -ENODATA)
+		if (err == -ENODATA) {
+			err = 0;
 			break;
+		}
 		if (err && err != -EAGAIN)
-			return err;
+			break;
 	}
 
-	err = sync_filesystem(sbi->sb);
-	if (err)
-		return err;
+	ret = sync_filesystem(sbi->sb);
+	if (ret || err) {
+		err = ret ? ret: err;
+		goto restore_flag;
+	}
 
-	if (f2fs_disable_cp_again(sbi))
-		return -EAGAIN;
+	if (f2fs_disable_cp_again(sbi)) {
+		err = -EAGAIN;
+		goto restore_flag;
+	}
 
 	mutex_lock(&sbi->gc_mutex);
 	cpc.reason = CP_PAUSE;
@@ -1488,7 +1501,9 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 
 	sbi->unusable_block_count = 0;
 	mutex_unlock(&sbi->gc_mutex);
-	return 0;
+restore_flag:
+	sbi->sb->s_flags = s_flags;	/* Restore MS_RDONLY status */
+	return err;
 }
 
 static void f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
@@ -3369,7 +3384,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	if (test_opt(sbi, DISABLE_CHECKPOINT)) {
 		err = f2fs_disable_checkpoint(sbi);
 		if (err)
-			goto free_meta;
+			goto sync_free_meta;
 	} else if (is_set_ckpt_flags(sbi, CP_DISABLED_FLAG)) {
 		f2fs_enable_checkpoint(sbi);
 	}
@@ -3382,7 +3397,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		/* After POR, we can run background GC thread.*/
 		err = f2fs_start_gc_thread(sbi);
 		if (err)
-			goto free_meta;
+			goto sync_free_meta;
 	}
 	kvfree(options);
 
@@ -3405,6 +3420,11 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	clear_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
 	return 0;
 
+sync_free_meta:
+	/* safe to flush all the data */
+	sync_filesystem(sbi->sb);
+	retry = false;
+
 free_meta:
 #ifdef CONFIG_QUOTA
 	f2fs_truncate_quota_inode_pages(sb);
@@ -3418,6 +3438,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	 * falls into an infinite loop in f2fs_sync_meta_pages().
 	 */
 	truncate_inode_pages_final(META_MAPPING(sbi));
+	/* evict some inodes being cached by GC */
+	evict_inodes(sb);
 	f2fs_unregister_sysfs(sbi);
 free_root_inode:
 	dput(sb->s_root);
-- 
2.19.0.605.g01d371f741-goog

Re: [f2fs-dev] [PATCH 1/7] f2fs: run discard jobs when put_super

[Chao Yu]

On 2019-1-29 7:47, Jaegeuk Kim wrote:
> When we umount f2fs, we need to avoid long delay due to discard commands, which
> is actually taking tens of seconds, if storage is very slow on UNMAP. So, this
> patch introduces timeout-based work on it.
> 
> By default, let me give 5 seconds for discard.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [f2fs-dev] [PATCH 2/7] f2fs: add quick mode of checkpoint=disable for QA

[Chao Yu]

On 2019-1-29 7:47, Jaegeuk Kim wrote:
> This mode returns mount() quickly with EAGAIN. We can trigger this by
> shutdown(F2FS_GOING_DOWN_NEED_FSCK).

Is the purpose of this patch making mount failed more quickly due to giving less
GC time in f2fs_disable_checkpoint(), so we can expect that the total test time
can be reduce?

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/checkpoint.c    | 5 +++++
>  fs/f2fs/f2fs.h          | 2 ++
>  fs/f2fs/file.c          | 6 +++---
>  fs/f2fs/segment.c       | 3 +++
>  fs/f2fs/super.c         | 5 +++++
>  include/linux/f2fs_fs.h | 1 +
>  6 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index f955cd3e0677..622dca707752 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1259,6 +1259,11 @@ static void update_ckpt_flags(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	else
>  		__clear_ckpt_flags(ckpt, CP_DISABLED_FLAG);
>  
> +	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK))
> +		__set_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
> +	else
> +		__clear_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
> +
>  	if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH))
>  		__set_ckpt_flags(ckpt, CP_QUOTA_NEED_FSCK_FLAG);
>  	else
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 6b6ec5600089..fe95abb05d40 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -191,6 +191,7 @@ enum {
>  #define DEF_CP_INTERVAL			60	/* 60 secs */
>  #define DEF_IDLE_INTERVAL		5	/* 5 secs */
>  #define DEF_DISABLE_INTERVAL		5	/* 5 secs */
> +#define DEF_DISABLE_QUICK_INTERVAL	1	/* 1 secs */
>  #define DEF_UMOUNT_DISCARD_TIMEOUT	5	/* 5 secs */
>  
>  struct cp_control {
> @@ -1101,6 +1102,7 @@ enum {
>  	SBI_IS_SHUTDOWN,			/* shutdown by ioctl */
>  	SBI_IS_RECOVERED,			/* recovered orphan/data */
>  	SBI_CP_DISABLED,			/* CP was disabled last mount */
> +	SBI_CP_DISABLED_QUICK,			/* CP was disabled quickly */
>  	SBI_QUOTA_NEED_FLUSH,			/* need to flush quota info in CP */
>  	SBI_QUOTA_SKIP_FLUSH,			/* skip flushing quota in current CP */
>  	SBI_QUOTA_NEED_REPAIR,			/* quota file may be corrupted */
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 0d461321edfc..fe6f92fbba38 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1972,11 +1972,11 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
>  		break;
>  	case F2FS_GOING_DOWN_NEED_FSCK:
>  		set_sbi_flag(sbi, SBI_NEED_FSCK);
> +		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
> +		set_sbi_flag(sbi, SBI_IS_DIRTY);
>  		/* do checkpoint only */
>  		ret = f2fs_sync_fs(sb, 1);
> -		if (ret)
> -			goto out;
> -		break;
> +		goto out;
>  	default:
>  		ret = -EINVAL;
>  		goto out;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 5b2b9be6f28d..342b720fb4db 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -868,6 +868,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi)
>  
>  	if (holes[DATA] > ovp || holes[NODE] > ovp)
>  		return -EAGAIN;
> +	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
> +		dirty_segments(sbi) > overprovision_segments(sbi))
> +		return -EAGAIN;
>  	return 0;
>  }
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 24efd76ca151..5e1f8573a17f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3205,6 +3205,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_QUOTA_NEED_FSCK_FLAG))
>  		set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
> +	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_DISABLED_QUICK_FLAG)) {
> +		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
> +		sbi->interval_time[DISABLE_TIME] = DEF_DISABLE_QUICK_INTERVAL;
> +	}
>  
>  	/* Initialize device list */
>  	err = f2fs_scan_devices(sbi);
> @@ -3392,6 +3396,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  				cur_cp_version(F2FS_CKPT(sbi)));
>  	f2fs_update_time(sbi, CP_TIME);
>  	f2fs_update_time(sbi, REQ_TIME);
> +	clear_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
>  	return 0;
>  
>  free_meta:
> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> index d7711048ef93..d6befe1f9dc7 100644
> --- a/include/linux/f2fs_fs.h
> +++ b/include/linux/f2fs_fs.h
> @@ -116,6 +116,7 @@ struct f2fs_super_block {
>  /*
>   * For checkpoint
>   */
> +#define CP_DISABLED_QUICK_FLAG		0x00002000
>  #define CP_DISABLED_FLAG		0x00001000
>  #define CP_QUOTA_NEED_FSCK_FLAG		0x00000800
>  #define CP_LARGE_NAT_BITMAP_FLAG	0x00000400
> 

Re: [f2fs-dev] [PATCH 3/7] f2fs: try to keep CP_TRIMMED_FLAG after successful umount

[Chao Yu]

On 2019-1-29 7:47, Jaegeuk Kim wrote:
> If every discard were issued successfully, we can avoid further discard.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [f2fs-dev] [PATCH 4/7] f2fs: don't wake up too frequently, if there is lots of IOs

[Chao Yu]

On 2019-1-29 7:47, Jaegeuk Kim wrote:
> Otherwise, it wakes up discard thread which will sleep again by busy IOs
> in a loop.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [f2fs-dev] [PATCH 5/7] f2fs: avoid null pointer exception in dcc_info

[Chao Yu]

On 2019-1-29 7:47, Jaegeuk Kim wrote:
> If dcc_info is not set yet, we can get null pointer panic.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [f2fs-dev] [PATCH 6/7] f2fs: flush quota blocks after turnning it off

[Chao Yu]

On 2019-1-29 7:47, Jaegeuk Kim wrote:
> After quota_off, we'll get some dirty blocks. If put_super don't have a chance
> to flush them by checkpoint, it causes NULL pointer exception in end_io after
> iput(node_inode). (e.g., by checkpoint=disable)
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/super.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 5e1f8573a17f..7694cb350734 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2026,6 +2026,12 @@ void f2fs_quota_off_umount(struct super_block *sb)
>  			set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
>  		}
>  	}
> +	/*
> +	 * In case of checkpoint=disable, we must flush quota blocks.

checkpoint=disable is one of the cases, right? e.g. IO error can be another case?

So here I guess we need to change the comments a bit for that.

Thanks,

> +	 * This can cause NULL exception for node_inode in end_io, since
> +	 * put_super already dropped it.
> +	 */
> +	sync_filesystem(sb);
>  }
>  
>  static void f2fs_truncate_quota_inode_pages(struct super_block *sb)
> 

Re: [f2fs-dev] [PATCH 2/7] f2fs: add quick mode of checkpoint=disable for QA

[Jaegeuk Kim]

On 02/01, Chao Yu wrote:
> On 2019-1-29 7:47, Jaegeuk Kim wrote:
> > This mode returns mount() quickly with EAGAIN. We can trigger this by
> > shutdown(F2FS_GOING_DOWN_NEED_FSCK).
> 
> Is the purpose of this patch making mount failed more quickly due to giving less
> GC time in f2fs_disable_checkpoint(), so we can expect that the total test time
> can be reduce?

Yup.

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/checkpoint.c    | 5 +++++
> >  fs/f2fs/f2fs.h          | 2 ++
> >  fs/f2fs/file.c          | 6 +++---
> >  fs/f2fs/segment.c       | 3 +++
> >  fs/f2fs/super.c         | 5 +++++
> >  include/linux/f2fs_fs.h | 1 +
> >  6 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index f955cd3e0677..622dca707752 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -1259,6 +1259,11 @@ static void update_ckpt_flags(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >  	else
> >  		__clear_ckpt_flags(ckpt, CP_DISABLED_FLAG);
> >  
> > +	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK))
> > +		__set_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
> > +	else
> > +		__clear_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
> > +
> >  	if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH))
> >  		__set_ckpt_flags(ckpt, CP_QUOTA_NEED_FSCK_FLAG);
> >  	else
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 6b6ec5600089..fe95abb05d40 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -191,6 +191,7 @@ enum {
> >  #define DEF_CP_INTERVAL			60	/* 60 secs */
> >  #define DEF_IDLE_INTERVAL		5	/* 5 secs */
> >  #define DEF_DISABLE_INTERVAL		5	/* 5 secs */
> > +#define DEF_DISABLE_QUICK_INTERVAL	1	/* 1 secs */
> >  #define DEF_UMOUNT_DISCARD_TIMEOUT	5	/* 5 secs */
> >  
> >  struct cp_control {
> > @@ -1101,6 +1102,7 @@ enum {
> >  	SBI_IS_SHUTDOWN,			/* shutdown by ioctl */
> >  	SBI_IS_RECOVERED,			/* recovered orphan/data */
> >  	SBI_CP_DISABLED,			/* CP was disabled last mount */
> > +	SBI_CP_DISABLED_QUICK,			/* CP was disabled quickly */
> >  	SBI_QUOTA_NEED_FLUSH,			/* need to flush quota info in CP */
> >  	SBI_QUOTA_SKIP_FLUSH,			/* skip flushing quota in current CP */
> >  	SBI_QUOTA_NEED_REPAIR,			/* quota file may be corrupted */
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 0d461321edfc..fe6f92fbba38 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -1972,11 +1972,11 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
> >  		break;
> >  	case F2FS_GOING_DOWN_NEED_FSCK:
> >  		set_sbi_flag(sbi, SBI_NEED_FSCK);
> > +		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
> > +		set_sbi_flag(sbi, SBI_IS_DIRTY);
> >  		/* do checkpoint only */
> >  		ret = f2fs_sync_fs(sb, 1);
> > -		if (ret)
> > -			goto out;
> > -		break;
> > +		goto out;
> >  	default:
> >  		ret = -EINVAL;
> >  		goto out;
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 5b2b9be6f28d..342b720fb4db 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -868,6 +868,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi)
> >  
> >  	if (holes[DATA] > ovp || holes[NODE] > ovp)
> >  		return -EAGAIN;
> > +	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
> > +		dirty_segments(sbi) > overprovision_segments(sbi))
> > +		return -EAGAIN;
> >  	return 0;
> >  }
> >  
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 24efd76ca151..5e1f8573a17f 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -3205,6 +3205,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> >  
> >  	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_QUOTA_NEED_FSCK_FLAG))
> >  		set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
> > +	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_DISABLED_QUICK_FLAG)) {
> > +		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
> > +		sbi->interval_time[DISABLE_TIME] = DEF_DISABLE_QUICK_INTERVAL;
> > +	}
> >  
> >  	/* Initialize device list */
> >  	err = f2fs_scan_devices(sbi);
> > @@ -3392,6 +3396,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> >  				cur_cp_version(F2FS_CKPT(sbi)));
> >  	f2fs_update_time(sbi, CP_TIME);
> >  	f2fs_update_time(sbi, REQ_TIME);
> > +	clear_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
> >  	return 0;
> >  
> >  free_meta:
> > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> > index d7711048ef93..d6befe1f9dc7 100644
> > --- a/include/linux/f2fs_fs.h
> > +++ b/include/linux/f2fs_fs.h
> > @@ -116,6 +116,7 @@ struct f2fs_super_block {
> >  /*
> >   * For checkpoint
> >   */
> > +#define CP_DISABLED_QUICK_FLAG		0x00002000
> >  #define CP_DISABLED_FLAG		0x00001000
> >  #define CP_QUOTA_NEED_FSCK_FLAG		0x00000800
> >  #define CP_LARGE_NAT_BITMAP_FLAG	0x00000400
> > 

Re: [f2fs-dev] [PATCH 6/7] f2fs: flush quota blocks after turnning it off

[Jaegeuk Kim]

On 02/02, Chao Yu wrote:
> On 2019-1-29 7:47, Jaegeuk Kim wrote:
> > After quota_off, we'll get some dirty blocks. If put_super don't have a chance
> > to flush them by checkpoint, it causes NULL pointer exception in end_io after
> > iput(node_inode). (e.g., by checkpoint=disable)
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/super.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 5e1f8573a17f..7694cb350734 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -2026,6 +2026,12 @@ void f2fs_quota_off_umount(struct super_block *sb)
> >  			set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
> >  		}
> >  	}
> > +	/*
> > +	 * In case of checkpoint=disable, we must flush quota blocks.
> 
> checkpoint=disable is one of the cases, right? e.g. IO error can be another case?
> 
> So here I guess we need to change the comments a bit for that.

I didn't test other cases, but checkpoint=disable could give real errors through
some tests. So, basically, I'd like to keep known cases only here.

> 
> Thanks,
> 
> > +	 * This can cause NULL exception for node_inode in end_io, since
> > +	 * put_super already dropped it.
> > +	 */
> > +	sync_filesystem(sb);
> >  }
> >  
> >  static void f2fs_truncate_quota_inode_pages(struct super_block *sb)
> > 

Re: [f2fs-dev] [PATCH 2/7] f2fs: add quick mode of checkpoint=disable for QA

[Chao Yu]

On 2019/2/5 0:55, Jaegeuk Kim wrote:
> On 02/01, Chao Yu wrote:
>> On 2019-1-29 7:47, Jaegeuk Kim wrote:
>>> This mode returns mount() quickly with EAGAIN. We can trigger this by
>>> shutdown(F2FS_GOING_DOWN_NEED_FSCK).
>>
>> Is the purpose of this patch making mount failed more quickly due to giving less
>> GC time in f2fs_disable_checkpoint(), so we can expect that the total test time
>> can be reduce?
> 
> Yup.

Got you. :)

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>>  fs/f2fs/checkpoint.c    | 5 +++++
>>>  fs/f2fs/f2fs.h          | 2 ++
>>>  fs/f2fs/file.c          | 6 +++---
>>>  fs/f2fs/segment.c       | 3 +++
>>>  fs/f2fs/super.c         | 5 +++++
>>>  include/linux/f2fs_fs.h | 1 +
>>>  6 files changed, 19 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index f955cd3e0677..622dca707752 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -1259,6 +1259,11 @@ static void update_ckpt_flags(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>  	else
>>>  		__clear_ckpt_flags(ckpt, CP_DISABLED_FLAG);
>>>  
>>> +	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK))
>>> +		__set_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
>>> +	else
>>> +		__clear_ckpt_flags(ckpt, CP_DISABLED_QUICK_FLAG);
>>> +
>>>  	if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH))
>>>  		__set_ckpt_flags(ckpt, CP_QUOTA_NEED_FSCK_FLAG);
>>>  	else
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 6b6ec5600089..fe95abb05d40 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -191,6 +191,7 @@ enum {
>>>  #define DEF_CP_INTERVAL			60	/* 60 secs */
>>>  #define DEF_IDLE_INTERVAL		5	/* 5 secs */
>>>  #define DEF_DISABLE_INTERVAL		5	/* 5 secs */
>>> +#define DEF_DISABLE_QUICK_INTERVAL	1	/* 1 secs */
>>>  #define DEF_UMOUNT_DISCARD_TIMEOUT	5	/* 5 secs */
>>>  
>>>  struct cp_control {
>>> @@ -1101,6 +1102,7 @@ enum {
>>>  	SBI_IS_SHUTDOWN,			/* shutdown by ioctl */
>>>  	SBI_IS_RECOVERED,			/* recovered orphan/data */
>>>  	SBI_CP_DISABLED,			/* CP was disabled last mount */
>>> +	SBI_CP_DISABLED_QUICK,			/* CP was disabled quickly */
>>>  	SBI_QUOTA_NEED_FLUSH,			/* need to flush quota info in CP */
>>>  	SBI_QUOTA_SKIP_FLUSH,			/* skip flushing quota in current CP */
>>>  	SBI_QUOTA_NEED_REPAIR,			/* quota file may be corrupted */
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index 0d461321edfc..fe6f92fbba38 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -1972,11 +1972,11 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
>>>  		break;
>>>  	case F2FS_GOING_DOWN_NEED_FSCK:
>>>  		set_sbi_flag(sbi, SBI_NEED_FSCK);
>>> +		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
>>> +		set_sbi_flag(sbi, SBI_IS_DIRTY);
>>>  		/* do checkpoint only */
>>>  		ret = f2fs_sync_fs(sb, 1);
>>> -		if (ret)
>>> -			goto out;
>>> -		break;
>>> +		goto out;
>>>  	default:
>>>  		ret = -EINVAL;
>>>  		goto out;
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 5b2b9be6f28d..342b720fb4db 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -868,6 +868,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi)
>>>  
>>>  	if (holes[DATA] > ovp || holes[NODE] > ovp)
>>>  		return -EAGAIN;
>>> +	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
>>> +		dirty_segments(sbi) > overprovision_segments(sbi))
>>> +		return -EAGAIN;
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 24efd76ca151..5e1f8573a17f 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -3205,6 +3205,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>  
>>>  	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_QUOTA_NEED_FSCK_FLAG))
>>>  		set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
>>> +	if (__is_set_ckpt_flags(F2FS_CKPT(sbi), CP_DISABLED_QUICK_FLAG)) {
>>> +		set_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
>>> +		sbi->interval_time[DISABLE_TIME] = DEF_DISABLE_QUICK_INTERVAL;
>>> +	}
>>>  
>>>  	/* Initialize device list */
>>>  	err = f2fs_scan_devices(sbi);
>>> @@ -3392,6 +3396,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>  				cur_cp_version(F2FS_CKPT(sbi)));
>>>  	f2fs_update_time(sbi, CP_TIME);
>>>  	f2fs_update_time(sbi, REQ_TIME);
>>> +	clear_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
>>>  	return 0;
>>>  
>>>  free_meta:
>>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
>>> index d7711048ef93..d6befe1f9dc7 100644
>>> --- a/include/linux/f2fs_fs.h
>>> +++ b/include/linux/f2fs_fs.h
>>> @@ -116,6 +116,7 @@ struct f2fs_super_block {
>>>  /*
>>>   * For checkpoint
>>>   */
>>> +#define CP_DISABLED_QUICK_FLAG		0x00002000
>>>  #define CP_DISABLED_FLAG		0x00001000
>>>  #define CP_QUOTA_NEED_FSCK_FLAG		0x00000800
>>>  #define CP_LARGE_NAT_BITMAP_FLAG	0x00000400
>>>
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> .
> 

Re: [f2fs-dev] [PATCH 6/7] f2fs: flush quota blocks after turnning it off

[Chao Yu]

On 2019/2/5 0:59, Jaegeuk Kim wrote:
> On 02/02, Chao Yu wrote:
>> On 2019-1-29 7:47, Jaegeuk Kim wrote:
>>> After quota_off, we'll get some dirty blocks. If put_super don't have a chance
>>> to flush them by checkpoint, it causes NULL pointer exception in end_io after
>>> iput(node_inode). (e.g., by checkpoint=disable)
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>>  fs/f2fs/super.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 5e1f8573a17f..7694cb350734 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -2026,6 +2026,12 @@ void f2fs_quota_off_umount(struct super_block *sb)
>>>  			set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
>>>  		}
>>>  	}
>>> +	/*
>>> +	 * In case of checkpoint=disable, we must flush quota blocks.
>>
>> checkpoint=disable is one of the cases, right? e.g. IO error can be another case?
>>
>> So here I guess we need to change the comments a bit for that.
> 
> I didn't test other cases, but checkpoint=disable could give real errors through
> some tests. So, basically, I'd like to keep known cases only here.

Alright, not a big deal. :)

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> 
>>
>> Thanks,
>>
>>> +	 * This can cause NULL exception for node_inode in end_io, since
>>> +	 * put_super already dropped it.
>>> +	 */
>>> +	sync_filesystem(sb);
>>>  }
>>>  
>>>  static void f2fs_truncate_quota_inode_pages(struct super_block *sb)
>>>
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
>