Commit ae5e070e authored by Qu Wenruo's avatar Qu Wenruo Committed by David Sterba

btrfs: qgroup: don't try to wait flushing if we're already holding a transaction

There is a chance of racing for qgroup flushing which may lead to
deadlock:

	Thread A		|	Thread B
   (not holding trans handle)	|  (holding a trans handle)
--------------------------------+--------------------------------
__btrfs_qgroup_reserve_meta()   | __btrfs_qgroup_reserve_meta()
|- try_flush_qgroup()		| |- try_flush_qgroup()
   |- QGROUP_FLUSHING bit set   |    |
   |				|    |- test_and_set_bit()
   |				|    |- wait_event()
   |- btrfs_join_transaction()	|
   |- btrfs_commit_transaction()|

			!!! DEAD LOCK !!!

Since thread A wants to commit transaction, but thread B is holding a
transaction handle, blocking the commit.
At the same time, thread B is waiting for thread A to finish its commit.

This is just a hot fix, and would lead to more EDQUOT when we're near
the qgroup limit.

The proper fix would be to make all metadata/data reservations happen
without holding a transaction handle.

CC: stable@vger.kernel.org # 5.9+
Reviewed-by: default avatarFilipe Manana <fdmanana@suse.com>
Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 9a664971
...@@ -3530,16 +3530,6 @@ static int try_flush_qgroup(struct btrfs_root *root) ...@@ -3530,16 +3530,6 @@ static int try_flush_qgroup(struct btrfs_root *root)
int ret; int ret;
bool can_commit = true; bool can_commit = true;
/*
* We don't want to run flush again and again, so if there is a running
* one, we won't try to start a new flush, but exit directly.
*/
if (test_and_set_bit(BTRFS_ROOT_QGROUP_FLUSHING, &root->state)) {
wait_event(root->qgroup_flush_wait,
!test_bit(BTRFS_ROOT_QGROUP_FLUSHING, &root->state));
return 0;
}
/* /*
* If current process holds a transaction, we shouldn't flush, as we * If current process holds a transaction, we shouldn't flush, as we
* assume all space reservation happens before a transaction handle is * assume all space reservation happens before a transaction handle is
...@@ -3554,6 +3544,26 @@ static int try_flush_qgroup(struct btrfs_root *root) ...@@ -3554,6 +3544,26 @@ static int try_flush_qgroup(struct btrfs_root *root)
current->journal_info != BTRFS_SEND_TRANS_STUB) current->journal_info != BTRFS_SEND_TRANS_STUB)
can_commit = false; can_commit = false;
/*
* We don't want to run flush again and again, so if there is a running
* one, we won't try to start a new flush, but exit directly.
*/
if (test_and_set_bit(BTRFS_ROOT_QGROUP_FLUSHING, &root->state)) {
/*
* We are already holding a transaction, thus we can block other
* threads from flushing. So exit right now. This increases
* the chance of EDQUOT for heavy load and near limit cases.
* But we can argue that if we're already near limit, EDQUOT is
* unavoidable anyway.
*/
if (!can_commit)
return 0;
wait_event(root->qgroup_flush_wait,
!test_bit(BTRFS_ROOT_QGROUP_FLUSHING, &root->state));
return 0;
}
ret = btrfs_start_delalloc_snapshot(root); ret = btrfs_start_delalloc_snapshot(root);
if (ret < 0) if (ret < 0)
goto out; goto out;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment