[PATCH v2] f2fs: fix livelock in syncing dirty inodes

[PATCH v2] f2fs: fix livelock in syncing dirty inodes

[Daeho Jeong]

From: Daeho Jeong <daehojeong@google.com>

During checkpoint, f2fs_sync_dirty_inodes() and f2fs_sync_inode_meta()
iterate over dirty inodes in their respective lists. If igrab() fails on
an inode (e.g. because it is in the freeing state), the loop continues
without moving the current inode to the tail of the list. As a result,
subsequent iterations pick the same inode repeatedly, preventing other
ready dirty inodes in the list from making forward progress and leading
to a livelock.

Fix this by moving the current inode to the tail of the list
(list_move_tail(&fi->{dirty_list,gdirty_list}, head)) before attempting
igrab() in both f2fs_sync_dirty_inodes() and f2fs_sync_inode_meta().

Additionally, if igrab() fails, yield the CPU with cond_resched() to
allow the evicting thread to finish eviction. Remove the redundant
f2fs_submit_merged_write() call, since .writepages already submits cached
bios via f2fs_submit_merged_write_cond().

v2:
- Also apply list_move_tail() to f2fs_sync_dirty_inodes().
- Remove redundant f2fs_submit_merged_write() calls from both functions,
  keeping only cond_resched().
- Update commit title and description.

Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 fs/f2fs/checkpoint.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index ef22692cef0a..d16988150a8c 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1410,6 +1410,7 @@ int f2fs_sync_dirty_inodes(struct f2fs_sb_info *sbi, enum inode_type type,
 		return 0;
 	}
 	fi = list_first_entry(head, struct f2fs_inode_info, dirty_list);
+	list_move_tail(&fi->dirty_list, head);
 	inode = igrab(&fi->vfs_inode);
 	spin_unlock(&sbi->inode_lock[type]);
 	if (inode) {
@@ -1432,11 +1433,6 @@ int f2fs_sync_dirty_inodes(struct f2fs_sb_info *sbi, enum inode_type type,
 		else
 			ino = cur_ino;
 	} else {
-		/*
-		 * We should submit bio, since it exists several
-		 * writebacking dentry pages in the freeing inode.
-		 */
-		f2fs_submit_merged_write(sbi, DATA);
 		cond_resched();
 	}
 	goto retry;
@@ -1460,6 +1456,7 @@ static int f2fs_sync_inode_meta(struct f2fs_sb_info *sbi)
 		}
 		fi = list_first_entry(head, struct f2fs_inode_info,
 							gdirty_list);
+		list_move_tail(&fi->gdirty_list, head);
 		inode = igrab(&fi->vfs_inode);
 		spin_unlock(&sbi->inode_lock[DIRTY_META]);
 		if (inode) {
@@ -1469,6 +1466,8 @@ static int f2fs_sync_inode_meta(struct f2fs_sb_info *sbi)
 			if (is_inode_flag_set(inode, FI_DIRTY_INODE))
 				f2fs_update_inode_page(inode);
 			iput(inode);
+		} else {
+			cond_resched();
 		}
 	}
 	return 0;
-- 
2.55.0.1003.g10538fe699-goog

Re: [f2fs-dev] [PATCH v2] f2fs: fix livelock in syncing dirty inodes

[Chao Yu]

On 9/10/26 03:06, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> During checkpoint, f2fs_sync_dirty_inodes() and f2fs_sync_inode_meta()
> iterate over dirty inodes in their respective lists. If igrab() fails on
> an inode (e.g. because it is in the freeing state), the loop continues
> without moving the current inode to the tail of the list. As a result,
> subsequent iterations pick the same inode repeatedly, preventing other
> ready dirty inodes in the list from making forward progress and leading
> to a livelock.
> 
> Fix this by moving the current inode to the tail of the list
> (list_move_tail(&fi->{dirty_list,gdirty_list}, head)) before attempting
> igrab() in both f2fs_sync_dirty_inodes() and f2fs_sync_inode_meta().
> 
> Additionally, if igrab() fails, yield the CPU with cond_resched() to
> allow the evicting thread to finish eviction. Remove the redundant
> f2fs_submit_merged_write() call, since .writepages already submits cached
> bios via f2fs_submit_merged_write_cond().
> 
> v2:
> - Also apply list_move_tail() to f2fs_sync_dirty_inodes().
> - Remove redundant f2fs_submit_merged_write() calls from both functions,
>   keeping only cond_resched().
> - Update commit title and description.
> 
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,