[20/23] btrfs: don't force commit if we are data

Commit Message

Josef Bacik Jan. 31, 2020, 10:36 p.m. UTC

We used to unconditionally commit the transaction at least 2 times and
then on the 3rd try check against pinned space to make sure committing
the transaction was worth the effort.  This is overkill, we know nobody
is going to steal our reservation, and if we can't make our reservation
with the pinned amount simply bail out.

Signed-off-by: Josef Bacik <[e-mail]>
---
 fs/btrfs/space-info.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

Comments

Nikolay Borisov Feb. 3, 2020, 4:12 p.m. UTC | #1

On 1.02.20 г. 0:36 ч., Josef Bacik wrote:
> We used to unconditionally commit the transaction at least 2 times and
> then on the 3rd try check against pinned space to make sure committing
> the transaction was worth the effort.  This is overkill, we know nobody
> is going to steal our reservation, and if we can't make our reservation
> with the pinned amount simply bail out.
> 
> Signed-off-by: Josef Bacik <[e-mail]>

Reviewed-by: Nikolay Borisov <[e-mail]>

I think the previous patch can be dropped and simply this one applied
after it.

Patch

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index cef14a4d4167..0c2d8e66cf5e 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -421,21 +421,11 @@  static int may_commit_transaction(struct btrfs_fs_info *fs_info,
 	u64 reclaim_bytes = 0;
 	u64 bytes_needed;
 	u64 cur_free_bytes = 0;
-	bool do_commit = false;
 
 	trans = (struct btrfs_trans_handle *)current->journal_info;
 	if (trans)
 		return -EAGAIN;
 
-	/*
-	 * If we are data just force the commit, we aren't likely to do this
-	 * over and over again.
-	 */
-	if (space_info->flags & BTRFS_BLOCK_GROUP_DATA) {
-		do_commit = true;
-		goto check_pinned;
-	}
-
 	spin_lock(&space_info->lock);
 	cur_free_bytes = btrfs_space_info_used(space_info, true);
 	if (cur_free_bytes < space_info->total_bytes)
@@ -460,7 +450,6 @@  static int may_commit_transaction(struct btrfs_fs_info *fs_info,
 	if (!bytes_needed)
 		return 0;
 
-check_pinned:
 	trans = btrfs_join_transaction(fs_info->extent_root);
 	if (IS_ERR(trans))
 		return PTR_ERR(trans);
@@ -470,8 +459,7 @@  static int may_commit_transaction(struct btrfs_fs_info *fs_info,
 	 * we have block groups that are going to be freed, allowing us to
 	 * possibly do a chunk allocation the next loop through.
 	 */
-	if (do_commit ||
-	    test_bit(BTRFS_TRANS_HAVE_FREE_BGS, &trans->transaction->flags) ||
+	if (test_bit(BTRFS_TRANS_HAVE_FREE_BGS, &trans->transaction->flags) ||
 	    __percpu_counter_compare(&space_info->total_bytes_pinned,
 				     bytes_needed,
 				     BTRFS_TOTAL_BYTES_PINNED_BATCH) >= 0)