All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	Filipe Manana <fdmanana@suse.com>,
	Sasha Levin <sashal@kernel.org>,
	linux-btrfs@vger.kernel.org
Subject: [PATCH AUTOSEL 5.9 18/35] btrfs: drop the path before adding qgroup items when enabling qgroups
Date: Mon,  2 Nov 2020 20:18:23 -0500	[thread overview]
Message-ID: <20201103011840.182814-18-sashal@kernel.org> (raw)
In-Reply-To: <20201103011840.182814-1-sashal@kernel.org>

From: Josef Bacik <josef@toxicpanda.com>

[ Upstream commit 5223cc60b40ae525ae6c94e98824129f1a5b4ae5 ]

When enabling qgroups we walk the tree_root and then add a qgroup item
for every root that we have.  This creates a lock dependency on the
tree_root and qgroup_root, which results in the following lockdep splat
(with tree locks using rwsem), eg. in tests btrfs/017 or btrfs/022:

  ======================================================
  WARNING: possible circular locking dependency detected
  5.9.0-default+ #1299 Not tainted
  ------------------------------------------------------
  btrfs/24552 is trying to acquire lock:
  ffff9142dfc5f630 (btrfs-quota-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]

  but task is already holding lock:
  ffff9142dfc5d0b0 (btrfs-root-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]

  which lock already depends on the new lock.

  the existing dependency chain (in reverse order) is:

  -> #1 (btrfs-root-00){++++}-{3:3}:
	 __lock_acquire+0x3fb/0x730
	 lock_acquire.part.0+0x6a/0x130
	 down_read_nested+0x46/0x130
	 __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]
	 __btrfs_read_lock_root_node+0x3a/0x50 [btrfs]
	 btrfs_search_slot_get_root+0x11d/0x290 [btrfs]
	 btrfs_search_slot+0xc3/0x9f0 [btrfs]
	 btrfs_insert_item+0x6e/0x140 [btrfs]
	 btrfs_create_tree+0x1cb/0x240 [btrfs]
	 btrfs_quota_enable+0xcd/0x790 [btrfs]
	 btrfs_ioctl_quota_ctl+0xc9/0xe0 [btrfs]
	 __x64_sys_ioctl+0x83/0xa0
	 do_syscall_64+0x2d/0x70
	 entry_SYSCALL_64_after_hwframe+0x44/0xa9

  -> #0 (btrfs-quota-00){++++}-{3:3}:
	 check_prev_add+0x91/0xc30
	 validate_chain+0x491/0x750
	 __lock_acquire+0x3fb/0x730
	 lock_acquire.part.0+0x6a/0x130
	 down_read_nested+0x46/0x130
	 __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]
	 __btrfs_read_lock_root_node+0x3a/0x50 [btrfs]
	 btrfs_search_slot_get_root+0x11d/0x290 [btrfs]
	 btrfs_search_slot+0xc3/0x9f0 [btrfs]
	 btrfs_insert_empty_items+0x58/0xa0 [btrfs]
	 add_qgroup_item.part.0+0x72/0x210 [btrfs]
	 btrfs_quota_enable+0x3bb/0x790 [btrfs]
	 btrfs_ioctl_quota_ctl+0xc9/0xe0 [btrfs]
	 __x64_sys_ioctl+0x83/0xa0
	 do_syscall_64+0x2d/0x70
	 entry_SYSCALL_64_after_hwframe+0x44/0xa9

  other info that might help us debug this:

   Possible unsafe locking scenario:

	 CPU0                    CPU1
	 ----                    ----
    lock(btrfs-root-00);
				 lock(btrfs-quota-00);
				 lock(btrfs-root-00);
    lock(btrfs-quota-00);

   *** DEADLOCK ***

  5 locks held by btrfs/24552:
   #0: ffff9142df431478 (sb_writers#10){.+.+}-{0:0}, at: mnt_want_write_file+0x22/0xa0
   #1: ffff9142f9b10cc0 (&fs_info->subvol_sem){++++}-{3:3}, at: btrfs_ioctl_quota_ctl+0x7b/0xe0 [btrfs]
   #2: ffff9142f9b11a08 (&fs_info->qgroup_ioctl_lock){+.+.}-{3:3}, at: btrfs_quota_enable+0x3b/0x790 [btrfs]
   #3: ffff9142df431698 (sb_internal#2){.+.+}-{0:0}, at: start_transaction+0x406/0x510 [btrfs]
   #4: ffff9142dfc5d0b0 (btrfs-root-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]

  stack backtrace:
  CPU: 1 PID: 24552 Comm: btrfs Not tainted 5.9.0-default+ #1299
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba527-rebuilt.opensuse.org 04/01/2014
  Call Trace:
   dump_stack+0x77/0x97
   check_noncircular+0xf3/0x110
   check_prev_add+0x91/0xc30
   validate_chain+0x491/0x750
   __lock_acquire+0x3fb/0x730
   lock_acquire.part.0+0x6a/0x130
   ? __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]
   ? lock_acquire+0xc4/0x140
   ? __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]
   down_read_nested+0x46/0x130
   ? __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]
   __btrfs_tree_read_lock+0x35/0x1c0 [btrfs]
   ? btrfs_root_node+0xd9/0x200 [btrfs]
   __btrfs_read_lock_root_node+0x3a/0x50 [btrfs]
   btrfs_search_slot_get_root+0x11d/0x290 [btrfs]
   btrfs_search_slot+0xc3/0x9f0 [btrfs]
   btrfs_insert_empty_items+0x58/0xa0 [btrfs]
   add_qgroup_item.part.0+0x72/0x210 [btrfs]
   btrfs_quota_enable+0x3bb/0x790 [btrfs]
   btrfs_ioctl_quota_ctl+0xc9/0xe0 [btrfs]
   __x64_sys_ioctl+0x83/0xa0
   do_syscall_64+0x2d/0x70
   entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fix this by dropping the path whenever we find a root item, add the
qgroup item, and then re-lookup the root item we found and continue
processing roots.

Reported-by: David Sterba <dsterba@suse.com>
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: Sasha Levin <sashal@kernel.org>
---
 fs/btrfs/qgroup.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index c0f350c3a0cf4..db953cb947bc4 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1026,6 +1026,10 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info)
 		btrfs_item_key_to_cpu(leaf, &found_key, slot);
 
 		if (found_key.type == BTRFS_ROOT_REF_KEY) {
+
+			/* Release locks on tree_root before we access quota_root */
+			btrfs_release_path(path);
+
 			ret = add_qgroup_item(trans, quota_root,
 					      found_key.offset);
 			if (ret) {
@@ -1044,6 +1048,20 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info)
 				btrfs_abort_transaction(trans, ret);
 				goto out_free_path;
 			}
+			ret = btrfs_search_slot_for_read(tree_root, &found_key,
+							 path, 1, 0);
+			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
+				goto out_free_path;
+			}
+			if (ret > 0) {
+				/*
+				 * Shouldn't happen, but in case it does we
+				 * don't need to do the btrfs_next_item, just
+				 * continue.
+				 */
+				continue;
+			}
 		}
 		ret = btrfs_next_item(tree_root, path);
 		if (ret < 0) {
-- 
2.27.0


  parent reply	other threads:[~2020-11-03  1:27 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-03  1:18 [PATCH AUTOSEL 5.9 01/35] ARM: dts: sun4i-a10: fix cpu_alert temperature Sasha Levin
2020-11-03  1:18 ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 02/35] arm64: dts: meson-axg: add USB nodes Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  8:55   ` Neil Armstrong
2020-11-03  8:55     ` Neil Armstrong
2020-11-03  8:55     ` Neil Armstrong
2020-11-08 13:14     ` Sasha Levin
2020-11-08 13:14       ` Sasha Levin
2020-11-08 13:14       ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 03/35] arm64: dts: meson-axg-s400: enable USB OTG Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  8:55   ` Neil Armstrong
2020-11-03  8:55     ` Neil Armstrong
2020-11-03  8:55     ` Neil Armstrong
2020-11-08 13:14     ` Sasha Levin
2020-11-08 13:14       ` Sasha Levin
2020-11-08 13:14       ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 04/35] arm64: dts: meson: add missing g12 rng clock Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 05/35] arm64: dts: amlogic: meson-g12: use the G12A specific dwmac compatible Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 06/35] x86/kexec: Use up-to-dated screen_info copy to fill boot params Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 07/35] hyperv_fb: Update screen_info after removing old framebuffer Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 08/35] arm64: dts: amlogic: add missing ethernet reset ID Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 09/35] io_uring: don't miss setting IO_WQ_WORK_CONCURRENT Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 10/35] of: Fix reserved-memory overlap detection Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 11/35] ARM: dts: mmp3: Add power domain for the camera Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 12/35] drm/sun4i: frontend: Rework a bit the phase data Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 13/35] drm/sun4i: frontend: Reuse the ch0 phase for RGB formats Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 14/35] drm/sun4i: frontend: Fix the scaler phase on A33 Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 15/35] drm/v3d: Fix double free in v3d_submit_cl_ioctl() Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 16/35] blk-cgroup: Fix memleak on error path Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 17/35] blk-cgroup: Pre-allocate tree node on blkg_conf_prep Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` Sasha Levin [this message]
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 19/35] btrfs: add a helper to read the tree_root commit root for backref lookup Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 20/35] scsi: core: Don't start concurrent async scan on same host Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 21/35] drm/amdgpu: disable DCN and VCN for navi10 blockchain SKU(v3) Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 22/35] drm/amdgpu: add DID for navi10 blockchain SKU Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 23/35] drm/amd/display: Fixed panic during seamless boot Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 24/35] scsi: ibmvscsi: Fix potential race after loss of transport Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 25/35] drm/amd/display: adding ddc_gpio_vga_reg_list to ddc reg def'ns Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 26/35] vsock: use ns_capable_noaudit() on socket create Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 27/35] nvme-rdma: handle unexpected nvme completion data length Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 28/35] nvmet: fix a NULL pointer dereference when tracing the flush command Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 29/35] staging: mmal-vchiq: Fix memory leak for vchiq_instance Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 30/35] drm/vc4: drv: Add error handding for bind Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 31/35] ACPI: NFIT: Fix comparison to '-ENXIO' Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 32/35] usb: cdns3: gadget: suspicious implicit sign extension Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 33/35] drm/nouveau/nouveau: fix the start/end range for migration Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 34/35] drm/nouveau/gem: fix "refcount_t: underflow; use-after-free" Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18   ` Sasha Levin
2020-11-03  1:18 ` [PATCH AUTOSEL 5.9 35/35] arm64/smp: Move rcu_cpu_starting() earlier Sasha Levin
2020-11-03  1:18   ` Sasha Levin

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=20201103011840.182814-18-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=dsterba@suse.com \
    --cc=fdmanana@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@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 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.