[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHJ8P3JebPryFL80s+x0gT5JDeaGGpq3MgjLyJATwuNyC2NM7w@mail.gmail.com>
Date: Tue, 20 Jan 2026 07:47:45 +0800
From: Zhiguo Niu <niuzhiguo84@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: jaegeuk@...nel.org, linux-kernel@...r.kernel.org,
linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: check skipped write in f2fs_enable_checkpoint()
Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@...ts.sourceforge.net>
于2026年1月19日周一 22:48写道:
>
> This patch introduces sbi->nr_pages[F2FS_SKIPPED_WRITE] to record any
> skipped write during data flush in f2fs_enable_checkpoint().
>
> So in the loop of data flush, if there is any skipped write in previous
> flush, let's retry sync_inode_sb(), otherwise, all dirty data written
> before f2fs_enable_checkpoint() should have been persisted, then break
> the retry loop.
>
> Signed-off-by: Chao Yu <chao@...nel.org>
> ---
> Changelog:
> - code is based on 'Revert "f2fs: add timeout in f2fs_enable_checkpoint()"'
> fs/f2fs/data.c | 12 ++++++++++++
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/super.c | 37 +++++++++++++++++++++++++++++++++----
> 3 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 5b4832956196..00108d5881aa 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -3500,6 +3500,15 @@ static inline void account_writeback(struct inode *inode, bool inc)
> f2fs_up_read(&F2FS_I(inode)->i_sem);
> }
>
> +static inline void update_skipped_write(struct f2fs_sb_info *sbi,
> + struct writeback_control *wbc)
> +{
> + long skipped = wbc->pages_skipped;
> +
> + if (skipped && is_sbi_flag_set(sbi, SBI_ENABLE_CHECKPOINT))
> + atomic_add(skipped, &sbi->nr_pages[F2FS_SKIPPED_WRITE]);
> +}
> +
> static int __f2fs_write_data_pages(struct address_space *mapping,
> struct writeback_control *wbc,
> enum iostat_type io_type)
> @@ -3564,10 +3573,13 @@ static int __f2fs_write_data_pages(struct address_space *mapping,
> */
>
> f2fs_remove_dirty_inode(inode);
> +
> + update_skipped_write(sbi, wbc);
Hi Chao,
Sorry, why do we need to update skipped write here as well?
thanks!
> return ret;
>
> skip_write:
> wbc->pages_skipped += get_dirty_pages(inode);
> + update_skipped_write(sbi, wbc);
> trace_f2fs_writepages(mapping->host, wbc, DATA);
> return 0;
> }
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 035239758e33..52cec6b3ecf0 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1238,6 +1238,7 @@ enum count_type {
> F2FS_RD_META,
> F2FS_DIO_WRITE,
> F2FS_DIO_READ,
> + F2FS_SKIPPED_WRITE, /* skip or fail during f2fs_enable_checkpoint() */
> NR_COUNT_TYPE,
> };
>
> @@ -1476,6 +1477,7 @@ enum {
> SBI_IS_RESIZEFS, /* resizefs is in process */
> SBI_IS_FREEZING, /* freezefs is in process */
> SBI_IS_WRITABLE, /* remove ro mountoption transiently */
> + SBI_ENABLE_CHECKPOINT, /* indicate it's during f2fs_enable_checkpoint() */
> MAX_SBI_FLAG,
> };
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 97c2264ec7fe..0afe9f829058 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2690,6 +2690,7 @@ static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
> long long start, writeback, end;
> int ret;
> struct f2fs_lock_context lc;
> + long long skipped_write, dirty_data;
>
> f2fs_info(sbi, "f2fs_enable_checkpoint() starts, meta: %lld, node: %lld, data: %lld",
> get_pages(sbi, F2FS_DIRTY_META),
> @@ -2698,17 +2699,45 @@ static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
>
> start = ktime_get();
>
> + set_sbi_flag(sbi, SBI_ENABLE_CHECKPOINT);
> +
> /* we should flush all the data to keep data consistency */
> do {
> + skipped_write = get_pages(sbi, F2FS_SKIPPED_WRITE);
> + dirty_data = get_pages(sbi, F2FS_DIRTY_DATA);
> +
> sync_inodes_sb(sbi->sb);
> f2fs_io_schedule_timeout(DEFAULT_SCHEDULE_TIMEOUT);
> - } while (get_pages(sbi, F2FS_DIRTY_DATA) && retry--);
> +
> + f2fs_info(sbi, "sync_inode_sb done, dirty_data: %lld, %lld, "
> + "skipped write: %lld, %lld, retry: %d",
> + get_pages(sbi, F2FS_DIRTY_DATA),
> + dirty_data,
> + get_pages(sbi, F2FS_SKIPPED_WRITE),
> + skipped_write, retry);
> +
> + /*
> + * sync_inodes_sb() has retry logic, so let's check dirty_data
> + * in prior to skipped_write in case there is no dirty data.
> + */
> + if (!get_pages(sbi, F2FS_DIRTY_DATA))
> + break;
> + if (get_pages(sbi, F2FS_SKIPPED_WRITE) == skipped_write)
> + break;
> + } while (retry--);
> +
> + clear_sbi_flag(sbi, SBI_ENABLE_CHECKPOINT);
>
> writeback = ktime_get();
>
> - if (unlikely(get_pages(sbi, F2FS_DIRTY_DATA)))
> - f2fs_warn(sbi, "checkpoint=enable has some unwritten data: %lld",
> - get_pages(sbi, F2FS_DIRTY_DATA));
> + if (unlikely(get_pages(sbi, F2FS_DIRTY_DATA) ||
> + get_pages(sbi, F2FS_SKIPPED_WRITE)))
> + f2fs_warn(sbi, "checkpoint=enable unwritten data: %lld, skipped data: %lld, retry: %d",
> + get_pages(sbi, F2FS_DIRTY_DATA),
> + get_pages(sbi, F2FS_SKIPPED_WRITE), retry);
> +
> + if (get_pages(sbi, F2FS_SKIPPED_WRITE))
> + atomic_set(&sbi->nr_pages[F2FS_SKIPPED_WRITE], 0);
>
> f2fs_down_write_trace(&sbi->gc_lock, &lc);
> f2fs_dirty_to_prefree(sbi);
> --
> 2.40.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Powered by blists - more mailing lists