diff mbox series

[10/11] btrfs: Factor out pinned extent clean up in btrfs_delete_unused_bgs

Message ID 20200120140918.15647-11-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series Make pinned extents tracking per-transaction | expand

Commit Message

Nikolay Borisov Jan. 20, 2020, 2:09 p.m. UTC
Next patch is going to refactor how pinned extents are tracked which
will necessitate changing this code. To ease that work and contain the
changes factor the code now in preparation, this will also help review.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/block-group.c | 69 ++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 29 deletions(-)

--
2.17.1

Comments

Josef Bacik Jan. 22, 2020, 8:14 p.m. UTC | #1
On 1/20/20 9:09 AM, Nikolay Borisov wrote:
> Next patch is going to refactor how pinned extents are tracked which
> will necessitate changing this code. To ease that work and contain the
> changes factor the code now in preparation, this will also help review.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 14851584e245..48bb9e08f2e8 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1233,6 +1233,45 @@  static int inc_block_group_ro(struct btrfs_block_group *cache, int force)
 	return ret;
 }

+static bool clean_pinned_extents(struct btrfs_block_group *bg)
+{
+	struct btrfs_fs_info *fs_info = bg->fs_info;
+	u64 start = bg->start;
+	u64 end = start + bg->length - 1;
+	int ret;
+
+	/*
+	 * Hold the unused_bg_unpin_mutex lock to avoid racing with
+	 * btrfs_finish_extent_commit(). If we are at transaction N,
+	 * another task might be running finish_extent_commit() for the
+	 * previous transaction N - 1, and have seen a range belonging
+	 * to the block group in freed_extents[] before we were able to
+	 * clear the whole block group range from freed_extents[]. This
+	 * means that task can lookup for the block group after we
+	 * unpinned it from freed_extents[] and removed it, leading to
+	 * a BUG_ON() at unpin_extent_range().
+	 */
+	mutex_lock(&fs_info->unused_bg_unpin_mutex);
+	ret = clear_extent_bits(&fs_info->freed_extents[0], start, end,
+			  EXTENT_DIRTY);
+	if (ret)
+		goto failure;
+
+	ret = clear_extent_bits(&fs_info->freed_extents[1], start, end,
+			  EXTENT_DIRTY);
+	if (ret)
+		goto failure;
+	mutex_unlock(&fs_info->unused_bg_unpin_mutex);
+
+	return true;
+
+failure:
+	mutex_unlock(&fs_info->unused_bg_unpin_mutex);
+	btrfs_dec_block_group_ro(bg);
+	return false;
+
+}
+
 /*
  * Process the unused_bgs list and remove any that don't have any allocated
  * space inside of them.
@@ -1250,7 +1289,6 @@  void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)

 	spin_lock(&fs_info->unused_bgs_lock);
 	while (!list_empty(&fs_info->unused_bgs)) {
-		u64 start, end;
 		int trimming;

 		block_group = list_first_entry(&fs_info->unused_bgs,
@@ -1329,35 +1367,8 @@  void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)
 		 * We could have pending pinned extents for this block group,
 		 * just delete them, we don't care about them anymore.
 		 */
-		start = block_group->start;
-		end = start + block_group->length - 1;
-		/*
-		 * Hold the unused_bg_unpin_mutex lock to avoid racing with
-		 * btrfs_finish_extent_commit(). If we are at transaction N,
-		 * another task might be running finish_extent_commit() for the
-		 * previous transaction N - 1, and have seen a range belonging
-		 * to the block group in freed_extents[] before we were able to
-		 * clear the whole block group range from freed_extents[]. This
-		 * means that task can lookup for the block group after we
-		 * unpinned it from freed_extents[] and removed it, leading to
-		 * a BUG_ON() at btrfs_unpin_extent_range().
-		 */
-		mutex_lock(&fs_info->unused_bg_unpin_mutex);
-		ret = clear_extent_bits(&fs_info->freed_extents[0], start, end,
-				  EXTENT_DIRTY);
-		if (ret) {
-			mutex_unlock(&fs_info->unused_bg_unpin_mutex);
-			btrfs_dec_block_group_ro(block_group);
+		if (!clean_pinned_extents(block_group))
 			goto end_trans;
-		}
-		ret = clear_extent_bits(&fs_info->freed_extents[1], start, end,
-				  EXTENT_DIRTY);
-		if (ret) {
-			mutex_unlock(&fs_info->unused_bg_unpin_mutex);
-			btrfs_dec_block_group_ro(block_group);
-			goto end_trans;
-		}
-		mutex_unlock(&fs_info->unused_bg_unpin_mutex);

 		/*
 		 * At this point, the block_group is read only and should fail