From: Josef Bacik <josef@toxicpanda.com>
To: linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: [PATCH 01/17] btrfs: drop path before adding new uuid tree entry
Date: Mon, 10 Aug 2020 11:42:26 -0400 [thread overview]
Message-ID: <20200810154242.782802-2-josef@toxicpanda.com> (raw)
In-Reply-To: <20200810154242.782802-1-josef@toxicpanda.com>
With the conversion to the rwsemaphore I got the following lockdep splat
======================================================
WARNING: possible circular locking dependency detected
5.8.0-rc7-00167-g0d7ba0c5b375-dirty #925 Not tainted
------------------------------------------------------
btrfs-uuid/7955 is trying to acquire lock:
ffff88bfbafec0f8 (btrfs-root-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x39/0x180
but task is already holding lock:
ffff88bfbafef2a8 (btrfs-uuid-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x39/0x180
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (btrfs-uuid-00){++++}-{3:3}:
down_read_nested+0x3e/0x140
__btrfs_tree_read_lock+0x39/0x180
__btrfs_read_lock_root_node+0x3a/0x50
btrfs_search_slot+0x4bd/0x990
btrfs_uuid_tree_add+0x89/0x2d0
btrfs_uuid_scan_kthread+0x330/0x390
kthread+0x133/0x150
ret_from_fork+0x1f/0x30
-> #0 (btrfs-root-00){++++}-{3:3}:
__lock_acquire+0x1272/0x2310
lock_acquire+0x9e/0x360
down_read_nested+0x3e/0x140
__btrfs_tree_read_lock+0x39/0x180
__btrfs_read_lock_root_node+0x3a/0x50
btrfs_search_slot+0x4bd/0x990
btrfs_find_root+0x45/0x1b0
btrfs_read_tree_root+0x61/0x100
btrfs_get_root_ref.part.50+0x143/0x630
btrfs_uuid_tree_iterate+0x207/0x314
btrfs_uuid_rescan_kthread+0x12/0x50
kthread+0x133/0x150
ret_from_fork+0x1f/0x30
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(btrfs-uuid-00);
lock(btrfs-root-00);
lock(btrfs-uuid-00);
lock(btrfs-root-00);
*** DEADLOCK ***
1 lock held by btrfs-uuid/7955:
#0: ffff88bfbafef2a8 (btrfs-uuid-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x39/0x180
stack backtrace:
CPU: 73 PID: 7955 Comm: btrfs-uuid Kdump: loaded Not tainted 5.8.0-rc7-00167-g0d7ba0c5b375-dirty #925
Hardware name: Quanta Tioga Pass Single Side 01-0030993006/Tioga Pass Single Side, BIOS F08_3A18 12/20/2018
Call Trace:
dump_stack+0x78/0xa0
check_noncircular+0x165/0x180
__lock_acquire+0x1272/0x2310
lock_acquire+0x9e/0x360
? __btrfs_tree_read_lock+0x39/0x180
? btrfs_root_node+0x1c/0x1d0
down_read_nested+0x3e/0x140
? __btrfs_tree_read_lock+0x39/0x180
__btrfs_tree_read_lock+0x39/0x180
__btrfs_read_lock_root_node+0x3a/0x50
btrfs_search_slot+0x4bd/0x990
btrfs_find_root+0x45/0x1b0
btrfs_read_tree_root+0x61/0x100
btrfs_get_root_ref.part.50+0x143/0x630
btrfs_uuid_tree_iterate+0x207/0x314
? btree_readpage+0x20/0x20
btrfs_uuid_rescan_kthread+0x12/0x50
kthread+0x133/0x150
? kthread_create_on_node+0x60/0x60
ret_from_fork+0x1f/0x30
This problem exists because we have two different rescan threads,
btrfs_uuid_scan_kthread which creates the uuid tree, and
btrfs_uuid_tree_iterate that goes through and updates or deletes any out
of date roots. The problem is they both do things in different order.
btrfs_uuid_scan_kthread() reads the tree_root, and then inserts entries
into the uuid_root. btrfs_uuid_tree_iterate() scans the uuid_root, but
then does a btrfs_get_fs_root() which can read from the tree_root.
It's actually easy enough to not be holding the path in
btrfs_uuid_scan_kthread() when we add a uuid entry, as we already drop
it further down and re-start the search when we loop. So simply move
the path release before we add our entry to the uuid tree.
This also fixes a problem where we're holding a path open after we do
btrfs_end_transaction(), which has it's own problems.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/volumes.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d7670e2a9f39..3ac44dad58bb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4462,6 +4462,7 @@ int btrfs_uuid_scan_kthread(void *data)
goto skip;
}
update_tree:
+ btrfs_release_path(path);
if (!btrfs_is_empty_uuid(root_item.uuid)) {
ret = btrfs_uuid_tree_add(trans, root_item.uuid,
BTRFS_UUID_KEY_SUBVOL,
@@ -4486,6 +4487,7 @@ int btrfs_uuid_scan_kthread(void *data)
}
skip:
+ btrfs_release_path(path);
if (trans) {
ret = btrfs_end_transaction(trans);
trans = NULL;
@@ -4493,7 +4495,6 @@ int btrfs_uuid_scan_kthread(void *data)
break;
}
- btrfs_release_path(path);
if (key.offset < (u64)-1) {
key.offset++;
} else if (key.type < BTRFS_ROOT_ITEM_KEY) {
--
2.24.1
next prev parent reply other threads:[~2020-08-10 15:42 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-10 15:42 [PATCH 00/17] Convert to an rwsem for our tree locking Josef Bacik
2020-08-10 15:42 ` Josef Bacik [this message]
2020-08-10 16:28 ` [PATCH 01/17] btrfs: drop path before adding new uuid tree entry Filipe Manana
2020-08-10 16:30 ` Filipe Manana
2020-08-11 14:35 ` Josef Bacik
2020-08-10 15:42 ` [PATCH 02/17] btrfs: fix potential deadlock in the search ioctl Josef Bacik
2020-08-10 16:45 ` Filipe Manana
2020-08-10 15:42 ` [PATCH 03/17] btrfs: do not hold device_list_mutex when closing devices Josef Bacik
2020-08-11 10:53 ` Filipe Manana
2020-08-14 14:11 ` David Sterba
2020-08-10 15:42 ` [PATCH 04/17] btrfs: allocate scrub workqueues outside of locks Josef Bacik
2020-08-11 11:22 ` Filipe Manana
2020-08-10 15:42 ` [PATCH 05/17] btrfs: set the correct lockdep class for new nodes Josef Bacik
2020-08-11 11:25 ` Filipe Manana
2020-08-10 15:42 ` [PATCH 06/17] btrfs: set the lockdep class for log tree extent buffers Josef Bacik
2020-08-11 11:28 ` Filipe Manana
2020-08-10 15:42 ` [PATCH 07/17] btrfs: rename eb->lock_nested to eb->lock_recursed Josef Bacik
2020-08-10 15:42 ` [PATCH 08/17] btrfs: introduce path->recurse Josef Bacik
2020-08-10 15:42 ` [PATCH 09/17] btrfs: add nesting tags to the locking helpers Josef Bacik
2020-08-14 14:41 ` David Sterba
2020-08-10 15:42 ` [PATCH 10/17] btrfs: introduce BTRFS_NESTING_COW for cow'ing blocks Josef Bacik
2020-08-10 15:42 ` [PATCH 11/17] btrfs: introduce BTRFS_NESTING_LEFT/BTRFS_NESTING_RIGHT Josef Bacik
2020-08-10 15:42 ` [PATCH 12/17] btrfs: introduce BTRFS_NESTING_LEFT/RIGHT_COW Josef Bacik
2020-08-10 15:42 ` [PATCH 13/17] btrfs: introduce BTRFS_NESTING_SPLIT for split blocks Josef Bacik
2020-08-10 15:42 ` [PATCH 14/17] btrfs: introduce BTRFS_NESTING_NEW_ROOT for adding new roots Josef Bacik
2020-08-14 14:45 ` David Sterba
2020-08-10 15:42 ` [PATCH 15/17] btrfs: change our extent buffer lock to a rw_semaphore Josef Bacik
2020-08-10 15:42 ` [PATCH 16/17] btrfs: remove all of the blocking helpers Josef Bacik
2020-08-10 15:42 ` [PATCH 17/17] btrfs: rip out path->leave_spinning Josef Bacik
2020-08-13 14:26 ` [PATCH 00/17] Convert to an rwsem for our tree locking David Sterba
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=20200810154242.782802-2-josef@toxicpanda.com \
--to=josef@toxicpanda.com \
--cc=kernel-team@fb.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