Skip to content

Commit

Permalink
btrfs: fix deadlock when cloning inline extents and low on available …
Browse files Browse the repository at this point in the history
…space

commit 76a6d5c upstream.

There are a few cases where cloning an inline extent requires copying data
into a page of the destination inode. For these cases we are allocating
the required data and metadata space while holding a leaf locked. This can
result in a deadlock when we are low on available space because allocating
the space may flush delalloc and two deadlock scenarios can happen:

1) When starting writeback for an inode with a very small dirty range that
   fits in an inline extent, we deadlock during the writeback when trying
   to insert the inline extent, at cow_file_range_inline(), if the extent
   is going to be located in the leaf for which we are already holding a
   read lock;

2) After successfully starting writeback, for non-inline extent cases,
   the async reclaim thread will hang waiting for an ordered extent to
   complete if the ordered extent completion needs to modify the leaf
   for which the clone task is holding a read lock (for adding or
   replacing file extent items). So the cloning task will wait forever
   on the async reclaim thread to make progress, which in turn is
   waiting for the ordered extent completion which in turn is waiting
   to acquire a write lock on the same leaf.

So fix this by making sure we release the path (and therefore the leaf)
every time we need to copy the inline extent's data into a page of the
destination inode, as by that time we do not need to have the leaf locked.

Fixes: 05a5a76 ("Btrfs: implement full reflink support for inline extents")
CC: stable@vger.kernel.org # 5.10+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
fdmanana authored and gregkh committed Jun 10, 2021
1 parent 0df50d4 commit baa6763
Showing 1 changed file with 22 additions and 16 deletions.
38 changes: 22 additions & 16 deletions fs/btrfs/reflink.c
Expand Up @@ -207,10 +207,7 @@ static int clone_copy_inline_extent(struct inode *dst,
* inline extent's data to the page.
*/
ASSERT(key.offset > 0);
ret = copy_inline_to_page(BTRFS_I(dst), new_key->offset,
inline_data, size, datal,
comp_type);
goto out;
goto copy_to_page;
}
} else if (i_size_read(dst) <= datal) {
struct btrfs_file_extent_item *ei;
Expand All @@ -226,13 +223,10 @@ static int clone_copy_inline_extent(struct inode *dst,
BTRFS_FILE_EXTENT_INLINE)
goto copy_inline_extent;

ret = copy_inline_to_page(BTRFS_I(dst), new_key->offset,
inline_data, size, datal, comp_type);
goto out;
goto copy_to_page;
}

copy_inline_extent:
ret = 0;
/*
* We have no extent items, or we have an extent at offset 0 which may
* or may not be inlined. All these cases are dealt the same way.
Expand All @@ -244,11 +238,13 @@ static int clone_copy_inline_extent(struct inode *dst,
* clone. Deal with all these cases by copying the inline extent
* data into the respective page at the destination inode.
*/
ret = copy_inline_to_page(BTRFS_I(dst), new_key->offset,
inline_data, size, datal, comp_type);
goto out;
goto copy_to_page;
}

/*
* Release path before starting a new transaction so we don't hold locks
* that would confuse lockdep.
*/
btrfs_release_path(path);
/*
* If we end up here it means were copy the inline extent into a leaf
Expand Down Expand Up @@ -281,11 +277,6 @@ static int clone_copy_inline_extent(struct inode *dst,
ret = btrfs_inode_set_file_extent_range(BTRFS_I(dst), 0, aligned_end);
out:
if (!ret && !trans) {
/*
* Release path before starting a new transaction so we don't
* hold locks that would confuse lockdep.
*/
btrfs_release_path(path);
/*
* No transaction here means we copied the inline extent into a
* page of the destination inode.
Expand All @@ -306,6 +297,21 @@ static int clone_copy_inline_extent(struct inode *dst,
*trans_out = trans;

return ret;

copy_to_page:
/*
* Release our path because we don't need it anymore and also because
* copy_inline_to_page() needs to reserve data and metadata, which may
* need to flush delalloc when we are low on available space and
* therefore cause a deadlock if writeback of an inline extent needs to
* write to the same leaf or an ordered extent completion needs to write
* to the same leaf.
*/
btrfs_release_path(path);

ret = copy_inline_to_page(BTRFS_I(dst), new_key->offset,
inline_data, size, datal, comp_type);
goto out;
}

/**
Expand Down

0 comments on commit baa6763

Please sign in to comment.