From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH] btrfs: qgroup: don't try to wait flushing if we're already holding a transaction
Date: Fri, 4 Dec 2020 09:24:47 +0800 [thread overview]
Message-ID: <20201204012448.26546-1-wqu@suse.com> (raw)
There is a chance of racing for qgroup flushing which may lead to
deadlock:
Thread A | Thread B
(no trans handler hold) | (already hold a trans handler)
--------------------------------+--------------------------------
__btrfs_qgroup_reserve_meta() | __btrfs_qgroup_reserve_meta()
|- try_flush_qgroup() | |- try_flushing_qgroup()
|- QGROUP_FLUSHING bit set | |
| | |- test_and_set_bit()
| | |- wait_event()
|- btrfs_join_transaction() |
|- btrfs_commit_transaction()|
!!! DEAD LOCK !!!
Since thread A want to commit transaction, but thread B is hold a
transaction handler, blocking the commit.
At the same time, thread B is waiting thread A to finish it commit.
This is just a hot fix, and would lead to more EDQUOT when we're near
the qgroup limit.
The root fix would to make all metadata/data reservation to happen
without a transaction handler hold.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/qgroup.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index fe3046007f52..7785dfa348d2 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -3530,16 +3530,6 @@ static int try_flush_qgroup(struct btrfs_root *root)
int ret;
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
* assume all space reservation happens before a transaction handle is
@@ -3554,6 +3544,27 @@ static int try_flush_qgroup(struct btrfs_root *root)
current->journal_info != BTRFS_SEND_TRANS_STUB)
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 trans, 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);
if (ret < 0)
goto out;
--
2.29.2
next reply other threads:[~2020-12-04 1:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-04 1:24 Qu Wenruo [this message]
2020-12-04 1:24 ` [PATCH] btrfs: qgroup: don't commit transaction when we already hold the handle Qu Wenruo
2020-12-04 7:37 ` Nikolay Borisov
2020-12-04 7:46 ` Qu Wenruo
2020-12-04 11:48 ` [PATCH] btrfs: qgroup: don't try to wait flushing if we're already holding a transaction Filipe Manana
2020-12-04 17:28 ` David Sterba
2020-12-05 2:55 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201204012448.26546-1-wqu@suse.com \
--to=wqu@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox