Messages in this thread | | | Subject | Re: [PATCH V3] f2fs: Avoid double lock for cp_rwsem during checkpoint | From | Sayali Lokhande <> | Date | Thu, 30 Apr 2020 15:36:42 +0530 |
| |
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 <sayalil@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; >> } >> >> /* >>
| |