[PATCH 0/4] Use global rsv stealing for evict and clean things up

[PATCH 0/4] Use global rsv stealing for evict and clean things up

[Josef Bacik]

Hello,

While trying to remove direct access of fs_info->extent_root I noticed we were
passing root into btrfs_reserve_metadata_bytes() for the sole purpose of
stealing from the global reserve if we were doing orphan cleanup.  This isn't
really necessary anymore, but I needed to clean up a few things

1) We have global reserve stealing logic in the flushing code now that does the
   proper ordering already.  We just hadn't converted evict to this yet, so I've
   done that.
2) Since we already do the global reserve stealing as a part of the reservation
   process we don't need the extra check to steal from the global reserve if we
   fail to make our reservation during orphan cleanup.
3) Since we no longer need this logic we don't need the orphan_cleanup_state bit
   in the root so we can remove that.
4) Finally with all of this removed we don't have a need for root in
   btrfs_reserve_metadata_bytes(), so change it to fs_info and change it's main
   callers as well.

With that we've got more consistent global reserve stealing handling in evict,
and I've cleaned up the reservation path so I no longer have to worry about a
couple of places where we were doing
btrfs_reserve_metadata_bytes(root->extent_root).  Thanks,

Josef Bacik (4):
  btrfs: make BTRFS_RESERVE_FLUSH_EVICT use the global rsv stealing code
  btrfs: remove global rsv stealing logic for orphan cleanup
  btrfs: get rid of root->orphan_cleanup_state
  btrfs: change root to fs_info for btrfs_reserve_metadata_bytes

 fs/btrfs/block-group.c    |  2 +-
 fs/btrfs/block-rsv.c      | 12 +++++++-----
 fs/btrfs/block-rsv.h      |  4 ++--
 fs/btrfs/ctree.h          |  9 ++-------
 fs/btrfs/delalloc-space.c |  2 +-
 fs/btrfs/delayed-inode.c  |  2 +-
 fs/btrfs/delayed-ref.c    |  4 ++--
 fs/btrfs/disk-io.c        |  1 -
 fs/btrfs/inode.c          | 21 ++++++++-------------
 fs/btrfs/props.c          |  5 +++--
 fs/btrfs/relocation.c     | 17 +++++++++--------
 fs/btrfs/root-tree.c      |  2 +-
 fs/btrfs/space-info.c     | 34 ++++++++++++++++++++++++----------
 fs/btrfs/space-info.h     |  2 +-
 fs/btrfs/transaction.c    |  4 ++--
 15 files changed, 64 insertions(+), 57 deletions(-)

-- 
2.26.3

[PATCH 1/4] btrfs: make BTRFS_RESERVE_FLUSH_EVICT use the global rsv stealing code

[Josef Bacik]

I forgot to convert this over when I introduced the global reserve
stealing code to the space flushing code.  Evict was simply trying to
make its reservation and then if it failed it would steal from the
global rsv, which is racey because it's outside of the normal ticketing
code.

Fix this by setting ticket->steal if we are BTRFS_RESERVE_FLUSH_EVICT,
and then make the priority flushing path do the steal for us.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/inode.c      | 15 ++++++---------
 fs/btrfs/space-info.c | 24 +++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5fec009fbe63..c783a3e434b9 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5523,7 +5523,6 @@ static struct btrfs_trans_handle *evict_refill_and_join(struct btrfs_root *root,
 							struct btrfs_block_rsv *rsv)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct btrfs_block_rsv *global_rsv = &fs_info->global_block_rsv;
 	struct btrfs_trans_handle *trans;
 	u64 delayed_refs_extra = btrfs_calc_insert_metadata_size(fs_info, 1);
 	int ret;
@@ -5538,18 +5537,16 @@ static struct btrfs_trans_handle *evict_refill_and_join(struct btrfs_root *root,
 	 * above.  We reserve our extra bit here because we generate a ton of
 	 * delayed refs activity by truncating.
 	 *
-	 * If we cannot make our reservation we'll attempt to steal from the
-	 * global reserve, because we really want to be able to free up space.
+	 * BTRFS_RESERVE_FLUSH_EVICT will steal from the global_rsv if it can,
+	 * if we fail to make this reservation we can re-try without the
+	 * delayed_refs_extra so we can make some forward progress.
 	 */
 	ret = btrfs_block_rsv_refill(root, rsv, rsv->size + delayed_refs_extra,
 				     BTRFS_RESERVE_FLUSH_EVICT);
 	if (ret) {
-		/*
-		 * Try to steal from the global reserve if there is space for
-		 * it.
-		 */
-		if (btrfs_check_space_for_delayed_refs(fs_info) ||
-		    btrfs_block_rsv_migrate(global_rsv, rsv, rsv->size, 0)) {
+		ret = btrfs_block_rsv_refill(root, rsv, rsv->size,
+					     BTRFS_RESERVE_FLUSH_EVICT);
+		if (ret) {
 			btrfs_warn(fs_info,
 				   "could not allocate space for delete; will truncate on mount");
 			return ERR_PTR(-ENOSPC);
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 48d77f360a24..c548d34aed28 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1380,6 +1380,22 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info,
 
 	spin_lock(&space_info->lock);
 	ret = ticket->error;
+
+	/*
+	 * If we can steal from the block rsv and we don't have an error and we
+	 * didn't make our reservation then go ahead and try to steal our
+	 * reservation.
+	 */
+	if (ticket->steal && !ret && ticket->bytes) {
+		/*
+		 * If we succeed we need to run btrfs_try_granting_tickets() for
+		 * the same reason as described below.
+		 */
+		if (steal_from_global_rsv(fs_info, space_info, ticket))
+			btrfs_try_granting_tickets(fs_info, space_info);
+	}
+
+
 	if (ticket->bytes || ticket->error) {
 		/*
 		 * We were a priority ticket, so we need to delete ourselves
@@ -1438,6 +1454,12 @@ static inline void maybe_clamp_preempt(struct btrfs_fs_info *fs_info,
 		space_info->clamp = min(space_info->clamp + 1, 8);
 }
 
+static inline bool is_steal_flush_state(enum btrfs_reserve_flush_enum flush)
+{
+	return (flush == BTRFS_RESERVE_FLUSH_ALL_STEAL ||
+		flush == BTRFS_RESERVE_FLUSH_EVICT);
+}
+
 /**
  * Try to reserve bytes from the block_rsv's space
  *
@@ -1511,7 +1533,7 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
 		ticket.error = 0;
 		space_info->reclaim_size += ticket.bytes;
 		init_waitqueue_head(&ticket.wait);
-		ticket.steal = (flush == BTRFS_RESERVE_FLUSH_ALL_STEAL);
+		ticket.steal = is_steal_flush_state(flush);
 		if (trace_btrfs_reserve_ticket_enabled())
 			start_ns = ktime_get_ns();
 
-- 
2.26.3

[PATCH 2/4] btrfs: remove global rsv stealing logic for orphan cleanup

[Josef Bacik]

This is very old code before we were stealing from the global reserve
during evict.  We have proper ways to steal from the global reserve
while we're evicting, so rip out this code as it's no longer necessary.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/space-info.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index c548d34aed28..25579b772639 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1607,16 +1607,9 @@ int btrfs_reserve_metadata_bytes(struct btrfs_root *root,
 				 enum btrfs_reserve_flush_enum flush)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct btrfs_block_rsv *global_rsv = &fs_info->global_block_rsv;
 	int ret;
 
 	ret = __reserve_bytes(fs_info, block_rsv->space_info, orig_bytes, flush);
-	if (ret == -ENOSPC &&
-	    unlikely(root->orphan_cleanup_state == ORPHAN_CLEANUP_STARTED)) {
-		if (block_rsv != global_rsv &&
-		    !btrfs_block_rsv_use_bytes(global_rsv, orig_bytes))
-			ret = 0;
-	}
 	if (ret == -ENOSPC) {
 		trace_btrfs_space_reservation(fs_info, "space_info:enospc",
 					      block_rsv->space_info->flags,
-- 
2.26.3

[PATCH 3/4] btrfs: get rid of root->orphan_cleanup_state

[Josef Bacik]

Now that we don't care about the stage of the orphan_cleanup_state,
simply replace it with a bit on ->state to make sure we don't call the
orphan cleanup every time we wander into this root.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h   | 9 ++-------
 fs/btrfs/disk-io.c | 1 -
 fs/btrfs/inode.c   | 4 +---
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 7553e9dc5f93..01f6a40ba2dd 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -511,11 +511,6 @@ struct btrfs_discard_ctl {
 	atomic64_t discard_bytes_saved;
 };
 
-enum btrfs_orphan_cleanup_state {
-	ORPHAN_CLEANUP_STARTED	= 1,
-	ORPHAN_CLEANUP_DONE	= 2,
-};
-
 void btrfs_init_async_reclaim_work(struct btrfs_fs_info *fs_info);
 
 /* fs_info */
@@ -1110,6 +1105,8 @@ enum {
 	BTRFS_ROOT_HAS_LOG_TREE,
 	/* Qgroup flushing is in progress */
 	BTRFS_ROOT_QGROUP_FLUSHING,
+	/* We started the orphan cleanup for this root. */
+	BTRFS_ROOT_ORPHAN_CLEANUP,
 };
 
 /*
@@ -1178,8 +1175,6 @@ struct btrfs_root {
 	spinlock_t log_extents_lock[2];
 	struct list_head logged_list[2];
 
-	int orphan_cleanup_state;
-
 	spinlock_t inode_lock;
 	/* red-black tree that keeps track of in-memory inodes */
 	struct rb_root inode_tree;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c7254331cf38..5be0ae67ceb7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1144,7 +1144,6 @@ static void __setup_root(struct btrfs_root *root, struct btrfs_fs_info *fs_info,
 	root->node = NULL;
 	root->commit_root = NULL;
 	root->state = 0;
-	root->orphan_cleanup_state = 0;
 
 	root->last_trans = 0;
 	root->free_objectid = 0;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c783a3e434b9..a9ebafb168b5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3475,7 +3475,7 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
 	u64 last_objectid = 0;
 	int ret = 0, nr_unlink = 0;
 
-	if (cmpxchg(&root->orphan_cleanup_state, 0, ORPHAN_CLEANUP_STARTED))
+	if (test_and_set_bit(BTRFS_ROOT_ORPHAN_CLEANUP, &root->state))
 		return 0;
 
 	path = btrfs_alloc_path();
@@ -3633,8 +3633,6 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
 	/* release the path since we're done with it */
 	btrfs_release_path(path);
 
-	root->orphan_cleanup_state = ORPHAN_CLEANUP_DONE;
-
 	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state)) {
 		trans = btrfs_join_transaction(root);
 		if (!IS_ERR(trans))
-- 
2.26.3

[PATCH 4/4] btrfs: change root to fs_info for btrfs_reserve_metadata_bytes

[Josef Bacik]

We used to need the root for btrfs_reserve_metadata_bytes to check the
orphan cleanup state, but we no longer need that, we simply need the
fs_info.  Change btrfs_reserve_metadata_bytes() to use the fs_info, and
change both btrfs_block_rsv_refill() and btrfs_block_rsv_add() to do the
same as they simply call btrfs_reserve_metadata_bytes() and then
manipulate the block_rsv that is being used.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/block-group.c    |  2 +-
 fs/btrfs/block-rsv.c      | 12 +++++++-----
 fs/btrfs/block-rsv.h      |  4 ++--
 fs/btrfs/delalloc-space.c |  2 +-
 fs/btrfs/delayed-inode.c  |  2 +-
 fs/btrfs/delayed-ref.c    |  4 ++--
 fs/btrfs/inode.c          |  4 ++--
 fs/btrfs/props.c          |  5 +++--
 fs/btrfs/relocation.c     | 17 +++++++++--------
 fs/btrfs/root-tree.c      |  2 +-
 fs/btrfs/space-info.c     |  3 +--
 fs/btrfs/space-info.h     |  2 +-
 fs/btrfs/transaction.c    |  4 ++--
 13 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 444e9c89ff3e..2390df7fa345 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -3790,7 +3790,7 @@ static void reserve_chunk_space(struct btrfs_trans_handle *trans,
 	}
 
 	if (!ret) {
-		ret = btrfs_block_rsv_add(fs_info->chunk_root,
+		ret = btrfs_block_rsv_add(fs_info,
 					  &fs_info->chunk_block_rsv,
 					  bytes, BTRFS_RESERVE_NO_FLUSH);
 		if (!ret)
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index 04a6226e0388..ac6fbe75cace 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -208,7 +208,7 @@ void btrfs_free_block_rsv(struct btrfs_fs_info *fs_info,
 	kfree(rsv);
 }
 
-int btrfs_block_rsv_add(struct btrfs_root *root,
+int btrfs_block_rsv_add(struct btrfs_fs_info *fs_info,
 			struct btrfs_block_rsv *block_rsv, u64 num_bytes,
 			enum btrfs_reserve_flush_enum flush)
 {
@@ -217,7 +217,8 @@ int btrfs_block_rsv_add(struct btrfs_root *root,
 	if (num_bytes == 0)
 		return 0;
 
-	ret = btrfs_reserve_metadata_bytes(root, block_rsv, num_bytes, flush);
+	ret = btrfs_reserve_metadata_bytes(fs_info, block_rsv, num_bytes,
+					   flush);
 	if (!ret)
 		btrfs_block_rsv_add_bytes(block_rsv, num_bytes, true);
 
@@ -241,7 +242,7 @@ int btrfs_block_rsv_check(struct btrfs_block_rsv *block_rsv, int min_factor)
 	return ret;
 }
 
-int btrfs_block_rsv_refill(struct btrfs_root *root,
+int btrfs_block_rsv_refill(struct btrfs_fs_info *fs_info,
 			   struct btrfs_block_rsv *block_rsv, u64 min_reserved,
 			   enum btrfs_reserve_flush_enum flush)
 {
@@ -262,7 +263,8 @@ int btrfs_block_rsv_refill(struct btrfs_root *root,
 	if (!ret)
 		return 0;
 
-	ret = btrfs_reserve_metadata_bytes(root, block_rsv, num_bytes, flush);
+	ret = btrfs_reserve_metadata_bytes(fs_info, block_rsv, num_bytes,
+					   flush);
 	if (!ret) {
 		btrfs_block_rsv_add_bytes(block_rsv, num_bytes, false);
 		return 0;
@@ -523,7 +525,7 @@ struct btrfs_block_rsv *btrfs_use_block_rsv(struct btrfs_trans_handle *trans,
 				block_rsv->type, ret);
 	}
 try_reserve:
-	ret = btrfs_reserve_metadata_bytes(root, block_rsv, blocksize,
+	ret = btrfs_reserve_metadata_bytes(fs_info, block_rsv, blocksize,
 					   BTRFS_RESERVE_NO_FLUSH);
 	if (!ret)
 		return block_rsv;
diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h
index 0b6ae5302837..07d61c2c5d28 100644
--- a/fs/btrfs/block-rsv.h
+++ b/fs/btrfs/block-rsv.h
@@ -57,11 +57,11 @@ void btrfs_init_metadata_block_rsv(struct btrfs_fs_info *fs_info,
 				   unsigned short type);
 void btrfs_free_block_rsv(struct btrfs_fs_info *fs_info,
 			  struct btrfs_block_rsv *rsv);
-int btrfs_block_rsv_add(struct btrfs_root *root,
+int btrfs_block_rsv_add(struct btrfs_fs_info *fs_info,
 			struct btrfs_block_rsv *block_rsv, u64 num_bytes,
 			enum btrfs_reserve_flush_enum flush);
 int btrfs_block_rsv_check(struct btrfs_block_rsv *block_rsv, int min_factor);
-int btrfs_block_rsv_refill(struct btrfs_root *root,
+int btrfs_block_rsv_refill(struct btrfs_fs_info *fs_info,
 			   struct btrfs_block_rsv *block_rsv, u64 min_reserved,
 			   enum btrfs_reserve_flush_enum flush);
 int btrfs_block_rsv_migrate(struct btrfs_block_rsv *src_rsv,
diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c
index 2059d1504149..bca438c7c972 100644
--- a/fs/btrfs/delalloc-space.c
+++ b/fs/btrfs/delalloc-space.c
@@ -331,7 +331,7 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
 	ret = btrfs_qgroup_reserve_meta_prealloc(root, qgroup_reserve, true);
 	if (ret)
 		return ret;
-	ret = btrfs_reserve_metadata_bytes(root, block_rsv, meta_reserve, flush);
+	ret = btrfs_reserve_metadata_bytes(fs_info, block_rsv, meta_reserve, flush);
 	if (ret) {
 		btrfs_qgroup_free_meta_prealloc(root, qgroup_reserve);
 		return ret;
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index e164766dcc38..6f134f2c5e68 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -629,7 +629,7 @@ static int btrfs_delayed_inode_reserve_metadata(
 					  BTRFS_QGROUP_RSV_META_PREALLOC, true);
 		if (ret < 0)
 			return ret;
-		ret = btrfs_block_rsv_add(root, dst_rsv, num_bytes,
+		ret = btrfs_block_rsv_add(fs_info, dst_rsv, num_bytes,
 					  BTRFS_RESERVE_NO_FLUSH);
 		/* NO_FLUSH could only fail with -ENOSPC */
 		ASSERT(ret == 0 || ret == -ENOSPC);
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index cca7e85e32dd..80950a409395 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -191,8 +191,8 @@ int btrfs_delayed_refs_rsv_refill(struct btrfs_fs_info *fs_info,
 	if (!num_bytes)
 		return 0;
 
-	ret = btrfs_reserve_metadata_bytes(fs_info->extent_root, block_rsv,
-					   num_bytes, flush);
+	ret = btrfs_reserve_metadata_bytes(fs_info, block_rsv, num_bytes,
+					   flush);
 	if (ret)
 		return ret;
 	btrfs_block_rsv_add_bytes(block_rsv, num_bytes, 0);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a9ebafb168b5..9e81fd94ab09 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5539,10 +5539,10 @@ static struct btrfs_trans_handle *evict_refill_and_join(struct btrfs_root *root,
 	 * if we fail to make this reservation we can re-try without the
 	 * delayed_refs_extra so we can make some forward progress.
 	 */
-	ret = btrfs_block_rsv_refill(root, rsv, rsv->size + delayed_refs_extra,
+	ret = btrfs_block_rsv_refill(fs_info, rsv, rsv->size + delayed_refs_extra,
 				     BTRFS_RESERVE_FLUSH_EVICT);
 	if (ret) {
-		ret = btrfs_block_rsv_refill(root, rsv, rsv->size,
+		ret = btrfs_block_rsv_refill(fs_info, rsv, rsv->size,
 					     BTRFS_RESERVE_FLUSH_EVICT);
 		if (ret) {
 			btrfs_warn(fs_info,
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index b1cb5a8c2999..141837a1c347 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -377,8 +377,9 @@ static int inherit_props(struct btrfs_trans_handle *trans,
 		 */
 		if (need_reserve) {
 			num_bytes = btrfs_calc_insert_metadata_size(fs_info, 1);
-			ret = btrfs_block_rsv_add(root, trans->block_rsv,
-					num_bytes, BTRFS_RESERVE_NO_FLUSH);
+			ret = btrfs_block_rsv_add(fs_info, trans->block_rsv,
+						  num_bytes,
+						  BTRFS_RESERVE_NO_FLUSH);
 			if (ret)
 				return ret;
 		}
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 33a0ee7ac590..f167ce645016 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1736,7 +1736,8 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
 	memset(&next_key, 0, sizeof(next_key));
 
 	while (1) {
-		ret = btrfs_block_rsv_refill(root, rc->block_rsv, min_reserved,
+		ret = btrfs_block_rsv_refill(fs_info, rc->block_rsv,
+					     min_reserved,
 					     BTRFS_RESERVE_FLUSH_LIMIT);
 		if (ret)
 			goto out;
@@ -1855,7 +1856,7 @@ int prepare_to_merge(struct reloc_control *rc, int err)
 again:
 	if (!err) {
 		num_bytes = rc->merging_rsv_size;
-		ret = btrfs_block_rsv_add(root, rc->block_rsv, num_bytes,
+		ret = btrfs_block_rsv_add(fs_info, rc->block_rsv, num_bytes,
 					  BTRFS_RESERVE_FLUSH_ALL);
 		if (ret)
 			err = ret;
@@ -2323,8 +2324,8 @@ static int reserve_metadata_space(struct btrfs_trans_handle *trans,
 	 * If we get an enospc just kick back -EAGAIN so we know to drop the
 	 * transaction and try to refill when we can flush all the things.
 	 */
-	ret = btrfs_block_rsv_refill(root, rc->block_rsv, num_bytes,
-				BTRFS_RESERVE_FLUSH_LIMIT);
+	ret = btrfs_block_rsv_refill(fs_info, rc->block_rsv, num_bytes,
+				     BTRFS_RESERVE_FLUSH_LIMIT);
 	if (ret) {
 		tmp = fs_info->nodesize * RELOCATION_RESERVED_NODES;
 		while (tmp <= rc->reserved_bytes)
@@ -3550,7 +3551,7 @@ int prepare_to_relocate(struct reloc_control *rc)
 	rc->reserved_bytes = 0;
 	rc->block_rsv->size = rc->extent_root->fs_info->nodesize *
 			      RELOCATION_RESERVED_NODES;
-	ret = btrfs_block_rsv_refill(rc->extent_root,
+	ret = btrfs_block_rsv_refill(rc->extent_root->fs_info,
 				     rc->block_rsv, rc->block_rsv->size,
 				     BTRFS_RESERVE_FLUSH_ALL);
 	if (ret)
@@ -3598,9 +3599,9 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
 
 	while (1) {
 		rc->reserved_bytes = 0;
-		ret = btrfs_block_rsv_refill(rc->extent_root,
-					rc->block_rsv, rc->block_rsv->size,
-					BTRFS_RESERVE_FLUSH_ALL);
+		ret = btrfs_block_rsv_refill(fs_info, rc->block_rsv,
+					     rc->block_rsv->size,
+					     BTRFS_RESERVE_FLUSH_ALL);
 		if (ret) {
 			err = ret;
 			break;
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index 702dc5441f03..df15e987fe20 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -503,7 +503,7 @@ int btrfs_subvolume_reserve_metadata(struct btrfs_root *root,
 	num_bytes = btrfs_calc_insert_metadata_size(fs_info, items);
 	rsv->space_info = btrfs_find_space_info(fs_info,
 					    BTRFS_BLOCK_GROUP_METADATA);
-	ret = btrfs_block_rsv_add(root, rsv, num_bytes,
+	ret = btrfs_block_rsv_add(fs_info, rsv, num_bytes,
 				  BTRFS_RESERVE_FLUSH_ALL);
 
 	if (ret == -ENOSPC && use_global_rsv)
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 25579b772639..f74b4063772e 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1601,12 +1601,11 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
  * regain reservations will be made and this will fail if there is not enough
  * space already.
  */
-int btrfs_reserve_metadata_bytes(struct btrfs_root *root,
+int btrfs_reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
 				 struct btrfs_block_rsv *block_rsv,
 				 u64 orig_bytes,
 				 enum btrfs_reserve_flush_enum flush)
 {
-	struct btrfs_fs_info *fs_info = root->fs_info;
 	int ret;
 
 	ret = __reserve_bytes(fs_info, block_rsv->space_info, orig_bytes, flush);
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index cb5056472e79..d841fed73492 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -123,7 +123,7 @@ void btrfs_clear_space_info_full(struct btrfs_fs_info *info);
 void btrfs_dump_space_info(struct btrfs_fs_info *fs_info,
 			   struct btrfs_space_info *info, u64 bytes,
 			   int dump_block_groups);
-int btrfs_reserve_metadata_bytes(struct btrfs_root *root,
+int btrfs_reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
 				 struct btrfs_block_rsv *block_rsv,
 				 u64 orig_bytes,
 				 enum btrfs_reserve_flush_enum flush);
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 1c3a1189c0bd..b958e0fdfe10 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -628,7 +628,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
 			reloc_reserved = true;
 		}
 
-		ret = btrfs_block_rsv_add(root, rsv, num_bytes, flush);
+		ret = btrfs_block_rsv_add(fs_info, rsv, num_bytes, flush);
 		if (ret)
 			goto reserve_fail;
 		if (delayed_refs_bytes) {
@@ -1578,7 +1578,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	btrfs_reloc_pre_snapshot(pending, &to_reserve);
 
 	if (to_reserve > 0) {
-		pending->error = btrfs_block_rsv_add(root,
+		pending->error = btrfs_block_rsv_add(fs_info,
 						     &pending->block_rsv,
 						     to_reserve,
 						     BTRFS_RESERVE_NO_FLUSH);
-- 
2.26.3

Re: [PATCH 1/4] btrfs: make BTRFS_RESERVE_FLUSH_EVICT use the global rsv stealing code

[Nikolay Borisov]

On 28.10.21 г. 22:50, Josef Bacik wrote:
> I forgot to convert this over when I introduced the global reserve
> stealing code to the space flushing code.  Evict was simply trying to
> make its reservation and then if it failed it would steal from the
> global rsv, which is racey because it's outside of the normal ticketing
> code.
> 
> Fix this by setting ticket->steal if we are BTRFS_RESERVE_FLUSH_EVICT,
> and then make the priority flushing path do the steal for us.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/inode.c      | 15 ++++++---------
>  fs/btrfs/space-info.c | 24 +++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 

<snip>

> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 48d77f360a24..c548d34aed28 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -1380,6 +1380,22 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info,
>  
>  	spin_lock(&space_info->lock);
>  	ret = ticket->error;
> +
> +	/*
> +	 * If we can steal from the block rsv and we don't have an error and we
> +	 * didn't make our reservation then go ahead and try to steal our
> +	 * reservation.
> +	 */
> +	if (ticket->steal && !ret && ticket->bytes) {
> +		/*
> +		 * If we succeed we need to run btrfs_try_granting_tickets() for
> +		 * the same reason as described below.
> +		 */
> +		if (steal_from_global_rsv(fs_info, space_info, ticket))
> +			btrfs_try_granting_tickets(fs_info, space_info);
> +	}

Instead of having this code here, wouldn't it be better if it's moved in
priority_reclaim_metadata_space since this behavior is really part of
the flushing behavior, similarly to how maybe_fail_all_tickets is called
from the ordinary reclaim path? Actually reading the comment about
calling btrfs_try_granting_tickets makes me think that the !list_empty()
condition should also be moved in priority_reclaim_metadata_space as
those behaviors only pertain to priority tickets, since ordinary ticket
will either have been satisfied or failed due to maybe_fail_all_tickets
always removing ordinary tickets from the ticket list hence the
list_empty() will always be true for them.


If I have to be even more nit-picky the priority flushing path is
priority_reclaim_metadata_space and not handle_reserve_ticket itself. So
the code is slightly contradicting the last sentence in the changelog :)


> +
> +
>  	if (ticket->bytes || ticket->error) {
>  		/*
>  		 * We were a priority ticket, so we need to delete ourselves
> @@ -1438,6 +1454,12 @@ static inline void maybe_clamp_preempt(struct btrfs_fs_info *fs_info,
>  		space_info->clamp = min(space_info->clamp + 1, 8);
>  }
>  
> +static inline bool is_steal_flush_state(enum btrfs_reserve_flush_enum flush)

nint: A better name is should_stable or can_steal

> +{
> +	return (flush == BTRFS_RESERVE_FLUSH_ALL_STEAL ||
> +		flush == BTRFS_RESERVE_FLUSH_EVICT);
> +}

<snip>

Re: [PATCH 2/4] btrfs: remove global rsv stealing logic for orphan cleanup

[Nikolay Borisov]

On 28.10.21 г. 22:50, Josef Bacik wrote:
> This is very old code before we were stealing from the global reserve
> during evict.  We have proper ways to steal from the global reserve
> while we're evicting, so rip out this code as it's no longer necessary.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

Re: [PATCH 3/4] btrfs: get rid of root->orphan_cleanup_state

[Nikolay Borisov]

On 28.10.21 г. 22:50, Josef Bacik wrote:
> Now that we don't care about the stage of the orphan_cleanup_state,
> simply replace it with a bit on ->state to make sure we don't call the
> orphan cleanup every time we wander into this root.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/ctree.h   | 9 ++-------
>  fs/btrfs/disk-io.c | 1 -
>  fs/btrfs/inode.c   | 4 +---
>  3 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 7553e9dc5f93..01f6a40ba2dd 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -511,11 +511,6 @@ struct btrfs_discard_ctl {
>  	atomic64_t discard_bytes_saved;
>  };
>  
> -enum btrfs_orphan_cleanup_state {
> -	ORPHAN_CLEANUP_STARTED	= 1,
> -	ORPHAN_CLEANUP_DONE	= 2,
> -};
> -
>  void btrfs_init_async_reclaim_work(struct btrfs_fs_info *fs_info);
>  
>  /* fs_info */
> @@ -1110,6 +1105,8 @@ enum {
>  	BTRFS_ROOT_HAS_LOG_TREE,
>  	/* Qgroup flushing is in progress */
>  	BTRFS_ROOT_QGROUP_FLUSHING,
> +	/* We started the orphan cleanup for this root. */
> +	BTRFS_ROOT_ORPHAN_CLEANUP,
>  };
>  
>  /*
> @@ -1178,8 +1175,6 @@ struct btrfs_root {
>  	spinlock_t log_extents_lock[2];
>  	struct list_head logged_list[2];
>  
> -	int orphan_cleanup_state;
> -
>  	spinlock_t inode_lock;
>  	/* red-black tree that keeps track of in-memory inodes */
>  	struct rb_root inode_tree;
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c7254331cf38..5be0ae67ceb7 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1144,7 +1144,6 @@ static void __setup_root(struct btrfs_root *root, struct btrfs_fs_info *fs_info,
>  	root->node = NULL;
>  	root->commit_root = NULL;
>  	root->state = 0;
> -	root->orphan_cleanup_state = 0;
>  
>  	root->last_trans = 0;
>  	root->free_objectid = 0;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index c783a3e434b9..a9ebafb168b5 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3475,7 +3475,7 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
>  	u64 last_objectid = 0;
>  	int ret = 0, nr_unlink = 0;
>  
> -	if (cmpxchg(&root->orphan_cleanup_state, 0, ORPHAN_CLEANUP_STARTED))
> +	if (test_and_set_bit(BTRFS_ROOT_ORPHAN_CLEANUP, &root->state))
>  		return 0;
>  
>  	path = btrfs_alloc_path();
> @@ -3633,8 +3633,6 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
>  	/* release the path since we're done with it */
>  	btrfs_release_path(path);
>  
> -	root->orphan_cleanup_state = ORPHAN_CLEANUP_DONE;

Don't you need to clear the bit at this stage?

> -
>  	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state)) {
>  		trans = btrfs_join_transaction(root);
>  		if (!IS_ERR(trans))
> 

Re: [PATCH 3/4] btrfs: get rid of root->orphan_cleanup_state

[Nikolay Borisov]

On 4.11.21 г. 14:55, Nikolay Borisov wrote:
> 
> 
> On 28.10.21 г. 22:50, Josef Bacik wrote:
>> Now that we don't care about the stage of the orphan_cleanup_state,
>> simply replace it with a bit on ->state to make sure we don't call the
>> orphan cleanup every time we wander into this root.

<snip>

> 
> Don't you need to clear the bit at this stage?

To answer my own question : No, this retain the old behavior where once
orpahn cleanup is set to done the cmpxchg will always fail for the
duration of this mount, same happens when we simply set the flag. So

Reviewed-by: Nikolay Borisov <nborisov@suse.com>


> 
>> -
>>  	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state)) {
>>  		trans = btrfs_join_transaction(root);
>>  		if (!IS_ERR(trans))
>>
> 

Re: [PATCH 4/4] btrfs: change root to fs_info for btrfs_reserve_metadata_bytes

[Nikolay Borisov]

On 28.10.21 г. 22:50, Josef Bacik wrote:
> We used to need the root for btrfs_reserve_metadata_bytes to check the
> orphan cleanup state, but we no longer need that, we simply need the
> fs_info.  Change btrfs_reserve_metadata_bytes() to use the fs_info, and
> change both btrfs_block_rsv_refill() and btrfs_block_rsv_add() to do the
> same as they simply call btrfs_reserve_metadata_bytes() and then
> manipulate the block_rsv that is being used.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>


Reviewed-by: Nikolay Borisov <nborisov@suse.com>