* [PATCH v4] btrfs: qgroup: fix deadlock between rescan worker and remove qgroup
@ 2022-02-28 1:43 Sidong Yang
2022-02-28 2:19 ` Shinichiro Kawasaki
2022-02-28 21:29 ` David Sterba
0 siblings, 2 replies; 3+ messages in thread
From: Sidong Yang @ 2022-02-28 1:43 UTC (permalink / raw)
To: Shinichiro Kawasaki, Filipe Manana, dsterba, linux-btrfs
Cc: Sidong Yang, Filipe Manana
The commit e804861bd4e6 ("btrfs: fix deadlock between quota disable and
qgroup rescan worker") by Kawasaki resolves deadlock between quota
disable and qgroup rescan worker. But also there is a deadlock case like
it. It's about enabling or disabling quota and creating or removing
qgroup. It can be reproduced in simple script below.
for i in {1..100}
do
btrfs quota enable /mnt &
btrfs qgroup create 1/0 /mnt &
btrfs qgroup destroy 1/0 /mnt &
btrfs quota disable /mnt &
done
Here's why the deadlock happens:
1) The quota rescan task is running.
2) Task A calls btrfs_quota_disable(), locks the qgroup_ioctl_lock
mutex, and then calls btrfs_qgroup_wait_for_completion(), to wait for
the quota rescan task to complete.
3) Task B calls btrfs_remove_qgroup() and it blocks when trying to lock
the qgroup_ioctl_lock mutex, because it's being held by task A. At that
point task B is holding a transaction handle for the current transaction.
4) The quota rescan task calls btrfs_commit_transaction(). This results
in it waiting for all other tasks to release their handles on the
transaction, but task B is blocked on the qgroup_ioctl_lock mutex
while holding a handle on the transaction, and that mutex is being held
by task A, which is waiting for the quota rescan task to complete,
resulting in a deadlock between these 3 tasks.
To resolve this issue, the thread disabling quota should unlock
qgroup_ioctl_lock before waiting rescan completion. Move
btrfs_qgroup_wait_for_completion() after unlock of qgroup_ioctl_lock.
Fixes: e804861bd4e6 ("btrfs: fix deadlock between quota disable and
qgroup rescan worker")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
v4: fix typos, changelog.
v3: fix comments, typos, changelog.
v2: add comments, move locking before clear_bit.
---
fs/btrfs/qgroup.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 2c0dd6b8a80c..1866b1f0da01 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1213,6 +1213,14 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
if (!fs_info->quota_root)
goto out;
+ /*
+ * Unlock the qgroup_ioctl_lock mutex before waiting for the rescan worker to
+ * complete. Otherwise we can deadlock because btrfs_remove_qgroup() needs
+ * to lock that mutex while holding a transaction handle and the rescan
+ * worker needs to commit a transaction.
+ */
+ mutex_unlock(&fs_info->qgroup_ioctl_lock);
+
/*
* Request qgroup rescan worker to complete and wait for it. This wait
* must be done before transaction start for quota disable since it may
@@ -1220,7 +1228,6 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
*/
clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
btrfs_qgroup_wait_for_completion(fs_info, false);
- mutex_unlock(&fs_info->qgroup_ioctl_lock);
/*
* 1 For the root item
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4] btrfs: qgroup: fix deadlock between rescan worker and remove qgroup
2022-02-28 1:43 [PATCH v4] btrfs: qgroup: fix deadlock between rescan worker and remove qgroup Sidong Yang
@ 2022-02-28 2:19 ` Shinichiro Kawasaki
2022-02-28 21:29 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: Shinichiro Kawasaki @ 2022-02-28 2:19 UTC (permalink / raw)
To: Sidong Yang; +Cc: Filipe Manana, dsterba, linux-btrfs, Filipe Manana
On Feb 28, 2022 / 01:43, Sidong Yang wrote:
> The commit e804861bd4e6 ("btrfs: fix deadlock between quota disable and
> qgroup rescan worker") by Kawasaki resolves deadlock between quota
> disable and qgroup rescan worker. But also there is a deadlock case like
> it. It's about enabling or disabling quota and creating or removing
> qgroup. It can be reproduced in simple script below.
>
> for i in {1..100}
> do
> btrfs quota enable /mnt &
> btrfs qgroup create 1/0 /mnt &
> btrfs qgroup destroy 1/0 /mnt &
> btrfs quota disable /mnt &
> done
>
> Here's why the deadlock happens:
>
> 1) The quota rescan task is running.
>
> 2) Task A calls btrfs_quota_disable(), locks the qgroup_ioctl_lock
> mutex, and then calls btrfs_qgroup_wait_for_completion(), to wait for
> the quota rescan task to complete.
>
> 3) Task B calls btrfs_remove_qgroup() and it blocks when trying to lock
> the qgroup_ioctl_lock mutex, because it's being held by task A. At that
> point task B is holding a transaction handle for the current transaction.
>
> 4) The quota rescan task calls btrfs_commit_transaction(). This results
> in it waiting for all other tasks to release their handles on the
> transaction, but task B is blocked on the qgroup_ioctl_lock mutex
> while holding a handle on the transaction, and that mutex is being held
> by task A, which is waiting for the quota rescan task to complete,
> resulting in a deadlock between these 3 tasks.
>
> To resolve this issue, the thread disabling quota should unlock
> qgroup_ioctl_lock before waiting rescan completion. Move
> btrfs_qgroup_wait_for_completion() after unlock of qgroup_ioctl_lock.
>
> Fixes: e804861bd4e6 ("btrfs: fix deadlock between quota disable and
> qgroup rescan worker")
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
Thanks. Looks good to me.
Reviewed-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
--
Best Regards,
Shin'ichiro Kawasaki
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] btrfs: qgroup: fix deadlock between rescan worker and remove qgroup
2022-02-28 1:43 [PATCH v4] btrfs: qgroup: fix deadlock between rescan worker and remove qgroup Sidong Yang
2022-02-28 2:19 ` Shinichiro Kawasaki
@ 2022-02-28 21:29 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2022-02-28 21:29 UTC (permalink / raw)
To: Sidong Yang
Cc: Shinichiro Kawasaki, Filipe Manana, dsterba, linux-btrfs, Filipe Manana
On Mon, Feb 28, 2022 at 01:43:40AM +0000, Sidong Yang wrote:
> The commit e804861bd4e6 ("btrfs: fix deadlock between quota disable and
> qgroup rescan worker") by Kawasaki resolves deadlock between quota
> disable and qgroup rescan worker. But also there is a deadlock case like
> it. It's about enabling or disabling quota and creating or removing
> qgroup. It can be reproduced in simple script below.
>
> for i in {1..100}
> do
> btrfs quota enable /mnt &
> btrfs qgroup create 1/0 /mnt &
> btrfs qgroup destroy 1/0 /mnt &
> btrfs quota disable /mnt &
> done
>
> Here's why the deadlock happens:
>
> 1) The quota rescan task is running.
>
> 2) Task A calls btrfs_quota_disable(), locks the qgroup_ioctl_lock
> mutex, and then calls btrfs_qgroup_wait_for_completion(), to wait for
> the quota rescan task to complete.
>
> 3) Task B calls btrfs_remove_qgroup() and it blocks when trying to lock
> the qgroup_ioctl_lock mutex, because it's being held by task A. At that
> point task B is holding a transaction handle for the current transaction.
>
> 4) The quota rescan task calls btrfs_commit_transaction(). This results
> in it waiting for all other tasks to release their handles on the
> transaction, but task B is blocked on the qgroup_ioctl_lock mutex
> while holding a handle on the transaction, and that mutex is being held
> by task A, which is waiting for the quota rescan task to complete,
> resulting in a deadlock between these 3 tasks.
>
> To resolve this issue, the thread disabling quota should unlock
> qgroup_ioctl_lock before waiting rescan completion. Move
> btrfs_qgroup_wait_for_completion() after unlock of qgroup_ioctl_lock.
>
> Fixes: e804861bd4e6 ("btrfs: fix deadlock between quota disable and
> qgroup rescan worker")
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
> v4: fix typos, changelog.
Perfect, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-28 21:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28 1:43 [PATCH v4] btrfs: qgroup: fix deadlock between rescan worker and remove qgroup Sidong Yang
2022-02-28 2:19 ` Shinichiro Kawasaki
2022-02-28 21:29 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.