On 4/30/2020 6:41 AM, Chao Yu wrote:
On 2020/4/30 1:00, Sayali Lokhande wrote:There could be a scenario where f2fs_sync_node_pages gets called during checkpoint, which in turn tries to flush inline data and calls iput(). This results in deadlock as iput() tries to hold cp_rwsem, which is already held at the beginning by checkpoint->block_operations(). Call stack : Thread A Thread B f2fs_write_checkpoint() - block_operations(sbi) - f2fs_lock_all(sbi); - down_write(&sbi->cp_rwsem); - open() - igrab() - write() write inline data - unlink() - f2fs_sync_node_pages() - if (is_inline_node(page)) - flush_inline_data() - ilookup() page = f2fs_pagecache_get_page() if (!page) goto iput_out; iput_out: -close() -iput() iput(inode); - f2fs_evict_inode() - f2fs_truncate_blocks() - f2fs_lock_op() - down_read(&sbi->cp_rwsem); Fixes: 399368372ed9 ("f2fs: introduce a new global lock scheme")IMO, it should be 2049d4fcb057 ("f2fs: avoid multiple node page writes due to inline_data") It brings iput() to checkpoint process for the first time. Thanks,
Agreed. will update it.
Signed-off-by: Sayali Lokhande <saya...@codeaurora.org> --- fs/f2fs/checkpoint.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 5ba649e..97b6378 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -1219,21 +1219,19 @@ static int block_operations(struct f2fs_sb_info *sbi) goto retry_flush_quotas; }-retry_flush_nodes:down_write(&sbi->node_write);if (get_pages(sbi, F2FS_DIRTY_NODES)) {up_write(&sbi->node_write); + up_write(&sbi->node_change); + f2fs_unlock_all(sbi); atomic_inc(&sbi->wb_sync_req[NODE]); err = f2fs_sync_node_pages(sbi, &wbc, false, FS_CP_NODE_IO); atomic_dec(&sbi->wb_sync_req[NODE]); - if (err) { - up_write(&sbi->node_change); - f2fs_unlock_all(sbi); + if (err) goto out; - } cond_resched(); - goto retry_flush_nodes; + goto retry_flush_quotas; }/*
