[PATCH -next v3] ext4: test if inode's all dirty pages are submitted to disk

[PATCH -next v3] ext4: test if inode's all dirty pages are submitted to disk

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

The commit aa373cf55099 ("writeback: stop background/kupdate works from
livelocking other works") introduced an issue where unmounting a filesystem
in a multi-logical-partition scenario could lead to batch file data loss.
This problem was not fixed until the commit d92109891f21 ("fs/writeback:
bail out if there is no more inodes for IO and queued once"). It took
considerable time to identify the root cause. Additionally, in actual
production environments, we frequently encountered file data loss after
normal system reboots. Therefore, we are adding a check in the inode
release flow to verify whether all dirty pages have been flushed to disk,
in order to determine whether the data loss is caused by a logic issue in
the filesystem code.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/inode.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 396dc3a5d16b..d4d65593bce2 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -184,6 +184,14 @@ void ext4_evict_inode(struct inode *inode)
 	if (EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)
 		ext4_evict_ea_inode(inode);
 	if (inode->i_nlink) {
+		/*
+		 * If there's dirty page will lead to data loss, user
+		 * could see stale data.
+		 */
+		if (unlikely(!ext4_emergency_state(inode->i_sb) &&
+		    mapping_tagged(&inode->i_data, PAGECACHE_TAG_DIRTY)))
+			ext4_warning_inode(inode, "data will be lost");
+
 		truncate_inode_pages_final(&inode->i_data);
 
 		goto no_delete;
-- 
2.34.1

Re: [PATCH -next v3] ext4: test if inode's all dirty pages are submitted to disk

[Jan Kara]

On Tue 03-03-26 09:22:42, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> The commit aa373cf55099 ("writeback: stop background/kupdate works from
> livelocking other works") introduced an issue where unmounting a filesystem
> in a multi-logical-partition scenario could lead to batch file data loss.
> This problem was not fixed until the commit d92109891f21 ("fs/writeback:
> bail out if there is no more inodes for IO and queued once"). It took
> considerable time to identify the root cause. Additionally, in actual
> production environments, we frequently encountered file data loss after
> normal system reboots. Therefore, we are adding a check in the inode
> release flow to verify whether all dirty pages have been flushed to disk,
> in order to determine whether the data loss is caused by a logic issue in
> the filesystem code.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

OK, I guess the warning is better than the stacktrace. BTW, what did
trigger the false positive warnings this time (I didn't really look at
syzbot reproducers)? Anyway, feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/inode.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 396dc3a5d16b..d4d65593bce2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -184,6 +184,14 @@ void ext4_evict_inode(struct inode *inode)
>  	if (EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)
>  		ext4_evict_ea_inode(inode);
>  	if (inode->i_nlink) {
> +		/*
> +		 * If there's dirty page will lead to data loss, user
> +		 * could see stale data.
> +		 */
> +		if (unlikely(!ext4_emergency_state(inode->i_sb) &&
> +		    mapping_tagged(&inode->i_data, PAGECACHE_TAG_DIRTY)))
> +			ext4_warning_inode(inode, "data will be lost");
> +
>  		truncate_inode_pages_final(&inode->i_data);
>  
>  		goto no_delete;
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH -next v3] ext4: test if inode's all dirty pages are submitted to disk

[Theodore Ts'o]

On Tue, 03 Mar 2026 09:22:42 +0800, Ye Bin wrote:
> The commit aa373cf55099 ("writeback: stop background/kupdate works from
> livelocking other works") introduced an issue where unmounting a filesystem
> in a multi-logical-partition scenario could lead to batch file data loss.
> This problem was not fixed until the commit d92109891f21 ("fs/writeback:
> bail out if there is no more inodes for IO and queued once"). It took
> considerable time to identify the root cause. Additionally, in actual
> production environments, we frequently encountered file data loss after
> normal system reboots. Therefore, we are adding a check in the inode
> release flow to verify whether all dirty pages have been flushed to disk,
> in order to determine whether the data loss is caused by a logic issue in
> the filesystem code.
> 
> [...]

Applied, thanks!

[1/1] ext4: test if inode's all dirty pages are submitted to disk
      commit: 7d7ddac60e99f0506ad0dc6ce9138e0c8f9cee94

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>