[f2fs-dev] f2fs: return true if all cmd were issued or no cmd need to be issued for f2fs_issue_discard_timeout()

Commit Message

李扬韬 Jan. 12, 2023, 7:14 p.m. UTC

f2fs_issue_discard_timeout() returns whether discard cmds are dropped,
which does not match the meaning of the function. Let's change it to
return whether all discard cmd are issued.

After commit 4d67490498ac ("f2fs: Don't create discard thread when
device doesn't support realtime discard"), f2fs_issue_discard_timeout()
is alse called by f2fs_remount(). Since the comments of
f2fs_issue_discard_timeout() doesn't make much sense, let's update it.

Signed-off-by: Yangtao Li <[e-mail]>
---
 fs/f2fs/segment.c | 13 ++++++++++---
 fs/f2fs/super.c   |  7 +++----
 2 files changed, 13 insertions(+), 7 deletions(-)

Comments

[e-mail] Jan. 30, 2023, 11 p.m. UTC | #1

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <[e-mail]>:

On Fri, 13 Jan 2023 03:14:04 +0800 you wrote:
> f2fs_issue_discard_timeout() returns whether discard cmds are dropped,
> which does not match the meaning of the function. Let's change it to
> return whether all discard cmd are issued.
> 
> After commit 4d67490498ac ("f2fs: Don't create discard thread when
> device doesn't support realtime discard"), f2fs_issue_discard_timeout()
> is alse called by f2fs_remount(). Since the comments of
> f2fs_issue_discard_timeout() doesn't make much sense, let's update it.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev] f2fs: return true if all cmd were issued or no cmd need to be issued for f2fs_issue_discard_timeout()
    https://git.kernel.org/jaegeuk/f2fs/c/255699a4d876

You are awesome, thank you!

Patch

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index bd1cd98fa6eb..9346209b7c94 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1650,7 +1650,14 @@  void f2fs_stop_discard_thread(struct f2fs_sb_info *sbi)
 	}
 }
 
-/* This comes from f2fs_put_super */
+/**
+ * f2fs_issue_discard_timeout() - Issue all discard cmd within UMOUNT_DISCARD_TIMEOUT
+ * @sbi: the f2fs_sb_info data for discard cmd to issue
+ *
+ * When UMOUNT_DISCARD_TIMEOUT is exceeded, all remaining discard commands will be dropped
+ *
+ * Return true if issued all discard cmd or no discard cmd need issue, otherwise return false.
+ */
 bool f2fs_issue_discard_timeout(struct f2fs_sb_info *sbi)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
@@ -1658,7 +1665,7 @@  bool f2fs_issue_discard_timeout(struct f2fs_sb_info *sbi)
 	bool dropped;
 
 	if (!atomic_read(&dcc->discard_cmd_cnt))
-		return false;
+		return true;
 
 	__init_discard_policy(sbi, &dpolicy, DPOLICY_UMOUNT,
 					dcc->discard_granularity);
@@ -1669,7 +1676,7 @@  bool f2fs_issue_discard_timeout(struct f2fs_sb_info *sbi)
 	__wait_all_discard_cmd(sbi, NULL);
 
 	f2fs_bug_on(sbi, atomic_read(&dcc->discard_cmd_cnt));
-	return dropped;
+	return !dropped;
 }
 
 static int issue_discard_thread(void *data)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 5fc83771042d..8f13798058f4 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1546,7 +1546,7 @@  static void f2fs_put_super(struct super_block *sb)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(sb);
 	int i;
-	bool dropped;
+	bool done;
 
 	/* unregister procfs/sysfs entries in advance to avoid race case */
 	f2fs_unregister_sysfs(sbi);
@@ -1576,9 +1576,8 @@  static void f2fs_put_super(struct super_block *sb)
 	}
 
 	/* be sure to wait for any on-going discard commands */
-	dropped = f2fs_issue_discard_timeout(sbi);
-
-	if (f2fs_realtime_discard_enable(sbi) && !sbi->discard_blks && !dropped) {
+	done = f2fs_issue_discard_timeout(sbi);
+	if (f2fs_realtime_discard_enable(sbi) && !sbi->discard_blks && done) {
 		struct cp_control cpc = {
 			.reason = CP_UMOUNT | CP_TRIMMED,
 		};