Re: [f2fs-dev] [PATCH 1/2] f2fs: check skipped write in f2fs_enable_checkpoint()

[Chao Yu <chao@kernel.org>]

On 1/20/2026 7:47 AM, Zhiguo Niu wrote:
> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.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@kernel.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?

Zhiguo,

In case we increase pages_skipped in below path:

- __f2fs_write_data_pages
  - f2fs_write_cache_pages
   - f2fs_write_single_data_page
    - folio_redirty_for_writepage
    : wbc->pages_skipped += nr;

Thanks,

> 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@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel