| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <18070a5f-6fdf-4063-ab99-86b8641bcc97@kernel.org>
Date: Tue, 20 Jan 2026 12:00:54 +0800
From: Chao Yu <chao@...nel.org>
To: Zhiguo Niu <niuzhiguo84@...il.com>
Cc: chao@...nel.org, 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()
On 1/20/2026 7:47 AM, Zhiguo Niu wrote:
> 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?
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@...ts.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Powered by blists - more mailing lists