[PATCH] Revert "f2fs: add timeout in f2fs_enable_checkpoint()"

Jaegeuk Kim posted 1 patch 3 weeks ago
fs/f2fs/f2fs.h  |  2 --
fs/f2fs/super.c | 15 ++++-----------
2 files changed, 4 insertions(+), 13 deletions(-)
[PATCH] Revert "f2fs: add timeout in f2fs_enable_checkpoint()"
Posted by Jaegeuk Kim 3 weeks ago
This reverts commit 4bc347779698b5e67e1514bab105c2c083e55502.

For stability, let's keep flushing all the data.

Cc: stable@kernel.org
Fixes: 4bc347779698 (f2fs: add timeout in f2fs_enable_checkpoint()")
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h  |  2 --
 fs/f2fs/super.c | 15 ++++-----------
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 90aa1d53722a..8c256fcdcf5b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -311,7 +311,6 @@ enum {
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
 #define DEF_DISABLE_INTERVAL		5	/* 5 secs */
-#define DEF_ENABLE_INTERVAL		16	/* 16 secs */
 #define DEF_DISABLE_QUICK_INTERVAL	1	/* 1 secs */
 #define DEF_UMOUNT_DISCARD_TIMEOUT	5	/* 5 secs */
 
@@ -1482,7 +1481,6 @@ enum {
 	DISCARD_TIME,
 	GC_TIME,
 	DISABLE_TIME,
-	ENABLE_TIME,
 	UMOUNT_DISCARD_TIMEOUT,
 	MAX_TIME,
 };
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 25f796232ad9..4869145531cc 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2686,7 +2686,7 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 
 static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
 {
-	unsigned int nr_pages = get_pages(sbi, F2FS_DIRTY_DATA) / 16;
+	int retry = DEFAULT_RETRY_IO_COUNT;
 	long long start, writeback, end;
 	int ret;
 	struct f2fs_lock_context lc;
@@ -2696,22 +2696,16 @@ static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
 					get_pages(sbi, F2FS_DIRTY_NODES),
 					get_pages(sbi, F2FS_DIRTY_DATA));
 
-	f2fs_update_time(sbi, ENABLE_TIME);
-
 	start = ktime_get();
 
 	/* we should flush all the data to keep data consistency */
-	while (get_pages(sbi, F2FS_DIRTY_DATA)) {
-		writeback_inodes_sb_nr(sbi->sb, nr_pages, WB_REASON_SYNC);
+	do {
+		sync_inodes_sb(sbi->sb);
 		f2fs_io_schedule_timeout(DEFAULT_SCHEDULE_TIMEOUT);
+	} while (get_pages(sbi, F2FS_DIRTY_DATA) && retry--);
 
-		if (f2fs_time_over(sbi, ENABLE_TIME))
-			break;
-	}
 	writeback = ktime_get();
 
-	sync_inodes_sb(sbi->sb);
-
 	if (unlikely(get_pages(sbi, F2FS_DIRTY_DATA)))
 		f2fs_warn(sbi, "checkpoint=enable has some unwritten data: %lld",
 					get_pages(sbi, F2FS_DIRTY_DATA));
@@ -4335,7 +4329,6 @@ 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[ENABLE_TIME] = DEF_ENABLE_INTERVAL;
 	sbi->interval_time[UMOUNT_DISCARD_TIMEOUT] =
 				DEF_UMOUNT_DISCARD_TIMEOUT;
 	clear_sbi_flag(sbi, SBI_NEED_FSCK);
-- 
2.52.0.457.g6b5491de43-goog
Re: [f2fs-dev] [PATCH] Revert "f2fs: add timeout in f2fs_enable_checkpoint()"
Posted by Chao Yu 2 weeks, 4 days ago
On 1/17/2026 5:50 AM, Jaegeuk Kim via Linux-f2fs-devel wrote:
> This reverts commit 4bc347779698b5e67e1514bab105c2c083e55502.
> 
> For stability, let's keep flushing all the data.
> 
> Cc: stable@kernel.org
> Fixes: 4bc347779698 (f2fs: add timeout in f2fs_enable_checkpoint()")

We don't need to add Cc stable and Fixes line due to there is no regression
issue, meanwhile it needs to change commit message a bit.

Otherwise, it looks good to me.

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/f2fs.h  |  2 --
>   fs/f2fs/super.c | 15 ++++-----------
>   2 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 90aa1d53722a..8c256fcdcf5b 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -311,7 +311,6 @@ enum {
>   #define DEF_CP_INTERVAL			60	/* 60 secs */
>   #define DEF_IDLE_INTERVAL		5	/* 5 secs */
>   #define DEF_DISABLE_INTERVAL		5	/* 5 secs */
> -#define DEF_ENABLE_INTERVAL		16	/* 16 secs */
>   #define DEF_DISABLE_QUICK_INTERVAL	1	/* 1 secs */
>   #define DEF_UMOUNT_DISCARD_TIMEOUT	5	/* 5 secs */
>   
> @@ -1482,7 +1481,6 @@ enum {
>   	DISCARD_TIME,
>   	GC_TIME,
>   	DISABLE_TIME,
> -	ENABLE_TIME,
>   	UMOUNT_DISCARD_TIMEOUT,
>   	MAX_TIME,
>   };
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 25f796232ad9..4869145531cc 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2686,7 +2686,7 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
>   
>   static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
>   {
> -	unsigned int nr_pages = get_pages(sbi, F2FS_DIRTY_DATA) / 16;
> +	int retry = DEFAULT_RETRY_IO_COUNT;
>   	long long start, writeback, end;
>   	int ret;
>   	struct f2fs_lock_context lc;
> @@ -2696,22 +2696,16 @@ static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
>   					get_pages(sbi, F2FS_DIRTY_NODES),
>   					get_pages(sbi, F2FS_DIRTY_DATA));
>   
> -	f2fs_update_time(sbi, ENABLE_TIME);
> -
>   	start = ktime_get();
>   
>   	/* we should flush all the data to keep data consistency */
> -	while (get_pages(sbi, F2FS_DIRTY_DATA)) {
> -		writeback_inodes_sb_nr(sbi->sb, nr_pages, WB_REASON_SYNC);
> +	do {
> +		sync_inodes_sb(sbi->sb);
>   		f2fs_io_schedule_timeout(DEFAULT_SCHEDULE_TIMEOUT);
> +	} while (get_pages(sbi, F2FS_DIRTY_DATA) && retry--);
>   
> -		if (f2fs_time_over(sbi, ENABLE_TIME))
> -			break;
> -	}
>   	writeback = ktime_get();
>   
> -	sync_inodes_sb(sbi->sb);
> -
>   	if (unlikely(get_pages(sbi, F2FS_DIRTY_DATA)))
>   		f2fs_warn(sbi, "checkpoint=enable has some unwritten data: %lld",
>   					get_pages(sbi, F2FS_DIRTY_DATA));
> @@ -4335,7 +4329,6 @@ 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[ENABLE_TIME] = DEF_ENABLE_INTERVAL;
>   	sbi->interval_time[UMOUNT_DISCARD_TIMEOUT] =
>   				DEF_UMOUNT_DISCARD_TIMEOUT;
>   	clear_sbi_flag(sbi, SBI_NEED_FSCK);