Btrfs: fix deadlock when using free space tree due to block group creation
diff mbox series

Message ID 20190108114441.5573-1-fdmanana@kernel.org
State New
Headers show
Series
  • Btrfs: fix deadlock when using free space tree due to block group creation
Related show

Commit Message

Filipe Manana Jan. 8, 2019, 11:44 a.m. UTC
From: Filipe Manana <fdmanana@suse.com>

When modifying the free space tree we can end up COWing one of its extent
buffers which in turn might result in allocating a new chunk, which in
turn can result in flushing (finish creation) of pending block groups. If
that happens we can deadlock because creating a pending block group needs
to update the free space tree, and if any of the updates tries to modify
the same extent buffer that we are COWing, we end up in a deadlock since
we try to write lock twice the same extent buffer.

So fix this by skipping pending block group creation if we are COWing an
extent buffer from the free space tree. This is a case missed by commit
5ce555578e091 ("Btrfs: fix deadlock when writing out free space caches").

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202173
Fixes: 5ce555578e091 ("Btrfs: fix deadlock when writing out free space caches
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ctree.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

David Sterba Jan. 8, 2019, 4:14 p.m. UTC | #1
On Tue, Jan 08, 2019 at 11:44:41AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When modifying the free space tree we can end up COWing one of its extent
> buffers which in turn might result in allocating a new chunk, which in
> turn can result in flushing (finish creation) of pending block groups. If
> that happens we can deadlock because creating a pending block group needs
> to update the free space tree, and if any of the updates tries to modify
> the same extent buffer that we are COWing, we end up in a deadlock since
> we try to write lock twice the same extent buffer.
> 
> So fix this by skipping pending block group creation if we are COWing an
> extent buffer from the free space tree. This is a case missed by commit
> 5ce555578e091 ("Btrfs: fix deadlock when writing out free space caches").
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202173
> Fixes: 5ce555578e091 ("Btrfs: fix deadlock when writing out free space caches
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Lighweight Reviewed-by, as the bug is in 4.19.x I'm going to push the
fix to 5.0. Thanks.
Filipe Manana Jan. 8, 2019, 4:17 p.m. UTC | #2
On Tue, Jan 8, 2019 at 4:14 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Tue, Jan 08, 2019 at 11:44:41AM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > When modifying the free space tree we can end up COWing one of its extent
> > buffers which in turn might result in allocating a new chunk, which in
> > turn can result in flushing (finish creation) of pending block groups. If
> > that happens we can deadlock because creating a pending block group needs
> > to update the free space tree, and if any of the updates tries to modify
> > the same extent buffer that we are COWing, we end up in a deadlock since
> > we try to write lock twice the same extent buffer.
> >
> > So fix this by skipping pending block group creation if we are COWing an
> > extent buffer from the free space tree. This is a case missed by commit
> > 5ce555578e091 ("Btrfs: fix deadlock when writing out free space caches").
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202173
> > Fixes: 5ce555578e091 ("Btrfs: fix deadlock when writing out free space caches
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
>
> Lighweight Reviewed-by, as the bug is in 4.19.x I'm going to push the
> fix to 5.0. Thanks.

The bug is in any kernel with free space tree support. In order to fix
it (apply this patch), the commit mentioned
in the Fixes tag is a dependency (it's not regression from that
commit, it simply didn't fix that case).

Patch
diff mbox series

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index d92462fe66c8..f64aad613727 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1016,19 +1016,21 @@  static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
 		parent_start = parent->start;
 
 	/*
-	 * If we are COWing a node/leaf from the extent, chunk or device trees,
-	 * make sure that we do not finish block group creation of pending block
-	 * groups. We do this to avoid a deadlock.
+	 * If we are COWing a node/leaf from the extent, chunk, device or free
+	 * space trees, make sure that we do not finish block group creation of
+	 * pending block groups. We do this to avoid a deadlock.
 	 * COWing can result in allocation of a new chunk, and flushing pending
 	 * block groups (btrfs_create_pending_block_groups()) can be triggered
 	 * when finishing allocation of a new chunk. Creation of a pending block
-	 * group modifies the extent, chunk and device trees, therefore we could
-	 * deadlock with ourselves since we are holding a lock on an extent
-	 * buffer that btrfs_create_pending_block_groups() may try to COW later.
+	 * group modifies the extent, chunk, device and free space trees,
+	 * therefore we could deadlock with ourselves since we are holding a
+	 * lock on an extent buffer that btrfs_create_pending_block_groups() may
+	 * try to COW later.
 	 */
 	if (root == fs_info->extent_root ||
 	    root == fs_info->chunk_root ||
-	    root == fs_info->dev_root)
+	    root == fs_info->dev_root ||
+	    root == fs_info->free_space_root)
 		trans->can_flush_pending_bgs = false;
 
 	cow = btrfs_alloc_tree_block(trans, root, parent_start,