* [PATCH] f2fs: finish previous checkpoints before returning from remount
@ 2023-10-20 16:36 ` Daeho Jeong
0 siblings, 0 replies; 4+ messages in thread
From: Daeho Jeong @ 2023-10-20 16:36 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong
From: Daeho Jeong <daehojeong@google.com>
Flush remaining checkpoint requests at the end of remount, since a new
checkpoint would be triggered while remount and we need to take care of
it before returning from remount, in order to avoid the below race
condition.
- Thread - checkpoint thread
do_remount()
down_write(&sb->s_umount);
f2fs_remount()
f2fs_disable_checkpoint(sbi) -> add checkpoints to the list
block_operations()
down_read_trylock(&sb->s_umount) = 0
up_write(&sb->s_umount);
f2fs_quota_sync()
dquot_writeback_dquots()
WARN_ON_ONCE(!rwsem_is_locked(&sb->s_umount));
Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
fs/f2fs/super.c | 59 +++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a8c8232852bb..d6edf7df6abd 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2286,9 +2286,9 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
unsigned long old_sb_flags;
int err;
bool need_restart_gc = false, need_stop_gc = false;
- bool need_restart_ckpt = false, need_stop_ckpt = false;
bool need_restart_flush = false, need_stop_flush = false;
bool need_restart_discard = false, need_stop_discard = false;
+ bool need_enable_checkpoint = false, need_disable_checkpoint = false;
bool no_read_extent_cache = !test_opt(sbi, READ_EXTENT_CACHE);
bool no_age_extent_cache = !test_opt(sbi, AGE_EXTENT_CACHE);
bool enable_checkpoint = !test_opt(sbi, DISABLE_CHECKPOINT);
@@ -2452,24 +2452,6 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
clear_sbi_flag(sbi, SBI_IS_CLOSE);
}
- if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) ||
- !test_opt(sbi, MERGE_CHECKPOINT)) {
- f2fs_stop_ckpt_thread(sbi);
- need_restart_ckpt = true;
- } else {
- /* Flush if the prevous checkpoint, if exists. */
- f2fs_flush_ckpt_thread(sbi);
-
- err = f2fs_start_ckpt_thread(sbi);
- if (err) {
- f2fs_err(sbi,
- "Failed to start F2FS issue_checkpoint_thread (%d)",
- err);
- goto restore_gc;
- }
- need_stop_ckpt = true;
- }
-
/*
* We stop issue flush thread if FS is mounted as RO
* or if flush_merge is not passed in mount option.
@@ -2481,7 +2463,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
} else {
err = f2fs_create_flush_cmd_control(sbi);
if (err)
- goto restore_ckpt;
+ goto restore_gc;
need_stop_flush = true;
}
@@ -2503,8 +2485,31 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
err = f2fs_disable_checkpoint(sbi);
if (err)
goto restore_discard;
+ need_enable_checkpoint = true;
} else {
f2fs_enable_checkpoint(sbi);
+ need_disable_checkpoint = true;
+ }
+ }
+
+ /*
+ * Place this routine at the end, since a new checkpoint would be
+ * triggered while remount and we need to take care of it before
+ * returning from remount.
+ */
+ if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) ||
+ !test_opt(sbi, MERGE_CHECKPOINT)) {
+ f2fs_stop_ckpt_thread(sbi);
+ } else {
+ /* Flush if the prevous checkpoint, if exists. */
+ f2fs_flush_ckpt_thread(sbi);
+
+ err = f2fs_start_ckpt_thread(sbi);
+ if (err) {
+ f2fs_err(sbi,
+ "Failed to start F2FS issue_checkpoint_thread (%d)",
+ err);
+ goto restore_checkpoint;
}
}
@@ -2522,6 +2527,13 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
adjust_unusable_cap_perc(sbi);
*flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
return 0;
+restore_checkpoint:
+ if (need_enable_checkpoint) {
+ f2fs_enable_checkpoint(sbi);
+ } else if (need_disable_checkpoint) {
+ if (f2fs_disable_checkpoint(sbi))
+ f2fs_warn(sbi, "checkpoint has not been disabled");
+ }
restore_discard:
if (need_restart_discard) {
if (f2fs_start_discard_thread(sbi))
@@ -2537,13 +2549,6 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
clear_opt(sbi, FLUSH_MERGE);
f2fs_destroy_flush_cmd_control(sbi, false);
}
-restore_ckpt:
- if (need_restart_ckpt) {
- if (f2fs_start_ckpt_thread(sbi))
- f2fs_warn(sbi, "background ckpt thread has stopped");
- } else if (need_stop_ckpt) {
- f2fs_stop_ckpt_thread(sbi);
- }
restore_gc:
if (need_restart_gc) {
if (f2fs_start_gc_thread(sbi))
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [f2fs-dev] [PATCH] f2fs: finish previous checkpoints before returning from remount
@ 2023-10-20 16:36 ` Daeho Jeong
0 siblings, 0 replies; 4+ messages in thread
From: Daeho Jeong @ 2023-10-20 16:36 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong
From: Daeho Jeong <daehojeong@google.com>
Flush remaining checkpoint requests at the end of remount, since a new
checkpoint would be triggered while remount and we need to take care of
it before returning from remount, in order to avoid the below race
condition.
- Thread - checkpoint thread
do_remount()
down_write(&sb->s_umount);
f2fs_remount()
f2fs_disable_checkpoint(sbi) -> add checkpoints to the list
block_operations()
down_read_trylock(&sb->s_umount) = 0
up_write(&sb->s_umount);
f2fs_quota_sync()
dquot_writeback_dquots()
WARN_ON_ONCE(!rwsem_is_locked(&sb->s_umount));
Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
fs/f2fs/super.c | 59 +++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a8c8232852bb..d6edf7df6abd 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2286,9 +2286,9 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
unsigned long old_sb_flags;
int err;
bool need_restart_gc = false, need_stop_gc = false;
- bool need_restart_ckpt = false, need_stop_ckpt = false;
bool need_restart_flush = false, need_stop_flush = false;
bool need_restart_discard = false, need_stop_discard = false;
+ bool need_enable_checkpoint = false, need_disable_checkpoint = false;
bool no_read_extent_cache = !test_opt(sbi, READ_EXTENT_CACHE);
bool no_age_extent_cache = !test_opt(sbi, AGE_EXTENT_CACHE);
bool enable_checkpoint = !test_opt(sbi, DISABLE_CHECKPOINT);
@@ -2452,24 +2452,6 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
clear_sbi_flag(sbi, SBI_IS_CLOSE);
}
- if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) ||
- !test_opt(sbi, MERGE_CHECKPOINT)) {
- f2fs_stop_ckpt_thread(sbi);
- need_restart_ckpt = true;
- } else {
- /* Flush if the prevous checkpoint, if exists. */
- f2fs_flush_ckpt_thread(sbi);
-
- err = f2fs_start_ckpt_thread(sbi);
- if (err) {
- f2fs_err(sbi,
- "Failed to start F2FS issue_checkpoint_thread (%d)",
- err);
- goto restore_gc;
- }
- need_stop_ckpt = true;
- }
-
/*
* We stop issue flush thread if FS is mounted as RO
* or if flush_merge is not passed in mount option.
@@ -2481,7 +2463,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
} else {
err = f2fs_create_flush_cmd_control(sbi);
if (err)
- goto restore_ckpt;
+ goto restore_gc;
need_stop_flush = true;
}
@@ -2503,8 +2485,31 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
err = f2fs_disable_checkpoint(sbi);
if (err)
goto restore_discard;
+ need_enable_checkpoint = true;
} else {
f2fs_enable_checkpoint(sbi);
+ need_disable_checkpoint = true;
+ }
+ }
+
+ /*
+ * Place this routine at the end, since a new checkpoint would be
+ * triggered while remount and we need to take care of it before
+ * returning from remount.
+ */
+ if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) ||
+ !test_opt(sbi, MERGE_CHECKPOINT)) {
+ f2fs_stop_ckpt_thread(sbi);
+ } else {
+ /* Flush if the prevous checkpoint, if exists. */
+ f2fs_flush_ckpt_thread(sbi);
+
+ err = f2fs_start_ckpt_thread(sbi);
+ if (err) {
+ f2fs_err(sbi,
+ "Failed to start F2FS issue_checkpoint_thread (%d)",
+ err);
+ goto restore_checkpoint;
}
}
@@ -2522,6 +2527,13 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
adjust_unusable_cap_perc(sbi);
*flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
return 0;
+restore_checkpoint:
+ if (need_enable_checkpoint) {
+ f2fs_enable_checkpoint(sbi);
+ } else if (need_disable_checkpoint) {
+ if (f2fs_disable_checkpoint(sbi))
+ f2fs_warn(sbi, "checkpoint has not been disabled");
+ }
restore_discard:
if (need_restart_discard) {
if (f2fs_start_discard_thread(sbi))
@@ -2537,13 +2549,6 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
clear_opt(sbi, FLUSH_MERGE);
f2fs_destroy_flush_cmd_control(sbi, false);
}
-restore_ckpt:
- if (need_restart_ckpt) {
- if (f2fs_start_ckpt_thread(sbi))
- f2fs_warn(sbi, "background ckpt thread has stopped");
- } else if (need_stop_ckpt) {
- f2fs_stop_ckpt_thread(sbi);
- }
restore_gc:
if (need_restart_gc) {
if (f2fs_start_gc_thread(sbi))
--
2.42.0.283.g2d96d420d3-goog
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: finish previous checkpoints before returning from remount
2023-10-20 16:36 ` [f2fs-dev] " Daeho Jeong
@ 2023-10-23 15:30 ` patchwork-bot+f2fs
-1 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+f2fs @ 2023-10-23 15:30 UTC (permalink / raw)
To: Daeho Jeong; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, daehojeong
Hello:
This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Fri, 20 Oct 2023 09:36:45 -0700 you wrote:
> From: Daeho Jeong <daehojeong@google.com>
>
> Flush remaining checkpoint requests at the end of remount, since a new
> checkpoint would be triggered while remount and we need to take care of
> it before returning from remount, in order to avoid the below race
> condition.
>
> [...]
Here is the summary with links:
- [f2fs-dev] f2fs: finish previous checkpoints before returning from remount
https://git.kernel.org/jaegeuk/f2fs/c/1e7bef5f90ed
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: finish previous checkpoints before returning from remount
@ 2023-10-23 15:30 ` patchwork-bot+f2fs
0 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+f2fs @ 2023-10-23 15:30 UTC (permalink / raw)
To: Daeho Jeong; +Cc: daehojeong, kernel-team, linux-kernel, linux-f2fs-devel
Hello:
This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Fri, 20 Oct 2023 09:36:45 -0700 you wrote:
> From: Daeho Jeong <daehojeong@google.com>
>
> Flush remaining checkpoint requests at the end of remount, since a new
> checkpoint would be triggered while remount and we need to take care of
> it before returning from remount, in order to avoid the below race
> condition.
>
> [...]
Here is the summary with links:
- [f2fs-dev] f2fs: finish previous checkpoints before returning from remount
https://git.kernel.org/jaegeuk/f2fs/c/1e7bef5f90ed
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-23 15:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-20 16:36 [PATCH] f2fs: finish previous checkpoints before returning from remount Daeho Jeong
2023-10-20 16:36 ` [f2fs-dev] " Daeho Jeong
2023-10-23 15:30 ` patchwork-bot+f2fs
2023-10-23 15:30 ` patchwork-bot+f2fs
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.