btrfs: drop path before adding new uuid tree entry
authorJosef Bacik <josef@toxicpanda.com>
Mon, 10 Aug 2020 15:42:26 +0000 (11:42 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 12 Sep 2020 09:47:35 +0000 (11:47 +0200)
commit 9771a5cf937129307d9f58922d60484d58ababe7 upstream.

With the conversion of the tree locks to rwsem 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.

CC: stable@vger.kernel.org # 4.4+
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/btrfs/volumes.c

index bace03a546b2dee9222692c6146d61a56553d7b9..c31b02692f706ac3b26e9a1c69df667a3f7da9d2 100644 (file)
@@ -4181,6 +4181,7 @@ static 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, fs_info->uuid_root,
                                                  root_item.uuid,
@@ -4206,6 +4207,7 @@ update_tree:
                }
 
 skip:
+               btrfs_release_path(path);
                if (trans) {
                        ret = btrfs_end_transaction(trans, fs_info->uuid_root);
                        trans = NULL;
@@ -4213,7 +4215,6 @@ skip:
                                break;
                }
 
-               btrfs_release_path(path);
                if (key.offset < (u64)-1) {
                        key.offset++;
                } else if (key.type < BTRFS_ROOT_ITEM_KEY) {