From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A198A25782A for ; Tue, 20 Jan 2026 04:00:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768881658; cv=none; b=NzI/xhHQy7m5NzcsUJrsnNIUaYaxr8A+XTvZB0HH9ulM/VrJ6ZmkgY5HMqaOf9181Ceme4aYeyD3qXIkUmhXBXx+iDqsvq6S3tDUU57Ju7W2CcpBo/H8qqo33A91mUkqvz0bIxSJ3d1h9Fp12FVn28XZmlCKFXwSo4ELCpiJZ+c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768881658; c=relaxed/simple; bh=YauNuqIweUUzHBAG/wxUTwg1vMjI3lqsPMp36RAUaiU=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=p3lVea42xg/+P0w8OYIq9gAbxrjgbWwNYXW1momyyirItjKj9q9dxp4Gbbd9fTaoa+7FObpQ92eAYoqKk6O1UvtfkmyJoZWTQVMGoKRP6qYOitJRzMM2tacHTQwOpbni4VEsg9epYSlRoWT2PVMW+jbW02lubYQaK22FdUtgQsk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=JOogOcwH; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="JOogOcwH" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2B1A5C16AAE; Tue, 20 Jan 2026 04:00:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1768881658; bh=YauNuqIweUUzHBAG/wxUTwg1vMjI3lqsPMp36RAUaiU=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=JOogOcwHYxKDN6t4zVnrKgbWosuUeYhad+mHGRgcN/INhK+XgdOEKnLHjLP7elTar 1W3vMdXACW+W/2k6Y1SjNA1d110hLQrkW419wfbnsqMmO+OPwLyv0SgDesLuE2WAJi 2aYXlU/mMw+yUJNAwJJGJLhWwx0nRgbaNpC+kP/ksxCmw9lHysu8xDkWIj7CHFyO+4 9cuGDLZVrgM/1jFEHWDgR++pBTICM+A4BGE2aPPXPQq8+H4+7dinfQtnJlIuuMfLbS 0aPjnxiGBWvjmVhhmVn+JoY4PXWFcUokxhvBQcpM8u728bKYSH/OPuA6GypgV8ktQY GMGfKzjle1P4Q== Message-ID: <18070a5f-6fdf-4063-ab99-86b8641bcc97@kernel.org> Date: Tue, 20 Jan 2026 12:00:54 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: chao@kernel.org, jaegeuk@kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: check skipped write in f2fs_enable_checkpoint() To: Zhiguo Niu References: <20260119133230.16481-1-chao@kernel.org> Content-Language: en-US From: Chao Yu In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 1/20/2026 7:47 AM, Zhiguo Niu wrote: > Chao Yu via Linux-f2fs-devel > 于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 >> --- >> 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