f2fs: quota: fix potential deadlock
xfstest generic/587 reports a deadlock issue as below:
======================================================
WARNING: possible circular locking dependency detected
5.14.0-rc1 #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:
-> #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
-> #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
#1: ffff8880084bc380 (&sbi->cp_rwsem){++++}-{3:3}, at: f2fs_quota_sync+0x3e/0x300 [f2fs]
#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 #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: db6ec53b7e
("f2fs: add a rw_sem to cover quota flag changes")
Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
This commit is contained in:
parent
1ffc8f5f77
commit
9de71ede81
1 changed files with 50 additions and 38 deletions
|
@ -2518,6 +2518,33 @@ static int f2fs_enable_quotas(struct super_block *sb)
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static int f2fs_quota_sync_file(struct f2fs_sb_info *sbi, int type)
|
||||||
|
{
|
||||||
|
struct quota_info *dqopt = sb_dqopt(sbi->sb);
|
||||||
|
struct address_space *mapping = dqopt->files[type]->i_mapping;
|
||||||
|
int ret = 0;
|
||||||
|
|
||||||
|
ret = dquot_writeback_dquots(sbi->sb, type);
|
||||||
|
if (ret)
|
||||||
|
goto out;
|
||||||
|
|
||||||
|
ret = filemap_fdatawrite(mapping);
|
||||||
|
if (ret)
|
||||||
|
goto out;
|
||||||
|
|
||||||
|
/* if we are using journalled quota */
|
||||||
|
if (is_journalled_quota(sbi))
|
||||||
|
goto out;
|
||||||
|
|
||||||
|
ret = filemap_fdatawait(mapping);
|
||||||
|
|
||||||
|
truncate_inode_pages(&dqopt->files[type]->i_data, 0);
|
||||||
|
out:
|
||||||
|
if (ret)
|
||||||
|
set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
|
||||||
int f2fs_quota_sync(struct super_block *sb, int type)
|
int f2fs_quota_sync(struct super_block *sb, int type)
|
||||||
{
|
{
|
||||||
struct f2fs_sb_info *sbi = F2FS_SB(sb);
|
struct f2fs_sb_info *sbi = F2FS_SB(sb);
|
||||||
|
@ -2525,6 +2552,20 @@ int f2fs_quota_sync(struct super_block *sb, int type)
|
||||||
int cnt;
|
int cnt;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Now when everything is written we can discard the pagecache so
|
||||||
|
* that userspace sees the changes.
|
||||||
|
*/
|
||||||
|
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
|
||||||
|
|
||||||
|
if (type != -1 && cnt != type)
|
||||||
|
continue;
|
||||||
|
|
||||||
|
if (!sb_has_quota_active(sb, type))
|
||||||
|
return 0;
|
||||||
|
|
||||||
|
inode_lock(dqopt->files[cnt]);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* do_quotactl
|
* do_quotactl
|
||||||
* f2fs_quota_sync
|
* f2fs_quota_sync
|
||||||
|
@ -2535,47 +2576,18 @@ int f2fs_quota_sync(struct super_block *sb, int type)
|
||||||
* down_read(quota_sem)
|
* down_read(quota_sem)
|
||||||
*/
|
*/
|
||||||
f2fs_lock_op(sbi);
|
f2fs_lock_op(sbi);
|
||||||
|
|
||||||
down_read(&sbi->quota_sem);
|
down_read(&sbi->quota_sem);
|
||||||
ret = dquot_writeback_dquots(sb, type);
|
|
||||||
if (ret)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
/*
|
ret = f2fs_quota_sync_file(sbi, cnt);
|
||||||
* Now when everything is written we can discard the pagecache so
|
|
||||||
* that userspace sees the changes.
|
|
||||||
*/
|
|
||||||
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
|
|
||||||
struct address_space *mapping;
|
|
||||||
|
|
||||||
if (type != -1 && cnt != type)
|
|
||||||
continue;
|
|
||||||
if (!sb_has_quota_active(sb, cnt))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
mapping = dqopt->files[cnt]->i_mapping;
|
|
||||||
|
|
||||||
ret = filemap_fdatawrite(mapping);
|
|
||||||
if (ret)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
/* if we are using journalled quota */
|
|
||||||
if (is_journalled_quota(sbi))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
ret = filemap_fdatawait(mapping);
|
|
||||||
if (ret)
|
|
||||||
set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
|
|
||||||
|
|
||||||
inode_lock(dqopt->files[cnt]);
|
|
||||||
truncate_inode_pages(&dqopt->files[cnt]->i_data, 0);
|
|
||||||
inode_unlock(dqopt->files[cnt]);
|
|
||||||
}
|
|
||||||
out:
|
|
||||||
if (ret)
|
|
||||||
set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
|
|
||||||
up_read(&sbi->quota_sem);
|
up_read(&sbi->quota_sem);
|
||||||
f2fs_unlock_op(sbi);
|
f2fs_unlock_op(sbi);
|
||||||
|
|
||||||
|
inode_unlock(dqopt->files[cnt]);
|
||||||
|
|
||||||
|
if (ret)
|
||||||
|
break;
|
||||||
|
}
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Add table
Reference in a new issue