Skip to content

Commit 2d586a3

Browse files
chaseyugregkh
authored andcommitted
f2fs: quota: fix potential deadlock
[ Upstream commit 9de71ed ] xfstest generic/587 reports a deadlock issue as below: ====================================================== WARNING: possible circular locking dependency detected 5.14.0-rc1 Freescale#69 Not tainted ------------------------------------------------------ repquota/8606 is trying to acquire lock: ffff888022ac9320 (&sb->s_type->i_mutex_key#18){+.+.}-{3:3}, at: f2fs_quota_sync+0x207/0x300 [f2fs] but task is already holding lock: ffff8880084bcde8 (&sbi->quota_sem){.+.+}-{3:3}, at: f2fs_quota_sync+0x59/0x300 [f2fs] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> Freescale#2 (&sbi->quota_sem){.+.+}-{3:3}: __lock_acquire+0x648/0x10b0 lock_acquire+0x128/0x470 down_read+0x3b/0x2a0 f2fs_quota_sync+0x59/0x300 [f2fs] f2fs_quota_on+0x48/0x100 [f2fs] do_quotactl+0x5e3/0xb30 __x64_sys_quotactl+0x23a/0x4e0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae -> Freescale#1 (&sbi->cp_rwsem){++++}-{3:3}: __lock_acquire+0x648/0x10b0 lock_acquire+0x128/0x470 down_read+0x3b/0x2a0 f2fs_unlink+0x353/0x670 [f2fs] vfs_unlink+0x1c7/0x380 do_unlinkat+0x413/0x4b0 __x64_sys_unlinkat+0x50/0xb0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae -> #0 (&sb->s_type->i_mutex_key#18){+.+.}-{3:3}: check_prev_add+0xdc/0xb30 validate_chain+0xa67/0xb20 __lock_acquire+0x648/0x10b0 lock_acquire+0x128/0x470 down_write+0x39/0xc0 f2fs_quota_sync+0x207/0x300 [f2fs] do_quotactl+0xaff/0xb30 __x64_sys_quotactl+0x23a/0x4e0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae other info that might help us debug this: Chain exists of: &sb->s_type->i_mutex_key#18 --> &sbi->cp_rwsem --> &sbi->quota_sem Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&sbi->quota_sem); lock(&sbi->cp_rwsem); lock(&sbi->quota_sem); lock(&sb->s_type->i_mutex_key#18); *** DEADLOCK *** 3 locks held by repquota/8606: #0: ffff88801efac0e0 (&type->s_umount_key#53){++++}-{3:3}, at: user_get_super+0xd9/0x190 Freescale#1: ffff8880084bc380 (&sbi->cp_rwsem){++++}-{3:3}, at: f2fs_quota_sync+0x3e/0x300 [f2fs] Freescale#2: ffff8880084bcde8 (&sbi->quota_sem){.+.+}-{3:3}, at: f2fs_quota_sync+0x59/0x300 [f2fs] stack backtrace: CPU: 6 PID: 8606 Comm: repquota Not tainted 5.14.0-rc1 Freescale#69 Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 Call Trace: dump_stack_lvl+0xce/0x134 dump_stack+0x17/0x20 print_circular_bug.isra.0.cold+0x239/0x253 check_noncircular+0x1be/0x1f0 check_prev_add+0xdc/0xb30 validate_chain+0xa67/0xb20 __lock_acquire+0x648/0x10b0 lock_acquire+0x128/0x470 down_write+0x39/0xc0 f2fs_quota_sync+0x207/0x300 [f2fs] do_quotactl+0xaff/0xb30 __x64_sys_quotactl+0x23a/0x4e0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x7f883b0b4efe The root cause is ABBA deadlock of inode lock and cp_rwsem, reorder locks in f2fs_quota_sync() as below to fix this issue: - lock inode - lock cp_rwsem - lock quota_sem Fixes: db6ec53 ("f2fs: add a rw_sem to cover quota flag changes") Signed-off-by: Chao Yu <[email protected]> Signed-off-by: Jaegeuk Kim <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent 70fd936 commit 2d586a3

File tree

1 file changed

+48
-36
lines changed

1 file changed

+48
-36
lines changed

fs/f2fs/super.c

+48-36
Original file line numberDiff line numberDiff line change
@@ -2206,64 +2206,76 @@ static int f2fs_enable_quotas(struct super_block *sb)
22062206
return 0;
22072207
}
22082208

2209+
static int f2fs_quota_sync_file(struct f2fs_sb_info *sbi, int type)
2210+
{
2211+
struct quota_info *dqopt = sb_dqopt(sbi->sb);
2212+
struct address_space *mapping = dqopt->files[type]->i_mapping;
2213+
int ret = 0;
2214+
2215+
ret = dquot_writeback_dquots(sbi->sb, type);
2216+
if (ret)
2217+
goto out;
2218+
2219+
ret = filemap_fdatawrite(mapping);
2220+
if (ret)
2221+
goto out;
2222+
2223+
/* if we are using journalled quota */
2224+
if (is_journalled_quota(sbi))
2225+
goto out;
2226+
2227+
ret = filemap_fdatawait(mapping);
2228+
2229+
truncate_inode_pages(&dqopt->files[type]->i_data, 0);
2230+
out:
2231+
if (ret)
2232+
set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
2233+
return ret;
2234+
}
2235+
22092236
int f2fs_quota_sync(struct super_block *sb, int type)
22102237
{
22112238
struct f2fs_sb_info *sbi = F2FS_SB(sb);
22122239
struct quota_info *dqopt = sb_dqopt(sb);
22132240
int cnt;
22142241
int ret;
22152242

2216-
/*
2217-
* do_quotactl
2218-
* f2fs_quota_sync
2219-
* down_read(quota_sem)
2220-
* dquot_writeback_dquots()
2221-
* f2fs_dquot_commit
2222-
* block_operation
2223-
* down_read(quota_sem)
2224-
*/
2225-
f2fs_lock_op(sbi);
2226-
2227-
down_read(&sbi->quota_sem);
2228-
ret = dquot_writeback_dquots(sb, type);
2229-
if (ret)
2230-
goto out;
2231-
22322243
/*
22332244
* Now when everything is written we can discard the pagecache so
22342245
* that userspace sees the changes.
22352246
*/
22362247
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
2237-
struct address_space *mapping;
22382248

22392249
if (type != -1 && cnt != type)
22402250
continue;
2241-
if (!sb_has_quota_active(sb, cnt))
2242-
continue;
22432251

2244-
mapping = dqopt->files[cnt]->i_mapping;
2252+
if (!sb_has_quota_active(sb, type))
2253+
return 0;
22452254

2246-
ret = filemap_fdatawrite(mapping);
2247-
if (ret)
2248-
goto out;
2255+
inode_lock(dqopt->files[cnt]);
22492256

2250-
/* if we are using journalled quota */
2251-
if (is_journalled_quota(sbi))
2252-
continue;
2257+
/*
2258+
* do_quotactl
2259+
* f2fs_quota_sync
2260+
* down_read(quota_sem)
2261+
* dquot_writeback_dquots()
2262+
* f2fs_dquot_commit
2263+
* block_operation
2264+
* down_read(quota_sem)
2265+
*/
2266+
f2fs_lock_op(sbi);
2267+
down_read(&sbi->quota_sem);
22532268

2254-
ret = filemap_fdatawait(mapping);
2255-
if (ret)
2256-
set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
2269+
ret = f2fs_quota_sync_file(sbi, cnt);
2270+
2271+
up_read(&sbi->quota_sem);
2272+
f2fs_unlock_op(sbi);
22572273

2258-
inode_lock(dqopt->files[cnt]);
2259-
truncate_inode_pages(&dqopt->files[cnt]->i_data, 0);
22602274
inode_unlock(dqopt->files[cnt]);
2275+
2276+
if (ret)
2277+
break;
22612278
}
2262-
out:
2263-
if (ret)
2264-
set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
2265-
up_read(&sbi->quota_sem);
2266-
f2fs_unlock_op(sbi);
22672279
return ret;
22682280
}
22692281

0 commit comments

Comments
 (0)