[PATCH 0/8 V2] Add delayed-refs support to btrfs-progs

[PATCH 0/8 V2] Add delayed-refs support to btrfs-progs

[Nikolay Borisov]

Hello, 

Here is the second version of the delayed refs for progs support. The first 
version can be found here [1]. I've taken into account all the feedback from 
Misono and have verified the code is working and rebased it atop btrfs-progs
4.17.1.

Changes since v1: 
 * Removed num_entries variable from delayed ref root
 
 * Added a patch to refactor btrfs_write_dirty_block_groups and subsequently 
 changed when this function is called to fix an issue reported by Misono. I
 verified that 'make test-fsck TEST_ENABLE_OVERRIDE=true TEST_ARGS_CHECK=--mode=lowmem'
 produces no errors

 * Added 2 patches which remove the newly added adapter functions at the 
 beggining of the series, following the wiring up of the delayed refs 
 infrastructured. The first one (dealing with __free_extent2) is trivial, while
 the second one (for alloc_reserved_tree_block2) is a bit more involved, since
 I've opted to merge the two functions. 

 * Rebased atop latest btrfs-progs release - 4.17.1

 * Dropped patches which have been merged in the mean time


[1] https://www.spinics.net/lists/linux-btrfs/msg79173.html

Nikolay Borisov (8):
  btrfs-progs: Add __free_extent2 function
  btrfs-progs: Add alloc_reserved_tree_block2 function
  btrfs-progs: Add delayed refs infrastructure
  btrfs-progs: Make btrfs_write_dirty_block_groups take only trans
    argument
  btrfs-progs: Wire up delayed refs
  btrfs-progs: Remove old delayed refs infrastructure
  btrfs-progs: Remove __free_extent2
  btrfs-progs: Merge alloc_reserved_tree_block(2|)

 Makefile      |   3 +-
 check/main.c  |   3 +-
 ctree.h       |   8 +-
 delayed-ref.c | 607 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 delayed-ref.h | 208 ++++++++++++++++++++
 disk-io.c     |   2 -
 extent-tree.c | 575 +++++++++++++++++++++++++++++++-----------------------
 kerncompat.h  |   8 +
 transaction.c |  29 ++-
 transaction.h |   4 +
 10 files changed, 1199 insertions(+), 248 deletions(-)
 create mode 100644 delayed-ref.c
 create mode 100644 delayed-ref.h

-- 
2.7.4

[PATCH 1/8] btrfs-progs: Add __free_extent2 function

[Nikolay Borisov]

This is a simple adapter to convert the arguments delayed ref arguments
to the existing arguments of __free_extent.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 extent-tree.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/extent-tree.c b/extent-tree.c
index 5d49af5a901e..34409e600087 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -2136,6 +2136,17 @@ void btrfs_unpin_extent(struct btrfs_fs_info *fs_info,
 	update_pinned_extents(fs_info, bytenr, num_bytes, 0);
 }
 
+static int __free_extent2(struct btrfs_trans_handle *trans,
+			  struct btrfs_delayed_ref_node *node,
+			  struct btrfs_delayed_extent_op *extent_op)
+{
+
+	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
+
+	return __free_extent(trans, node->bytenr, node->num_bytes,
+			     ref->parent, ref->root, ref->level, 0, 1);
+}
+
 /*
  * remove an extent from the root, returns 0 on success
  */
-- 
2.7.4

[PATCH 2/8] btrfs-progs: Add alloc_reserved_tree_block2 function

[Nikolay Borisov]

This is a simple adapter function to convert the delayed-refs structures
to the current arguments of alloc_reserved_tree_block.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 extent-tree.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/extent-tree.c b/extent-tree.c
index 34409e600087..f7b59f84bf3d 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -2687,6 +2687,30 @@ int btrfs_reserve_extent(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
+static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
+				      struct btrfs_delayed_ref_node *node,
+				      struct btrfs_delayed_extent_op *extent_op)
+{
+
+	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
+	struct btrfs_key ins;
+	bool skinny_metadata = btrfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
+
+	ins.objectid = node->bytenr;
+	if (skinny_metadata) {
+		ins.offset = ref->level;
+		ins.type = BTRFS_METADATA_ITEM_KEY;
+	} else {
+		ins.offset = node->num_bytes;
+		ins.type = BTRFS_EXTENT_ITEM_KEY;
+	}
+
+	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
+					 extent_op->flags_to_set,
+					 &extent_op->key, ref->level, &ins);
+
+}
+
 static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
 				     u64 root_objectid, u64 generation,
 				     u64 flags, struct btrfs_disk_key *key,
-- 
2.7.4

[PATCH 3/8] btrfs-progs: Add delayed refs infrastructure

[Nikolay Borisov]

This commit pulls those portions of the kernel implementation of
delayed refs which are necessary to have them working in user-space.
I've done the following modifications:

1. Replaced all kmem_cache_alloc calls to kmalloc.

2. Removed all locking-related code, since we are single threaded in
userspace.

3. Removed code which deals with data refs - delayed refs in user space
are going to be used only for cowonly trees.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 Makefile      |   3 +-
 ctree.h       |   3 +
 delayed-ref.c | 607 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 delayed-ref.h | 208 ++++++++++++++++++++
 extent-tree.c | 226 ++++++++++++++++++++++
 kerncompat.h  |   8 +
 transaction.h |   4 +
 7 files changed, 1058 insertions(+), 1 deletion(-)
 create mode 100644 delayed-ref.c
 create mode 100644 delayed-ref.h

diff --git a/Makefile b/Makefile
index fcfc815a2a5b..f4ab14ea74c8 100644
--- a/Makefile
+++ b/Makefile
@@ -116,7 +116,8 @@ objects = ctree.o disk-io.o kernel-lib/radix-tree.o extent-tree.o print-tree.o \
 	  qgroup.o free-space-cache.o kernel-lib/list_sort.o props.o \
 	  kernel-shared/ulist.o qgroup-verify.o backref.o string-table.o task-utils.o \
 	  inode.o file.o find-root.o free-space-tree.o help.o send-dump.o \
-	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o
+	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o \
+	  delayed-ref.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
 	       cmds-quota.o cmds-qgroup.o cmds-replace.o check/main.o \
diff --git a/ctree.h b/ctree.h
index 4719962df67d..5242595fe355 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2790,4 +2790,7 @@ int btrfs_punch_hole(struct btrfs_trans_handle *trans,
 int btrfs_read_file(struct btrfs_root *root, u64 ino, u64 start, int len,
 		    char *dest);
 
+/* extent-tree.c */
+int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans, unsigned long nr);
+
 #endif
diff --git a/delayed-ref.c b/delayed-ref.c
new file mode 100644
index 000000000000..e8123436a58f
--- /dev/null
+++ b/delayed-ref.c
@@ -0,0 +1,607 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2009 Oracle.  All rights reserved.
+ */
+
+#include "ctree.h"
+#include "btrfs-list.h"
+#include "delayed-ref.h"
+#include "transaction.h"
+
+/*
+ * delayed back reference update tracking.  For subvolume trees
+ * we queue up extent allocations and backref maintenance for
+ * delayed processing.   This avoids deep call chains where we
+ * add extents in the middle of btrfs_search_slot, and it allows
+ * us to buffer up frequently modified backrefs in an rb tree instead
+ * of hammering updates on the extent allocation tree.
+ */
+
+/*
+ * compare two delayed tree backrefs with same bytenr and type
+ */
+static int comp_tree_refs(struct btrfs_delayed_tree_ref *ref1,
+			  struct btrfs_delayed_tree_ref *ref2)
+{
+	if (ref1->node.type == BTRFS_TREE_BLOCK_REF_KEY) {
+		if (ref1->root < ref2->root)
+			return -1;
+		if (ref1->root > ref2->root)
+			return 1;
+	} else {
+		if (ref1->parent < ref2->parent)
+			return -1;
+		if (ref1->parent > ref2->parent)
+			return 1;
+	}
+	return 0;
+}
+
+static int comp_refs(struct btrfs_delayed_ref_node *ref1,
+		     struct btrfs_delayed_ref_node *ref2,
+		     bool check_seq)
+{
+	int ret = 0;
+
+	if (ref1->type < ref2->type)
+		return -1;
+	if (ref1->type > ref2->type)
+		return 1;
+	if (ref1->type == BTRFS_TREE_BLOCK_REF_KEY ||
+	    ref1->type == BTRFS_SHARED_BLOCK_REF_KEY)
+		ret = comp_tree_refs(btrfs_delayed_node_to_tree_ref(ref1),
+				     btrfs_delayed_node_to_tree_ref(ref2));
+	else
+		BUG();
+
+	if (ret)
+		return ret;
+	if (check_seq) {
+		if (ref1->seq < ref2->seq)
+			return -1;
+		if (ref1->seq > ref2->seq)
+			return 1;
+	}
+	return 0;
+}
+
+/* insert a new ref to head ref rbtree */
+static struct btrfs_delayed_ref_head *htree_insert(struct rb_root *root,
+						   struct rb_node *node)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *parent_node = NULL;
+	struct btrfs_delayed_ref_head *entry;
+	struct btrfs_delayed_ref_head *ins;
+	u64 bytenr;
+
+	ins = rb_entry(node, struct btrfs_delayed_ref_head, href_node);
+	bytenr = ins->bytenr;
+	while (*p) {
+		parent_node = *p;
+		entry = rb_entry(parent_node, struct btrfs_delayed_ref_head,
+				 href_node);
+
+		if (bytenr < entry->bytenr)
+			p = &(*p)->rb_left;
+		else if (bytenr > entry->bytenr)
+			p = &(*p)->rb_right;
+		else
+			return entry;
+	}
+
+	rb_link_node(node, parent_node, p);
+	rb_insert_color(node, root);
+	return NULL;
+}
+
+static struct btrfs_delayed_ref_node* tree_insert(struct rb_root *root,
+		struct btrfs_delayed_ref_node *ins)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *node = &ins->ref_node;
+	struct rb_node *parent_node = NULL;
+	struct btrfs_delayed_ref_node *entry;
+
+	while (*p) {
+		int comp;
+
+		parent_node = *p;
+		entry = rb_entry(parent_node, struct btrfs_delayed_ref_node,
+				 ref_node);
+		comp = comp_refs(ins, entry, true);
+		if (comp < 0)
+			p = &(*p)->rb_left;
+		else if (comp > 0)
+			p = &(*p)->rb_right;
+		else
+			return entry;
+	}
+
+	rb_link_node(node, parent_node, p);
+	rb_insert_color(node, root);
+	return NULL;
+}
+
+/*
+ * find an head entry based on bytenr. This returns the delayed ref
+ * head if it was able to find one, or NULL if nothing was in that spot.
+ * If return_bigger is given, the next bigger entry is returned if no exact
+ * match is found.
+ */
+static struct btrfs_delayed_ref_head *
+find_ref_head(struct rb_root *root, u64 bytenr,
+	      int return_bigger)
+{
+	struct rb_node *n;
+	struct btrfs_delayed_ref_head *entry;
+
+	n = root->rb_node;
+	entry = NULL;
+	while (n) {
+		entry = rb_entry(n, struct btrfs_delayed_ref_head, href_node);
+
+		if (bytenr < entry->bytenr)
+			n = n->rb_left;
+		else if (bytenr > entry->bytenr)
+			n = n->rb_right;
+		else
+			return entry;
+	}
+	if (entry && return_bigger) {
+		if (bytenr > entry->bytenr) {
+			n = rb_next(&entry->href_node);
+			if (!n)
+				n = rb_first(root);
+			entry = rb_entry(n, struct btrfs_delayed_ref_head,
+					 href_node);
+			return entry;
+		}
+		return entry;
+	}
+	return NULL;
+}
+
+static inline void drop_delayed_ref(struct btrfs_trans_handle *trans,
+				    struct btrfs_delayed_ref_root *delayed_refs,
+				    struct btrfs_delayed_ref_head *head,
+				    struct btrfs_delayed_ref_node *ref)
+{
+	rb_erase(&ref->ref_node, &head->ref_tree);
+	RB_CLEAR_NODE(&ref->ref_node);
+	if (!list_empty(&ref->add_list))
+		list_del(&ref->add_list);
+	ref->in_tree = 0;
+	btrfs_put_delayed_ref(ref);
+	if (trans->delayed_ref_updates)
+		trans->delayed_ref_updates--;
+}
+
+static bool merge_ref(struct btrfs_trans_handle *trans,
+		      struct btrfs_delayed_ref_root *delayed_refs,
+		      struct btrfs_delayed_ref_head *head,
+		      struct btrfs_delayed_ref_node *ref,
+		      u64 seq)
+{
+	struct btrfs_delayed_ref_node *next;
+	struct rb_node *node = rb_next(&ref->ref_node);
+	bool done = false;
+
+	while (!done && node) {
+		int mod;
+
+		next = rb_entry(node, struct btrfs_delayed_ref_node, ref_node);
+		node = rb_next(node);
+		if (seq && next->seq >= seq)
+			break;
+		if (comp_refs(ref, next, false))
+			break;
+
+		if (ref->action == next->action) {
+			mod = next->ref_mod;
+		} else {
+			if (ref->ref_mod < next->ref_mod) {
+				swap(ref, next);
+				done = true;
+			}
+			mod = -next->ref_mod;
+		}
+
+		drop_delayed_ref(trans, delayed_refs, head, next);
+		ref->ref_mod += mod;
+		if (ref->ref_mod == 0) {
+			drop_delayed_ref(trans, delayed_refs, head, ref);
+			done = true;
+		} else {
+			/*
+			 * Can't have multiples of the same ref on a tree block.
+			 */
+			WARN_ON(ref->type == BTRFS_TREE_BLOCK_REF_KEY ||
+				ref->type == BTRFS_SHARED_BLOCK_REF_KEY);
+		}
+	}
+
+	return done;
+}
+
+void btrfs_merge_delayed_refs(struct btrfs_trans_handle *trans,
+			      struct btrfs_delayed_ref_root *delayed_refs,
+			      struct btrfs_delayed_ref_head *head)
+{
+	struct btrfs_delayed_ref_node *ref;
+	struct rb_node *node;
+
+	if (RB_EMPTY_ROOT(&head->ref_tree))
+		return;
+
+	/* We don't have too many refs to merge for data. */
+	if (head->is_data)
+		return;
+
+again:
+	for (node = rb_first(&head->ref_tree); node; node = rb_next(node)) {
+		ref = rb_entry(node, struct btrfs_delayed_ref_node, ref_node);
+		if (merge_ref(trans, delayed_refs, head, ref, 0))
+			goto again;
+	}
+}
+
+struct btrfs_delayed_ref_head *
+btrfs_select_ref_head(struct btrfs_trans_handle *trans)
+{
+	struct btrfs_delayed_ref_root *delayed_refs;
+	struct btrfs_delayed_ref_head *head;
+	u64 start;
+	bool loop = false;
+
+	delayed_refs = &trans->delayed_refs;
+
+again:
+	start = delayed_refs->run_delayed_start;
+	head = find_ref_head(&delayed_refs->href_root, start, 1);
+	if (!head && !loop) {
+		delayed_refs->run_delayed_start = 0;
+		start = 0;
+		loop = true;
+		head = find_ref_head(&delayed_refs->href_root, start, 1);
+		if (!head)
+			return NULL;
+	} else if (!head && loop) {
+		return NULL;
+	}
+
+	while (head->processing) {
+		struct rb_node *node;
+
+		node = rb_next(&head->href_node);
+		if (!node) {
+			if (loop)
+				return NULL;
+			delayed_refs->run_delayed_start = 0;
+			start = 0;
+			loop = true;
+			goto again;
+		}
+		head = rb_entry(node, struct btrfs_delayed_ref_head,
+				href_node);
+	}
+
+	head->processing = 1;
+	WARN_ON(delayed_refs->num_heads_ready == 0);
+	delayed_refs->num_heads_ready--;
+	delayed_refs->run_delayed_start = head->bytenr +
+		head->num_bytes;
+	return head;
+}
+
+/*
+ * Helper to insert the ref_node to the tail or merge with tail.
+ *
+ * Return 0 for insert.
+ * Return >0 for merge.
+ */
+static int insert_delayed_ref(struct btrfs_trans_handle *trans,
+			      struct btrfs_delayed_ref_root *root,
+			      struct btrfs_delayed_ref_head *href,
+			      struct btrfs_delayed_ref_node *ref)
+{
+	struct btrfs_delayed_ref_node *exist;
+	int mod;
+	int ret = 0;
+
+	exist = tree_insert(&href->ref_tree, ref);
+	if (!exist)
+		goto inserted;
+
+	/* Now we are sure we can merge */
+	ret = 1;
+	if (exist->action == ref->action) {
+		mod = ref->ref_mod;
+	} else {
+		/* Need to change action */
+		if (exist->ref_mod < ref->ref_mod) {
+			exist->action = ref->action;
+			mod = -exist->ref_mod;
+			exist->ref_mod = ref->ref_mod;
+			if (ref->action == BTRFS_ADD_DELAYED_REF)
+				list_add_tail(&exist->add_list,
+					      &href->ref_add_list);
+			else if (ref->action == BTRFS_DROP_DELAYED_REF) {
+				ASSERT(!list_empty(&exist->add_list));
+				list_del(&exist->add_list);
+			} else {
+				ASSERT(0);
+			}
+		} else
+			mod = -ref->ref_mod;
+	}
+	exist->ref_mod += mod;
+
+	/* remove existing tail if its ref_mod is zero */
+	if (exist->ref_mod == 0)
+		drop_delayed_ref(trans, root, href, exist);
+	return ret;
+inserted:
+	if (ref->action == BTRFS_ADD_DELAYED_REF)
+		list_add_tail(&ref->add_list, &href->ref_add_list);
+	trans->delayed_ref_updates++;
+	return ret;
+}
+
+/*
+ * helper function to update the accounting in the head ref
+ * existing and update must have the same bytenr
+ */
+static noinline void
+update_existing_head_ref(struct btrfs_delayed_ref_root *delayed_refs,
+			 struct btrfs_delayed_ref_head *existing,
+			 struct btrfs_delayed_ref_head *update,
+			 int *old_ref_mod_ret)
+{
+	int old_ref_mod;
+
+	BUG_ON(existing->is_data != update->is_data);
+
+	if (update->must_insert_reserved) {
+		/* if the extent was freed and then
+		 * reallocated before the delayed ref
+		 * entries were processed, we can end up
+		 * with an existing head ref without
+		 * the must_insert_reserved flag set.
+		 * Set it again here
+		 */
+		existing->must_insert_reserved = update->must_insert_reserved;
+
+		/*
+		 * update the num_bytes so we make sure the accounting
+		 * is done correctly
+		 */
+		existing->num_bytes = update->num_bytes;
+
+	}
+
+	if (update->extent_op) {
+		if (!existing->extent_op) {
+			existing->extent_op = update->extent_op;
+		} else {
+			if (update->extent_op->update_key) {
+				memcpy(&existing->extent_op->key,
+				       &update->extent_op->key,
+				       sizeof(update->extent_op->key));
+				existing->extent_op->update_key = true;
+			}
+			if (update->extent_op->update_flags) {
+				existing->extent_op->flags_to_set |=
+					update->extent_op->flags_to_set;
+				existing->extent_op->update_flags = true;
+			}
+			btrfs_free_delayed_extent_op(update->extent_op);
+		}
+	}
+	/*
+	 * update the reference mod on the head to reflect this new operation,
+	 * only need the lock for this case cause we could be processing it
+	 * currently, for refs we just added we know we're a-ok.
+	 */
+	old_ref_mod = existing->total_ref_mod;
+	if (old_ref_mod_ret)
+		*old_ref_mod_ret = old_ref_mod;
+	existing->ref_mod += update->ref_mod;
+	existing->total_ref_mod += update->ref_mod;
+
+}
+
+static void init_delayed_ref_head(struct btrfs_delayed_ref_head *head_ref,
+				  void *qrecord,
+				  u64 bytenr, u64 num_bytes, u64 ref_root,
+				  u64 reserved, int action, bool is_data,
+				  bool is_system)
+{
+	int count_mod = 1;
+	int must_insert_reserved = 0;
+
+	/* If reserved is provided, it must be a data extent. */
+	BUG_ON(!is_data && reserved);
+
+	/*
+	 * The head node stores the sum of all the mods, so dropping a ref
+	 * should drop the sum in the head node by one.
+	 */
+	if (action == BTRFS_UPDATE_DELAYED_HEAD)
+		count_mod = 0;
+	else if (action == BTRFS_DROP_DELAYED_REF)
+		count_mod = -1;
+
+	/*
+	 * BTRFS_ADD_DELAYED_EXTENT means that we need to update the reserved
+	 * accounting when the extent is finally added, or if a later
+	 * modification deletes the delayed ref without ever inserting the
+	 * extent into the extent allocation tree.  ref->must_insert_reserved
+	 * is the flag used to record that accounting mods are required.
+	 *
+	 * Once we record must_insert_reserved, switch the action to
+	 * BTRFS_ADD_DELAYED_REF because other special casing is not required.
+	 */
+	if (action == BTRFS_ADD_DELAYED_EXTENT)
+		must_insert_reserved = 1;
+	else
+		must_insert_reserved = 0;
+
+	head_ref->refs = 1;
+	head_ref->bytenr = bytenr;
+	head_ref->num_bytes = num_bytes;
+	head_ref->ref_mod = count_mod;
+	head_ref->must_insert_reserved = must_insert_reserved;
+	head_ref->is_data = is_data;
+	head_ref->is_system = is_system;
+	head_ref->ref_tree = RB_ROOT;
+	INIT_LIST_HEAD(&head_ref->ref_add_list);
+	RB_CLEAR_NODE(&head_ref->href_node);
+	head_ref->processing = 0;
+	head_ref->total_ref_mod = count_mod;
+}
+
+/*
+ * helper function to actually insert a head node into the rbtree.
+ * this does all the dirty work in terms of maintaining the correct
+ * overall modification count.
+ */
+static noinline struct btrfs_delayed_ref_head *
+add_delayed_ref_head(struct btrfs_trans_handle *trans,
+		     struct btrfs_delayed_ref_head *head_ref,
+		     void *qrecord,
+		     int action, int *qrecord_inserted_ret,
+		     int *old_ref_mod, int *new_ref_mod)
+{
+	struct btrfs_delayed_ref_head *existing;
+	struct btrfs_delayed_ref_root *delayed_refs;
+
+	delayed_refs = &trans->delayed_refs;
+
+	existing = htree_insert(&delayed_refs->href_root, &head_ref->href_node);
+	if (existing) {
+		update_existing_head_ref(delayed_refs, existing, head_ref, old_ref_mod);
+		/*
+		 * we've updated the existing ref, free the newly
+		 * allocated ref
+		 */
+		kfree(head_ref);
+		head_ref = existing;
+	} else {
+		if (old_ref_mod)
+			*old_ref_mod = 0;
+		delayed_refs->num_heads++;
+		delayed_refs->num_heads_ready++;
+		trans->delayed_ref_updates++;
+	}
+	if (new_ref_mod)
+		*new_ref_mod = head_ref->total_ref_mod;
+
+	return head_ref;
+}
+
+/*
+ * init_delayed_ref_common - Initialize the structure which represents a
+ *			     modification to a an extent.
+ *
+ * @fs_info:    Internal to the mounted filesystem mount structure.
+ *
+ * @ref:	The structure which is going to be initialized.
+ *
+ * @bytenr:	The logical address of the extent for which a modification is
+ *		going to be recorded.
+ *
+ * @num_bytes:  Size of the extent whose modification is being recorded.
+ *
+ * @ref_root:	The id of the root where this modification has originated, this
+ *		can be either one of the well-known metadata trees or the
+ *		subvolume id which references this extent.
+ *
+ * @action:	Can be one of BTRFS_ADD_DELAYED_REF/BTRFS_DROP_DELAYED_REF or
+ *		BTRFS_ADD_DELAYED_EXTENT
+ *
+ * @ref_type:	Holds the type of the extent which is being recorded, can be
+ *		one of BTRFS_SHARED_BLOCK_REF_KEY/BTRFS_TREE_BLOCK_REF_KEY
+ *		when recording a metadata extent or BTRFS_SHARED_DATA_REF_KEY/
+ *		BTRFS_EXTENT_DATA_REF_KEY when recording data extent
+ */
+static void init_delayed_ref_common(struct btrfs_fs_info *fs_info,
+				    struct btrfs_delayed_ref_node *ref,
+				    u64 bytenr, u64 num_bytes, u64 ref_root,
+				    int action, u8 ref_type)
+{
+	if (action == BTRFS_ADD_DELAYED_EXTENT)
+		action = BTRFS_ADD_DELAYED_REF;
+
+	ref->refs = 1;
+	ref->bytenr = bytenr;
+	ref->num_bytes = num_bytes;
+	ref->ref_mod = 1;
+	ref->action = action;
+	ref->is_head = 0;
+	ref->in_tree = 1;
+	ref->seq = 0;
+	ref->type = ref_type;
+	RB_CLEAR_NODE(&ref->ref_node);
+	INIT_LIST_HEAD(&ref->add_list);
+}
+
+/*
+ * add a delayed tree ref.  This does all of the accounting required
+ * to make sure the delayed ref is eventually processed before this
+ * transaction commits.
+ */
+int btrfs_add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
+			       struct btrfs_trans_handle *trans,
+			       u64 bytenr, u64 num_bytes, u64 parent,
+			       u64 ref_root, int level, int action,
+			       struct btrfs_delayed_extent_op *extent_op,
+			       int *old_ref_mod, int *new_ref_mod)
+{
+	struct btrfs_delayed_tree_ref *ref;
+	struct btrfs_delayed_ref_head *head_ref;
+	struct btrfs_delayed_ref_root *delayed_refs;
+	bool is_system = (ref_root == BTRFS_CHUNK_TREE_OBJECTID);
+	int ret;
+	u8 ref_type;
+
+	BUG_ON(extent_op && extent_op->is_data);
+	ref = kmalloc(sizeof(*ref), GFP_NOFS);
+	if (!ref)
+		return -ENOMEM;
+
+	if (parent)
+		ref_type = BTRFS_SHARED_BLOCK_REF_KEY;
+	else
+		ref_type = BTRFS_TREE_BLOCK_REF_KEY;
+	init_delayed_ref_common(fs_info, &ref->node, bytenr, num_bytes,
+				ref_root, action, ref_type);
+	ref->root = ref_root;
+	ref->parent = parent;
+	ref->level = level;
+
+	head_ref = kmalloc(sizeof(*head_ref), GFP_NOFS);
+	if (!head_ref)
+		goto free_ref;
+
+	init_delayed_ref_head(head_ref, NULL, bytenr, num_bytes,
+			      ref_root, 0, action, false, is_system);
+	head_ref->extent_op = extent_op;
+
+	delayed_refs = &trans->delayed_refs;
+
+	head_ref = add_delayed_ref_head(trans, head_ref, NULL, action, NULL,
+			old_ref_mod, new_ref_mod);
+
+	ret = insert_delayed_ref(trans, delayed_refs, head_ref, &ref->node);
+
+	if (ret > 0)
+		kfree(ref);
+
+	return 0;
+
+free_ref:
+	kfree(ref);
+
+	return -ENOMEM;
+}
diff --git a/delayed-ref.h b/delayed-ref.h
new file mode 100644
index 000000000000..30a68b2a278c
--- /dev/null
+++ b/delayed-ref.h
@@ -0,0 +1,208 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2008 Oracle.  All rights reserved.
+ */
+
+#ifndef BTRFS_DELAYED_REF_H
+#define BTRFS_DELAYED_REF_H
+
+#include "kerncompat.h"
+
+/* these are the possible values of struct btrfs_delayed_ref_node->action */
+#define BTRFS_ADD_DELAYED_REF    1 /* add one backref to the tree */
+#define BTRFS_DROP_DELAYED_REF   2 /* delete one backref from the tree */
+#define BTRFS_ADD_DELAYED_EXTENT 3 /* record a full extent allocation */
+#define BTRFS_UPDATE_DELAYED_HEAD 4 /* not changing ref count on head ref */
+
+struct btrfs_delayed_ref_node {
+	struct rb_node ref_node;
+	/*
+	 * If action is BTRFS_ADD_DELAYED_REF, also link this node to
+	 * ref_head->ref_add_list, then we do not need to iterate the
+	 * whole ref_head->ref_list to find BTRFS_ADD_DELAYED_REF nodes.
+	 */
+	struct list_head add_list;
+
+	/* the starting bytenr of the extent */
+	u64 bytenr;
+
+	/* the size of the extent */
+	u64 num_bytes;
+
+	/* seq number to keep track of insertion order */
+	u64 seq;
+
+	/* ref count on this data structure */
+	u64 refs;
+
+	/*
+	 * how many refs is this entry adding or deleting.  For
+	 * head refs, this may be a negative number because it is keeping
+	 * track of the total mods done to the reference count.
+	 * For individual refs, this will always be a positive number
+	 *
+	 * It may be more than one, since it is possible for a single
+	 * parent to have more than one ref on an extent
+	 */
+	int ref_mod;
+
+	unsigned int action:8;
+	unsigned int type:8;
+	/* is this node still in the rbtree? */
+	unsigned int is_head:1;
+	unsigned int in_tree:1;
+};
+
+struct btrfs_delayed_extent_op {
+	struct btrfs_disk_key key;
+	u8 level;
+	bool update_key;
+	bool update_flags;
+	bool is_data;
+	u64 flags_to_set;
+};
+
+/*
+ * the head refs are used to hold a lock on a given extent, which allows us
+ * to make sure that only one process is running the delayed refs
+ * at a time for a single extent.  They also store the sum of all the
+ * reference count modifications we've queued up.
+ */
+struct btrfs_delayed_ref_head {
+	u64 bytenr;
+	u64 num_bytes;
+	u64 refs;
+
+	struct rb_root ref_tree;
+	/* accumulate add BTRFS_ADD_DELAYED_REF nodes to this ref_add_list. */
+	struct list_head ref_add_list;
+
+	struct rb_node href_node;
+
+	struct btrfs_delayed_extent_op *extent_op;
+
+	/*
+	 * This is used to track the final ref_mod from all the refs associated
+	 * with this head ref, this is not adjusted as delayed refs are run,
+	 * this is meant to track if we need to do the csum accounting or not.
+	 */
+	int total_ref_mod;
+
+	/*
+	 * This is the current outstanding mod references for this bytenr.  This
+	 * is used with lookup_extent_info to get an accurate reference count
+	 * for a bytenr, so it is adjusted as delayed refs are run so that any
+	 * on disk reference count + ref_mod is accurate.
+	 */
+	int ref_mod;
+
+	/*
+	 * when a new extent is allocated, it is just reserved in memory
+	 * The actual extent isn't inserted into the extent allocation tree
+	 * until the delayed ref is processed.  must_insert_reserved is
+	 * used to flag a delayed ref so the accounting can be updated
+	 * when a full insert is done.
+	 *
+	 * It is possible the extent will be freed before it is ever
+	 * inserted into the extent allocation tree.  In this case
+	 * we need to update the in ram accounting to properly reflect
+	 * the free has happened.
+	 */
+	unsigned int must_insert_reserved:1;
+	unsigned int is_data:1;
+	unsigned int is_system:1;
+	unsigned int processing:1;
+};
+
+struct btrfs_delayed_tree_ref {
+	struct btrfs_delayed_ref_node node;
+	u64 root;
+	u64 parent;
+	int level;
+};
+
+struct btrfs_delayed_ref_root {
+	/* head ref rbtree */
+	struct rb_root href_root;
+
+	/* dirty extent records */
+	struct rb_root dirty_extent_root;
+
+	/* total number of head nodes in tree */
+	unsigned long num_heads;
+
+	/* total number of head nodes ready for processing */
+	unsigned long num_heads_ready;
+
+	/*
+	 * set when the tree is flushing before a transaction commit,
+	 * used by the throttling code to decide if new updates need
+	 * to be run right away
+	 */
+	int flushing;
+
+	u64 run_delayed_start;
+};
+
+
+static inline struct btrfs_delayed_extent_op *
+btrfs_alloc_delayed_extent_op(void)
+{
+	return kmalloc(sizeof(struct btrfs_delayed_extent_op), GFP_KERNEL);
+}
+
+static inline void
+btrfs_free_delayed_extent_op(struct btrfs_delayed_extent_op *op)
+{
+	if (op)
+		kfree(op);
+}
+
+static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
+{
+	WARN_ON(ref->refs == 0);
+	if (--ref->refs) {
+		WARN_ON(ref->in_tree);
+		switch (ref->type) {
+		case BTRFS_TREE_BLOCK_REF_KEY:
+		case BTRFS_SHARED_BLOCK_REF_KEY:
+			kfree(ref);
+			break;
+		case BTRFS_EXTENT_DATA_REF_KEY:
+		case BTRFS_SHARED_DATA_REF_KEY:
+			kfree(ref);
+			break;
+		default:
+			BUG();
+		}
+	}
+}
+
+static inline void btrfs_put_delayed_ref_head(struct btrfs_delayed_ref_head *head)
+{
+	if (--head->refs)
+		kfree(head);
+}
+
+int btrfs_add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
+			       struct btrfs_trans_handle *trans,
+			       u64 bytenr, u64 num_bytes, u64 parent,
+			       u64 ref_root, int level, int action,
+			       struct btrfs_delayed_extent_op *extent_op,
+			       int *old_ref_mod, int *new_ref_mod);
+void btrfs_merge_delayed_refs(struct btrfs_trans_handle *trans,
+			      struct btrfs_delayed_ref_root *delayed_refs,
+			      struct btrfs_delayed_ref_head *head);
+
+struct btrfs_delayed_ref_head *
+btrfs_select_ref_head(struct btrfs_trans_handle *trans);
+
+/*
+ * helper functions to cast a node into its container
+ */
+static inline struct btrfs_delayed_tree_ref *
+btrfs_delayed_node_to_tree_ref(struct btrfs_delayed_ref_node *node)
+{
+	return container_of(node, struct btrfs_delayed_tree_ref, node);
+}
+#endif
diff --git a/extent-tree.c b/extent-tree.c
index f7b59f84bf3d..3356dd2e4cf6 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -4218,3 +4218,229 @@ u64 add_new_free_space(struct btrfs_block_group_cache *block_group,
 
 	return total_added;
 }
+
+static void cleanup_extent_op(struct btrfs_trans_handle *trans,
+			     struct btrfs_fs_info *fs_info,
+			     struct btrfs_delayed_ref_head *head)
+{
+	struct btrfs_delayed_extent_op *extent_op = head->extent_op;
+
+	if (!extent_op)
+		return;
+	head->extent_op = NULL;
+	btrfs_free_delayed_extent_op(extent_op);
+}
+
+static void unselect_delayed_ref_head(struct btrfs_delayed_ref_root *delayed_refs,
+				      struct btrfs_delayed_ref_head *head)
+{
+	head->processing = 0;
+	delayed_refs->num_heads_ready++;
+}
+
+static int cleanup_ref_head(struct btrfs_trans_handle *trans,
+			    struct btrfs_fs_info *fs_info,
+			    struct btrfs_delayed_ref_head *head)
+{
+	struct btrfs_delayed_ref_root *delayed_refs;
+
+	delayed_refs = &trans->delayed_refs;
+
+	cleanup_extent_op(trans, fs_info, head);
+
+	/*
+	 * Need to drop our head ref lock and re-acquire the delayed ref lock
+	 * and then re-check to make sure nobody got added.
+	 */
+	if (!RB_EMPTY_ROOT(&head->ref_tree) || head->extent_op)
+		return 1;
+
+	delayed_refs->num_heads--;
+	rb_erase(&head->href_node, &delayed_refs->href_root);
+	RB_CLEAR_NODE(&head->href_node);
+
+	if (head->must_insert_reserved)
+		btrfs_pin_extent(fs_info, head->bytenr, head->num_bytes);
+
+	btrfs_put_delayed_ref_head(head);
+	return 0;
+}
+
+static inline struct btrfs_delayed_ref_node *
+select_delayed_ref(struct btrfs_delayed_ref_head *head)
+{
+	struct btrfs_delayed_ref_node *ref;
+
+	if (RB_EMPTY_ROOT(&head->ref_tree))
+		return NULL;
+	/*
+	 * Select a delayed ref of type BTRFS_ADD_DELAYED_REF first.
+	 * This is to prevent a ref count from going down to zero, which deletes
+	 * the extent item from the extent tree, when there still are references
+	 * to add, which would fail because they would not find the extent item.
+	 */
+	if (!list_empty(&head->ref_add_list))
+		return list_first_entry(&head->ref_add_list,
+					struct btrfs_delayed_ref_node,
+					add_list);
+	ref = rb_entry(rb_first(&head->ref_tree),
+		       struct btrfs_delayed_ref_node, ref_node);
+	ASSERT(list_empty(&ref->add_list));
+	return ref;
+}
+
+
+static int run_delayed_tree_ref(struct btrfs_trans_handle *trans,
+				struct btrfs_fs_info *fs_info,
+				struct btrfs_delayed_ref_node *node,
+				struct btrfs_delayed_extent_op *extent_op,
+				int insert_reserved)
+{
+	int ret = 0;
+	struct btrfs_delayed_tree_ref *ref;
+	u64 parent = 0;
+	u64 ref_root = 0;
+
+	ref = btrfs_delayed_node_to_tree_ref(node);
+
+	if (node->type == BTRFS_SHARED_BLOCK_REF_KEY)
+			parent = ref->parent;
+	ref_root = ref->root;
+
+	if (node->ref_mod != 1) {
+		printf("btree block(%llu) has %d references rather than 1: action %d ref_root %llu parent %llu",
+			node->bytenr, node->ref_mod, node->action, ref_root,
+			parent);
+		return -EIO;
+	}
+	if (node->action == BTRFS_ADD_DELAYED_REF && insert_reserved) {
+		BUG_ON(!extent_op || !extent_op->update_flags);
+		ret = alloc_reserved_tree_block2(trans, node, extent_op);
+	} else if (node->action == BTRFS_DROP_DELAYED_REF) {
+		ret = __free_extent2(trans, node, extent_op);
+	} else {
+		BUG();
+	}
+
+	return ret;
+}
+
+/* helper function to actually process a single delayed ref entry */
+static int run_one_delayed_ref(struct btrfs_trans_handle *trans,
+			       struct btrfs_fs_info *fs_info,
+			       struct btrfs_delayed_ref_node *node,
+			       struct btrfs_delayed_extent_op *extent_op,
+			       int insert_reserved)
+{
+	int ret = 0;
+
+	if (node->type == BTRFS_TREE_BLOCK_REF_KEY ||
+		node->type == BTRFS_SHARED_BLOCK_REF_KEY) {
+		ret = run_delayed_tree_ref(trans, fs_info, node, extent_op,
+					   insert_reserved);
+	} else
+		BUG();
+	return ret;
+}
+
+int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans, unsigned long nr)
+{
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_delayed_ref_root *delayed_refs;
+	struct btrfs_delayed_ref_node *ref;
+	struct btrfs_delayed_ref_head *locked_ref = NULL;
+	struct btrfs_delayed_extent_op *extent_op;
+	int ret;
+	int must_insert_reserved = 0;
+
+	delayed_refs = &trans->delayed_refs;
+	while (1) {
+		if (!locked_ref) {
+			locked_ref = btrfs_select_ref_head(trans);
+			if (!locked_ref)
+				break;
+		}
+		/*
+		 * We need to try and merge add/drops of the same ref since we
+		 * can run into issues with relocate dropping the implicit ref
+		 * and then it being added back again before the drop can
+		 * finish.	If we merged anything we need to re-loop so we can
+		 * get a good ref.
+		 * Or we can get node references of the same type that weren't
+		 * merged when created due to bumps in the tree mod seq, and
+		 * we need to merge them to prevent adding an inline extent
+		 * backref before dropping it (triggering a BUG_ON at
+		 * insert_inline_extent_backref()).
+		 */
+		btrfs_merge_delayed_refs(trans, delayed_refs, locked_ref);
+		ref = select_delayed_ref(locked_ref);
+		/*
+		 * We're done processing refs in this ref_head, clean everything
+		 * up and move on to the next ref_head.
+		 */
+		if (!ref) {
+			ret = cleanup_ref_head(trans, fs_info, locked_ref);
+			if (ret > 0 ) {
+				/* We dropped our lock, we need to loop. */
+				ret = 0;
+				continue;
+			} else if (ret) {
+				return ret;
+			}
+			locked_ref = NULL;
+			continue;
+		}
+
+		ref->in_tree = 0;
+		rb_erase(&ref->ref_node, &locked_ref->ref_tree);
+		RB_CLEAR_NODE(&ref->ref_node);
+		if (!list_empty(&ref->add_list))
+				list_del(&ref->add_list);
+		/*
+		 * When we play the delayed ref, also correct the ref_mod on
+		 * head
+		 */
+		switch (ref->action) {
+		case BTRFS_ADD_DELAYED_REF:
+		case BTRFS_ADD_DELAYED_EXTENT:
+			locked_ref->ref_mod -= ref->ref_mod;
+			break;
+		case BTRFS_DROP_DELAYED_REF:
+			locked_ref->ref_mod += ref->ref_mod;
+			break;
+		default:
+			WARN_ON(1);
+		}
+
+		/*
+		 * Record the must-insert_reserved flag before we drop the spin
+		 * lock.
+		 */
+		must_insert_reserved = locked_ref->must_insert_reserved;
+		locked_ref->must_insert_reserved = 0;
+
+		extent_op = locked_ref->extent_op;
+		locked_ref->extent_op = NULL;
+
+		ret = run_one_delayed_ref(trans, fs_info, ref, extent_op,
+					  must_insert_reserved);
+
+		btrfs_free_delayed_extent_op(extent_op);
+		/*
+		 * If we are re-initing extent tree in this transaction
+		 * failure in freeing old roots are expected (because we don't
+		 * have the old extent tree, hence backref resolution will
+		 * return -EIO).
+		 */
+		if (ret && (!trans->reinit_extent_tree ||
+		     ref->action != BTRFS_DROP_DELAYED_REF)) {
+			unselect_delayed_ref_head(delayed_refs, locked_ref);
+			btrfs_put_delayed_ref(ref);
+			return ret;
+		}
+
+		btrfs_put_delayed_ref(ref);
+	}
+
+	return 0;
+}
diff --git a/kerncompat.h b/kerncompat.h
index fa96715fb70c..1a2bc18c3ac2 100644
--- a/kerncompat.h
+++ b/kerncompat.h
@@ -263,6 +263,14 @@ static inline int IS_ERR_OR_NULL(const void *ptr)
 	return !ptr || IS_ERR(ptr);
 }
 
+/**
+ * swap - swap values of @a and @b
+ * @a: first value
+ * @b: second value
+ */
+#define swap(a, b) \
+        do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
+
 /*
  * This looks more complex than it should be. But we need to
  * get the type for the ~ right in round_down (it needs to be
diff --git a/transaction.h b/transaction.h
index 750e329e1ba8..34060252dd5c 100644
--- a/transaction.h
+++ b/transaction.h
@@ -21,6 +21,7 @@
 
 #include "kerncompat.h"
 #include "ctree.h"
+#include "delayed-ref.h"
 
 struct btrfs_trans_handle {
 	struct btrfs_fs_info *fs_info;
@@ -28,9 +29,12 @@ struct btrfs_trans_handle {
 	u64 alloc_exclude_start;
 	u64 alloc_exclude_nr;
 	bool reinit_extent_tree;
+	u64 delayed_ref_updates;
 	unsigned long blocks_reserved;
 	unsigned long blocks_used;
 	struct btrfs_block_group_cache *block_group;
+	struct btrfs_delayed_ref_root delayed_refs;
+
 };
 
 struct btrfs_trans_handle* btrfs_start_transaction(struct btrfs_root *root,
-- 
2.7.4

[PATCH 4/8] btrfs-progs: Make btrfs_write_dirty_block_groups take only trans argument

[Nikolay Borisov]

The root argument is used only to get a reference to the fs_info, this
can be achieved with the transaction handle being passed so use that.
This is in preparation for moving this function in the main transaction
commit routine. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 ctree.h       | 3 +--
 extent-tree.c | 5 ++---
 transaction.c | 4 ++--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/ctree.h b/ctree.h
index 5242595fe355..75675ef3f781 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2523,8 +2523,7 @@ int btrfs_update_extent_ref(struct btrfs_trans_handle *trans,
 			    u64 orig_parent, u64 parent,
 			    u64 root_objectid, u64 ref_generation,
 			    u64 owner_objectid);
-int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans,
-				    struct btrfs_root *root);
+int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans);
 int btrfs_free_block_groups(struct btrfs_fs_info *info);
 int btrfs_read_block_groups(struct btrfs_root *root);
 struct btrfs_block_group_cache *
diff --git a/extent-tree.c b/extent-tree.c
index 3356dd2e4cf6..7d6c37c6b371 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1727,8 +1727,7 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
 
 }
 
-int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans,
-				   struct btrfs_root *root)
+int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans)
 {
 	struct extent_io_tree *block_group_cache;
 	struct btrfs_block_group_cache *cache;
@@ -1739,7 +1738,7 @@ int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans,
 	u64 end;
 	u64 ptr;
 
-	block_group_cache = &root->fs_info->block_group_cache;
+	block_group_cache = &trans->fs_info->block_group_cache;
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
diff --git a/transaction.c b/transaction.c
index ecafbb156610..96d9891b0d1c 100644
--- a/transaction.c
+++ b/transaction.c
@@ -61,7 +61,7 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
 	u64 old_root_bytenr;
 	struct btrfs_root *tree_root = root->fs_info->tree_root;
 
-	btrfs_write_dirty_block_groups(trans, root);
+	btrfs_write_dirty_block_groups(trans);
 	while(1) {
 		old_root_bytenr = btrfs_root_bytenr(&root->root_item);
 		if (old_root_bytenr == root->node->start)
@@ -75,7 +75,7 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
 					&root->root_key,
 					&root->root_item);
 		BUG_ON(ret);
-		btrfs_write_dirty_block_groups(trans, root);
+		btrfs_write_dirty_block_groups(trans);
 	}
 	return 0;
 }
-- 
2.7.4

[PATCH 5/8] btrfs-progs: Wire up delayed refs

[Nikolay Borisov]

This commit enables the delayed refs infrastructures. This entails doing
the following:

1. Replacing existing calls of btrfs_extent_post_op (which is the
equivalent of delayed refs) with the proper btrfs_run_delayed_refs.
As well as eliminating open-coded calls to finish_current_insert and
del_pending_extents which execute the delayed ops.

2. Wiring up the addition of delayed refs when freeing extents
(btrfs_free_extent) and when adding new extents (alloc_tree_block).

3. Adding calls to btrfs_run_delayed refs in the transaction commit
path alongside comments why every call is needed, since it's not always
obvious (those call sites were derived empirically by running and
debugging existing tests)

4. Correctly flagging the transaction in which we are reinitialising
the extent tree.

5 Moving btrfs_write_dirty_block_groups to btrfs_write_dirty_block_groups
since blockgroups should be written to disk after the last delayed refs
have been run.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 check/main.c  |   3 +-
 extent-tree.c | 166 ++++++++++++++++++++++++++++++----------------------------
 transaction.c |  27 +++++++++-
 3 files changed, 112 insertions(+), 84 deletions(-)

diff --git a/check/main.c b/check/main.c
index bc2ee22f7943..b361cd7e26a0 100644
--- a/check/main.c
+++ b/check/main.c
@@ -8710,7 +8710,7 @@ static int reinit_extent_tree(struct btrfs_trans_handle *trans,
 			fprintf(stderr, "Error adding block group\n");
 			return ret;
 		}
-		btrfs_extent_post_op(trans);
+		btrfs_run_delayed_refs(trans, -1);
 	}
 
 	ret = reset_balance(trans, fs_info);
@@ -9767,6 +9767,7 @@ int cmd_check(int argc, char **argv)
 			goto close_out;
 		}
 
+		trans->reinit_extent_tree = true;
 		if (init_extent_tree) {
 			printf("Creating a new extent tree\n");
 			ret = reinit_extent_tree(trans, info,
diff --git a/extent-tree.c b/extent-tree.c
index 7d6c37c6b371..2fa51bbc0359 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1418,8 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
 		err = ret;
 out:
 	btrfs_free_path(path);
-	finish_current_insert(trans);
-	del_pending_extents(trans);
 	BUG_ON(err);
 	return err;
 }
@@ -1602,8 +1600,6 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, u64 bytenr,
 	btrfs_set_extent_flags(l, item, flags);
 out:
 	btrfs_free_path(path);
-	finish_current_insert(trans);
-	del_pending_extents(trans);
 	return ret;
 }
 
@@ -1701,7 +1697,6 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
 				 struct btrfs_block_group_cache *cache)
 {
 	int ret;
-	int pending_ret;
 	struct btrfs_root *extent_root = trans->fs_info->extent_root;
 	unsigned long bi;
 	struct extent_buffer *leaf;
@@ -1717,12 +1712,8 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
 	btrfs_mark_buffer_dirty(leaf);
 	btrfs_release_path(path);
 fail:
-	finish_current_insert(trans);
-	pending_ret = del_pending_extents(trans);
 	if (ret)
 		return ret;
-	if (pending_ret)
-		return pending_ret;
 	return 0;
 
 }
@@ -2049,6 +2040,7 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
 	int skinny_metadata =
 		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
 
+
 	while(1) {
 		ret = find_first_extent_bit(&info->extent_ins, 0, &start,
 					    &end, EXTENT_LOCKED);
@@ -2080,6 +2072,8 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
 			BUG_ON(1);
 		}
 
+
+		printf("shouldn't be executed\n");
 		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
 		kfree(extent_op);
 	}
@@ -2379,7 +2373,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 	}
 fail:
 	btrfs_free_path(path);
-	finish_current_insert(trans);
 	return ret;
 }
 
@@ -2462,33 +2455,30 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
 		      u64 bytenr, u64 num_bytes, u64 parent,
 		      u64 root_objectid, u64 owner, u64 offset)
 {
-	struct btrfs_root *extent_root = root->fs_info->extent_root;
-	int pending_ret;
 	int ret;
 
 	WARN_ON(num_bytes < root->fs_info->sectorsize);
-	if (root == extent_root) {
-		struct pending_extent_op *extent_op;
-
-		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
-		BUG_ON(!extent_op);
-
-		extent_op->type = PENDING_EXTENT_DELETE;
-		extent_op->bytenr = bytenr;
-		extent_op->num_bytes = num_bytes;
-		extent_op->level = (int)owner;
-
-		set_extent_bits(&root->fs_info->pending_del,
-				bytenr, bytenr + num_bytes - 1,
-				EXTENT_LOCKED);
-		set_state_private(&root->fs_info->pending_del,
-				  bytenr, (unsigned long)extent_op);
-		return 0;
+	/*
+	 * tree log blocks never actually go into the extent allocation
+	 * tree, just update pinning info and exit early.
+	 */
+	if (root_objectid == BTRFS_TREE_LOG_OBJECTID) {
+		printf("PINNING EXTENTS IN LOG TREE\n");
+		WARN_ON(owner >= BTRFS_FIRST_FREE_OBJECTID);
+		btrfs_pin_extent(trans->fs_info, bytenr, num_bytes);
+		ret = 0;
+	} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
+		BUG_ON(offset);
+		ret = btrfs_add_delayed_tree_ref(trans->fs_info, trans,
+						 bytenr, num_bytes, parent,
+						 root_objectid, (int)owner,
+						 BTRFS_DROP_DELAYED_REF,
+						 NULL, NULL, NULL);
+	} else {
+		ret = __free_extent(trans, bytenr, num_bytes, parent,
+				    root_objectid, owner, offset, 1);
 	}
-	ret = __free_extent(trans, bytenr, num_bytes, parent,
-			    root_objectid, owner, offset, 1);
-	pending_ret = del_pending_extents(trans);
-	return ret ? ret : pending_ret;
+	return ret;
 }
 
 static u64 stripe_align(struct btrfs_root *root, u64 val)
@@ -2694,6 +2684,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
 	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
 	struct btrfs_key ins;
 	bool skinny_metadata = btrfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
+	int ret;
+	u64 start, end;
 
 	ins.objectid = node->bytenr;
 	if (skinny_metadata) {
@@ -2704,10 +2696,25 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
 		ins.type = BTRFS_EXTENT_ITEM_KEY;
 	}
 
-	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
-					 extent_op->flags_to_set,
-					 &extent_op->key, ref->level, &ins);
+	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
+		ret = find_first_extent_bit(&trans->fs_info->extent_ins,
+					    node->bytenr, &start, &end,
+					    EXTENT_LOCKED);
+		ASSERT(!ret);
+		ASSERT(start == node->bytenr);
+		ASSERT(end == node->bytenr + node->num_bytes - 1);
+	}
+
+	ret = alloc_reserved_tree_block(trans, ref->root, trans->transid,
+					extent_op->flags_to_set,
+					&extent_op->key, ref->level, &ins);
 
+	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
+		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
+				  EXTENT_LOCKED);
+	}
+
+	return ret;
 }
 
 static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
@@ -2772,39 +2779,50 @@ static int alloc_tree_block(struct btrfs_trans_handle *trans,
 			    u64 search_end, struct btrfs_key *ins)
 {
 	int ret;
+	u64 extent_size;
+	struct btrfs_delayed_extent_op *extent_op;
+	bool skinny_metadata = btrfs_fs_incompat(root->fs_info,
+						 SKINNY_METADATA);
+
+	extent_op = btrfs_alloc_delayed_extent_op();
+	if (!extent_op)
+		return -ENOMEM;
+
 	ret = btrfs_reserve_extent(trans, root, num_bytes, empty_size,
 				   hint_byte, search_end, ins, 0);
 	BUG_ON(ret);
 
+	if (key)
+		memcpy(&extent_op->key, key, sizeof(extent_op->key));
+	else
+		memset(&extent_op->key, 0, sizeof(extent_op->key));
+	extent_op->flags_to_set = flags;
+	extent_op->update_key = skinny_metadata ? false : true;
+	extent_op->update_flags = true;
+	extent_op->is_data = false;
+	extent_op->level = level;
+
+	extent_size = ins->offset;
+
+	if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
+		ins->offset = level;
+		ins->type = BTRFS_METADATA_ITEM_KEY;
+	}
+
+	/* Ensure this reserved extent is not found by the allocator */
 	if (root_objectid == BTRFS_EXTENT_TREE_OBJECTID) {
-		struct pending_extent_op *extent_op;
-
-		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
-		BUG_ON(!extent_op);
-
-		extent_op->type = PENDING_EXTENT_INSERT;
-		extent_op->bytenr = ins->objectid;
-		extent_op->num_bytes = ins->offset;
-		extent_op->level = level;
-		extent_op->flags = flags;
-		memcpy(&extent_op->key, key, sizeof(*key));
-
-		set_extent_bits(&root->fs_info->extent_ins, ins->objectid,
-				ins->objectid + ins->offset - 1,
-				EXTENT_LOCKED);
-		set_state_private(&root->fs_info->extent_ins,
-				  ins->objectid, (unsigned long)extent_op);
-	} else {
-		if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
-			ins->offset = level;
-			ins->type = BTRFS_METADATA_ITEM_KEY;
-		}
-		ret = alloc_reserved_tree_block(trans, root_objectid,
-						generation, flags,
-						key, level, ins);
-		finish_current_insert(trans);
-		del_pending_extents(trans);
+		ret = set_extent_bits(&trans->fs_info->extent_ins,
+				      ins->objectid,
+				      ins->objectid + extent_size - 1,
+				      EXTENT_LOCKED);
+
+		BUG_ON(ret);
 	}
+
+	ret = btrfs_add_delayed_tree_ref(root->fs_info, trans, ins->objectid,
+					 extent_size, 0, root_objectid,
+					 level, BTRFS_ADD_DELAYED_EXTENT,
+					 extent_op, NULL, NULL);
 	return ret;
 }
 
@@ -3329,11 +3347,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
 				sizeof(cache->item));
 	BUG_ON(ret);
 
-	ret = finish_current_insert(trans);
-	BUG_ON(ret);
-	ret = del_pending_extents(trans);
-	BUG_ON(ret);
-
 	return 0;
 }
 
@@ -3429,10 +3442,6 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
 					sizeof(cache->item));
 		BUG_ON(ret);
 
-		finish_current_insert(trans);
-		ret = del_pending_extents(trans);
-		BUG_ON(ret);
-
 		cur_start = cache->key.objectid + cache->key.offset;
 	}
 	return 0;
@@ -3814,14 +3823,9 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans)
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *root = fs_info->extent_root;
 
-	while(extent_root_pending_ops(fs_info)) {
-		ret = finish_current_insert(trans);
-		if (ret)
-			return ret;
-		ret = del_pending_extents(trans);
-		if (ret)
-			return ret;
-	}
+	ret = btrfs_run_delayed_refs(trans, -1);
+	if (ret)
+		return ret;
 
 	while(1) {
 		cache = btrfs_lookup_first_block_group(fs_info, start);
@@ -4026,7 +4030,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
 		} else if (ret != -EEXIST) {
 			goto fail;
 		}
-		btrfs_extent_post_op(trans);
+		btrfs_run_delayed_refs(trans, -1);
 		extent_bytenr = disk_bytenr;
 		extent_num_bytes = num_bytes;
 		extent_offset = 0;
diff --git a/transaction.c b/transaction.c
index 96d9891b0d1c..bfda769210ee 100644
--- a/transaction.c
+++ b/transaction.c
@@ -61,7 +61,6 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
 	u64 old_root_bytenr;
 	struct btrfs_root *tree_root = root->fs_info->tree_root;
 
-	btrfs_write_dirty_block_groups(trans);
 	while(1) {
 		old_root_bytenr = btrfs_root_bytenr(&root->root_item);
 		if (old_root_bytenr == root->node->start)
@@ -98,6 +97,17 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
 	if (ret)
 		return ret;
 
+	/*
+	 * If the above CoW is the first one to dirty the current tree_root,
+	 * delayed refs for it won't be run until after this function has
+	 * finished executing, meaning we won't process the extent tree root,
+	 * which will have been added to ->dirty_cowonly_roots.  So run
+	 * delayed refs here as well.
+	 */
+	ret = btrfs_run_delayed_refs(trans, -1);
+	if (ret)
+		return ret;
+
 	while(!list_empty(&fs_info->dirty_cowonly_roots)) {
 		next = fs_info->dirty_cowonly_roots.next;
 		list_del_init(next);
@@ -147,6 +157,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
 
 	if (trans->fs_info->transaction_aborted)
 		return -EROFS;
+	/*
+	 * Flush all accumulated delayed refs so that root-tree updates are
+	 * consistent
+	 */
+	ret = btrfs_run_delayed_refs(trans, -1);
+	BUG_ON(ret);
 
 	if (root->commit_root == root->node)
 		goto commit_tree;
@@ -164,11 +180,18 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
 	ret = btrfs_update_root(trans, root->fs_info->tree_root,
 				&root->root_key, &root->root_item);
 	BUG_ON(ret);
+
 commit_tree:
 	ret = commit_tree_roots(trans, fs_info);
 	BUG_ON(ret);
-	ret = __commit_transaction(trans, root);
+	/*
+	 * Ensure that all comitted roots are properly accounted in the
+	 * extent tree
+	 */
+	ret = btrfs_run_delayed_refs(trans, -1);
 	BUG_ON(ret);
+	btrfs_write_dirty_block_groups(trans);
+	__commit_transaction(trans, root);
 	write_ctree_super(trans);
 	btrfs_finish_extent_commit(trans, fs_info->extent_root,
 			           &fs_info->pinned_extents);
-- 
2.7.4

Re: [PATCH 5/8] btrfs-progs: Wire up delayed refs

[Qu Wenruo]

On 2018/8/16 下午9:10, Nikolay Borisov wrote:
> This commit enables the delayed refs infrastructures. This entails doing
> the following:
> 
> 1. Replacing existing calls of btrfs_extent_post_op (which is the
> equivalent of delayed refs) with the proper btrfs_run_delayed_refs.
> As well as eliminating open-coded calls to finish_current_insert and
> del_pending_extents which execute the delayed ops.
> 
> 2. Wiring up the addition of delayed refs when freeing extents
> (btrfs_free_extent) and when adding new extents (alloc_tree_block).
> 
> 3. Adding calls to btrfs_run_delayed refs in the transaction commit
> path alongside comments why every call is needed, since it's not always
> obvious (those call sites were derived empirically by running and
> debugging existing tests)
> 
> 4. Correctly flagging the transaction in which we are reinitialising
> the extent tree.
> 
> 5 Moving btrfs_write_dirty_block_groups to btrfs_write_dirty_block_groups
> since blockgroups should be written to disk after the last delayed refs
> have been run.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.com>

Is there something (maybe btrfs_run_delayed_refs()?) missing in btrfs-image?

btrfs-image from devel branch can't restore image correctly, the block
group used bytes is not correct, thus it can't pass misc nor fsck tests.

Thanks,
Qu

> ---
>  check/main.c  |   3 +-
>  extent-tree.c | 166 ++++++++++++++++++++++++++++++----------------------------
>  transaction.c |  27 +++++++++-
>  3 files changed, 112 insertions(+), 84 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index bc2ee22f7943..b361cd7e26a0 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -8710,7 +8710,7 @@ static int reinit_extent_tree(struct btrfs_trans_handle *trans,
>  			fprintf(stderr, "Error adding block group\n");
>  			return ret;
>  		}
> -		btrfs_extent_post_op(trans);
> +		btrfs_run_delayed_refs(trans, -1);
>  	}
>  
>  	ret = reset_balance(trans, fs_info);
> @@ -9767,6 +9767,7 @@ int cmd_check(int argc, char **argv)
>  			goto close_out;
>  		}
>  
> +		trans->reinit_extent_tree = true;
>  		if (init_extent_tree) {
>  			printf("Creating a new extent tree\n");
>  			ret = reinit_extent_tree(trans, info,
> diff --git a/extent-tree.c b/extent-tree.c
> index 7d6c37c6b371..2fa51bbc0359 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -1418,8 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>  		err = ret;
>  out:
>  	btrfs_free_path(path);
> -	finish_current_insert(trans);
> -	del_pending_extents(trans);
>  	BUG_ON(err);
>  	return err;
>  }
> @@ -1602,8 +1600,6 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, u64 bytenr,
>  	btrfs_set_extent_flags(l, item, flags);
>  out:
>  	btrfs_free_path(path);
> -	finish_current_insert(trans);
> -	del_pending_extents(trans);
>  	return ret;
>  }
>  
> @@ -1701,7 +1697,6 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>  				 struct btrfs_block_group_cache *cache)
>  {
>  	int ret;
> -	int pending_ret;
>  	struct btrfs_root *extent_root = trans->fs_info->extent_root;
>  	unsigned long bi;
>  	struct extent_buffer *leaf;
> @@ -1717,12 +1712,8 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>  	btrfs_mark_buffer_dirty(leaf);
>  	btrfs_release_path(path);
>  fail:
> -	finish_current_insert(trans);
> -	pending_ret = del_pending_extents(trans);
>  	if (ret)
>  		return ret;
> -	if (pending_ret)
> -		return pending_ret;
>  	return 0;
>  
>  }
> @@ -2049,6 +2040,7 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>  	int skinny_metadata =
>  		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
>  
> +
>  	while(1) {
>  		ret = find_first_extent_bit(&info->extent_ins, 0, &start,
>  					    &end, EXTENT_LOCKED);
> @@ -2080,6 +2072,8 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>  			BUG_ON(1);
>  		}
>  
> +
> +		printf("shouldn't be executed\n");
>  		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
>  		kfree(extent_op);
>  	}
> @@ -2379,7 +2373,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
>  	}
>  fail:
>  	btrfs_free_path(path);
> -	finish_current_insert(trans);
>  	return ret;
>  }
>  
> @@ -2462,33 +2455,30 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>  		      u64 bytenr, u64 num_bytes, u64 parent,
>  		      u64 root_objectid, u64 owner, u64 offset)
>  {
> -	struct btrfs_root *extent_root = root->fs_info->extent_root;
> -	int pending_ret;
>  	int ret;
>  
>  	WARN_ON(num_bytes < root->fs_info->sectorsize);
> -	if (root == extent_root) {
> -		struct pending_extent_op *extent_op;
> -
> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
> -		BUG_ON(!extent_op);
> -
> -		extent_op->type = PENDING_EXTENT_DELETE;
> -		extent_op->bytenr = bytenr;
> -		extent_op->num_bytes = num_bytes;
> -		extent_op->level = (int)owner;
> -
> -		set_extent_bits(&root->fs_info->pending_del,
> -				bytenr, bytenr + num_bytes - 1,
> -				EXTENT_LOCKED);
> -		set_state_private(&root->fs_info->pending_del,
> -				  bytenr, (unsigned long)extent_op);
> -		return 0;
> +	/*
> +	 * tree log blocks never actually go into the extent allocation
> +	 * tree, just update pinning info and exit early.
> +	 */
> +	if (root_objectid == BTRFS_TREE_LOG_OBJECTID) {
> +		printf("PINNING EXTENTS IN LOG TREE\n");
> +		WARN_ON(owner >= BTRFS_FIRST_FREE_OBJECTID);
> +		btrfs_pin_extent(trans->fs_info, bytenr, num_bytes);
> +		ret = 0;
> +	} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
> +		BUG_ON(offset);
> +		ret = btrfs_add_delayed_tree_ref(trans->fs_info, trans,
> +						 bytenr, num_bytes, parent,
> +						 root_objectid, (int)owner,
> +						 BTRFS_DROP_DELAYED_REF,
> +						 NULL, NULL, NULL);
> +	} else {
> +		ret = __free_extent(trans, bytenr, num_bytes, parent,
> +				    root_objectid, owner, offset, 1);
>  	}
> -	ret = __free_extent(trans, bytenr, num_bytes, parent,
> -			    root_objectid, owner, offset, 1);
> -	pending_ret = del_pending_extents(trans);
> -	return ret ? ret : pending_ret;
> +	return ret;
>  }
>  
>  static u64 stripe_align(struct btrfs_root *root, u64 val)
> @@ -2694,6 +2684,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>  	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
>  	struct btrfs_key ins;
>  	bool skinny_metadata = btrfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
> +	int ret;
> +	u64 start, end;
>  
>  	ins.objectid = node->bytenr;
>  	if (skinny_metadata) {
> @@ -2704,10 +2696,25 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>  		ins.type = BTRFS_EXTENT_ITEM_KEY;
>  	}
>  
> -	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
> -					 extent_op->flags_to_set,
> -					 &extent_op->key, ref->level, &ins);
> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
> +		ret = find_first_extent_bit(&trans->fs_info->extent_ins,
> +					    node->bytenr, &start, &end,
> +					    EXTENT_LOCKED);
> +		ASSERT(!ret);
> +		ASSERT(start == node->bytenr);
> +		ASSERT(end == node->bytenr + node->num_bytes - 1);
> +	}
> +
> +	ret = alloc_reserved_tree_block(trans, ref->root, trans->transid,
> +					extent_op->flags_to_set,
> +					&extent_op->key, ref->level, &ins);
>  
> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
> +		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
> +				  EXTENT_LOCKED);
> +	}
> +
> +	return ret;
>  }
>  
>  static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
> @@ -2772,39 +2779,50 @@ static int alloc_tree_block(struct btrfs_trans_handle *trans,
>  			    u64 search_end, struct btrfs_key *ins)
>  {
>  	int ret;
> +	u64 extent_size;
> +	struct btrfs_delayed_extent_op *extent_op;
> +	bool skinny_metadata = btrfs_fs_incompat(root->fs_info,
> +						 SKINNY_METADATA);
> +
> +	extent_op = btrfs_alloc_delayed_extent_op();
> +	if (!extent_op)
> +		return -ENOMEM;
> +
>  	ret = btrfs_reserve_extent(trans, root, num_bytes, empty_size,
>  				   hint_byte, search_end, ins, 0);
>  	BUG_ON(ret);
>  
> +	if (key)
> +		memcpy(&extent_op->key, key, sizeof(extent_op->key));
> +	else
> +		memset(&extent_op->key, 0, sizeof(extent_op->key));
> +	extent_op->flags_to_set = flags;
> +	extent_op->update_key = skinny_metadata ? false : true;
> +	extent_op->update_flags = true;
> +	extent_op->is_data = false;
> +	extent_op->level = level;
> +
> +	extent_size = ins->offset;
> +
> +	if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
> +		ins->offset = level;
> +		ins->type = BTRFS_METADATA_ITEM_KEY;
> +	}
> +
> +	/* Ensure this reserved extent is not found by the allocator */
>  	if (root_objectid == BTRFS_EXTENT_TREE_OBJECTID) {
> -		struct pending_extent_op *extent_op;
> -
> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
> -		BUG_ON(!extent_op);
> -
> -		extent_op->type = PENDING_EXTENT_INSERT;
> -		extent_op->bytenr = ins->objectid;
> -		extent_op->num_bytes = ins->offset;
> -		extent_op->level = level;
> -		extent_op->flags = flags;
> -		memcpy(&extent_op->key, key, sizeof(*key));
> -
> -		set_extent_bits(&root->fs_info->extent_ins, ins->objectid,
> -				ins->objectid + ins->offset - 1,
> -				EXTENT_LOCKED);
> -		set_state_private(&root->fs_info->extent_ins,
> -				  ins->objectid, (unsigned long)extent_op);
> -	} else {
> -		if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
> -			ins->offset = level;
> -			ins->type = BTRFS_METADATA_ITEM_KEY;
> -		}
> -		ret = alloc_reserved_tree_block(trans, root_objectid,
> -						generation, flags,
> -						key, level, ins);
> -		finish_current_insert(trans);
> -		del_pending_extents(trans);
> +		ret = set_extent_bits(&trans->fs_info->extent_ins,
> +				      ins->objectid,
> +				      ins->objectid + extent_size - 1,
> +				      EXTENT_LOCKED);
> +
> +		BUG_ON(ret);
>  	}
> +
> +	ret = btrfs_add_delayed_tree_ref(root->fs_info, trans, ins->objectid,
> +					 extent_size, 0, root_objectid,
> +					 level, BTRFS_ADD_DELAYED_EXTENT,
> +					 extent_op, NULL, NULL);
>  	return ret;
>  }
>  
> @@ -3329,11 +3347,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
>  				sizeof(cache->item));
>  	BUG_ON(ret);
>  
> -	ret = finish_current_insert(trans);
> -	BUG_ON(ret);
> -	ret = del_pending_extents(trans);
> -	BUG_ON(ret);
> -
>  	return 0;
>  }
>  
> @@ -3429,10 +3442,6 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
>  					sizeof(cache->item));
>  		BUG_ON(ret);
>  
> -		finish_current_insert(trans);
> -		ret = del_pending_extents(trans);
> -		BUG_ON(ret);
> -
>  		cur_start = cache->key.objectid + cache->key.offset;
>  	}
>  	return 0;
> @@ -3814,14 +3823,9 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans)
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
>  	struct btrfs_root *root = fs_info->extent_root;
>  
> -	while(extent_root_pending_ops(fs_info)) {
> -		ret = finish_current_insert(trans);
> -		if (ret)
> -			return ret;
> -		ret = del_pending_extents(trans);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = btrfs_run_delayed_refs(trans, -1);
> +	if (ret)
> +		return ret;
>  
>  	while(1) {
>  		cache = btrfs_lookup_first_block_group(fs_info, start);
> @@ -4026,7 +4030,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>  		} else if (ret != -EEXIST) {
>  			goto fail;
>  		}
> -		btrfs_extent_post_op(trans);
> +		btrfs_run_delayed_refs(trans, -1);
>  		extent_bytenr = disk_bytenr;
>  		extent_num_bytes = num_bytes;
>  		extent_offset = 0;
> diff --git a/transaction.c b/transaction.c
> index 96d9891b0d1c..bfda769210ee 100644
> --- a/transaction.c
> +++ b/transaction.c
> @@ -61,7 +61,6 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
>  	u64 old_root_bytenr;
>  	struct btrfs_root *tree_root = root->fs_info->tree_root;
>  
> -	btrfs_write_dirty_block_groups(trans);
>  	while(1) {
>  		old_root_bytenr = btrfs_root_bytenr(&root->root_item);
>  		if (old_root_bytenr == root->node->start)
> @@ -98,6 +97,17 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * If the above CoW is the first one to dirty the current tree_root,
> +	 * delayed refs for it won't be run until after this function has
> +	 * finished executing, meaning we won't process the extent tree root,
> +	 * which will have been added to ->dirty_cowonly_roots.  So run
> +	 * delayed refs here as well.
> +	 */
> +	ret = btrfs_run_delayed_refs(trans, -1);
> +	if (ret)
> +		return ret;
> +
>  	while(!list_empty(&fs_info->dirty_cowonly_roots)) {
>  		next = fs_info->dirty_cowonly_roots.next;
>  		list_del_init(next);
> @@ -147,6 +157,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>  
>  	if (trans->fs_info->transaction_aborted)
>  		return -EROFS;
> +	/*
> +	 * Flush all accumulated delayed refs so that root-tree updates are
> +	 * consistent
> +	 */
> +	ret = btrfs_run_delayed_refs(trans, -1);
> +	BUG_ON(ret);
>  
>  	if (root->commit_root == root->node)
>  		goto commit_tree;
> @@ -164,11 +180,18 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>  	ret = btrfs_update_root(trans, root->fs_info->tree_root,
>  				&root->root_key, &root->root_item);
>  	BUG_ON(ret);
> +
>  commit_tree:
>  	ret = commit_tree_roots(trans, fs_info);
>  	BUG_ON(ret);
> -	ret = __commit_transaction(trans, root);
> +	/*
> +	 * Ensure that all comitted roots are properly accounted in the
> +	 * extent tree
> +	 */
> +	ret = btrfs_run_delayed_refs(trans, -1);
>  	BUG_ON(ret);
> +	btrfs_write_dirty_block_groups(trans);
> +	__commit_transaction(trans, root);
>  	write_ctree_super(trans);
>  	btrfs_finish_extent_commit(trans, fs_info->extent_root,
>  			           &fs_info->pinned_extents);
> 

Re: [PATCH 5/8] btrfs-progs: Wire up delayed refs

[Nikolay Borisov]

On  5.09.2018 05:10, Qu Wenruo wrote:
> 
> 
> On 2018/8/16 下午9:10, Nikolay Borisov wrote:
>> This commit enables the delayed refs infrastructures. This entails doing
>> the following:
>>
>> 1. Replacing existing calls of btrfs_extent_post_op (which is the
>> equivalent of delayed refs) with the proper btrfs_run_delayed_refs.
>> As well as eliminating open-coded calls to finish_current_insert and
>> del_pending_extents which execute the delayed ops.
>>
>> 2. Wiring up the addition of delayed refs when freeing extents
>> (btrfs_free_extent) and when adding new extents (alloc_tree_block).
>>
>> 3. Adding calls to btrfs_run_delayed refs in the transaction commit
>> path alongside comments why every call is needed, since it's not always
>> obvious (those call sites were derived empirically by running and
>> debugging existing tests)
>>
>> 4. Correctly flagging the transaction in which we are reinitialising
>> the extent tree.
>>
>> 5 Moving btrfs_write_dirty_block_groups to btrfs_write_dirty_block_groups
>> since blockgroups should be written to disk after the last delayed refs
>> have been run.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> Signed-off-by: David Sterba <dsterba@suse.com>
> 
> Is there something (maybe btrfs_run_delayed_refs()?) missing in btrfs-image?
> 
> btrfs-image from devel branch can't restore image correctly, the block
> group used bytes is not correct, thus it can't pass misc nor fsck tests.

This is really strange, all fsck/misc tests passed with those patches.
Can you be more specific which tests exactly you mean ?

> 
> Thanks,
> Qu
> 
>> ---
>>  check/main.c  |   3 +-
>>  extent-tree.c | 166 ++++++++++++++++++++++++++++++----------------------------
>>  transaction.c |  27 +++++++++-
>>  3 files changed, 112 insertions(+), 84 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index bc2ee22f7943..b361cd7e26a0 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -8710,7 +8710,7 @@ static int reinit_extent_tree(struct btrfs_trans_handle *trans,
>>  			fprintf(stderr, "Error adding block group\n");
>>  			return ret;
>>  		}
>> -		btrfs_extent_post_op(trans);
>> +		btrfs_run_delayed_refs(trans, -1);
>>  	}
>>  
>>  	ret = reset_balance(trans, fs_info);
>> @@ -9767,6 +9767,7 @@ int cmd_check(int argc, char **argv)
>>  			goto close_out;
>>  		}
>>  
>> +		trans->reinit_extent_tree = true;
>>  		if (init_extent_tree) {
>>  			printf("Creating a new extent tree\n");
>>  			ret = reinit_extent_tree(trans, info,
>> diff --git a/extent-tree.c b/extent-tree.c
>> index 7d6c37c6b371..2fa51bbc0359 100644
>> --- a/extent-tree.c
>> +++ b/extent-tree.c
>> @@ -1418,8 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>>  		err = ret;
>>  out:
>>  	btrfs_free_path(path);
>> -	finish_current_insert(trans);
>> -	del_pending_extents(trans);
>>  	BUG_ON(err);
>>  	return err;
>>  }
>> @@ -1602,8 +1600,6 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, u64 bytenr,
>>  	btrfs_set_extent_flags(l, item, flags);
>>  out:
>>  	btrfs_free_path(path);
>> -	finish_current_insert(trans);
>> -	del_pending_extents(trans);
>>  	return ret;
>>  }
>>  
>> @@ -1701,7 +1697,6 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>  				 struct btrfs_block_group_cache *cache)
>>  {
>>  	int ret;
>> -	int pending_ret;
>>  	struct btrfs_root *extent_root = trans->fs_info->extent_root;
>>  	unsigned long bi;
>>  	struct extent_buffer *leaf;
>> @@ -1717,12 +1712,8 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>  	btrfs_mark_buffer_dirty(leaf);
>>  	btrfs_release_path(path);
>>  fail:
>> -	finish_current_insert(trans);
>> -	pending_ret = del_pending_extents(trans);
>>  	if (ret)
>>  		return ret;
>> -	if (pending_ret)
>> -		return pending_ret;
>>  	return 0;
>>  
>>  }
>> @@ -2049,6 +2040,7 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>  	int skinny_metadata =
>>  		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
>>  
>> +
>>  	while(1) {
>>  		ret = find_first_extent_bit(&info->extent_ins, 0, &start,
>>  					    &end, EXTENT_LOCKED);
>> @@ -2080,6 +2072,8 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>  			BUG_ON(1);
>>  		}
>>  
>> +
>> +		printf("shouldn't be executed\n");
>>  		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
>>  		kfree(extent_op);
>>  	}
>> @@ -2379,7 +2373,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
>>  	}
>>  fail:
>>  	btrfs_free_path(path);
>> -	finish_current_insert(trans);
>>  	return ret;
>>  }
>>  
>> @@ -2462,33 +2455,30 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>>  		      u64 bytenr, u64 num_bytes, u64 parent,
>>  		      u64 root_objectid, u64 owner, u64 offset)
>>  {
>> -	struct btrfs_root *extent_root = root->fs_info->extent_root;
>> -	int pending_ret;
>>  	int ret;
>>  
>>  	WARN_ON(num_bytes < root->fs_info->sectorsize);
>> -	if (root == extent_root) {
>> -		struct pending_extent_op *extent_op;
>> -
>> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
>> -		BUG_ON(!extent_op);
>> -
>> -		extent_op->type = PENDING_EXTENT_DELETE;
>> -		extent_op->bytenr = bytenr;
>> -		extent_op->num_bytes = num_bytes;
>> -		extent_op->level = (int)owner;
>> -
>> -		set_extent_bits(&root->fs_info->pending_del,
>> -				bytenr, bytenr + num_bytes - 1,
>> -				EXTENT_LOCKED);
>> -		set_state_private(&root->fs_info->pending_del,
>> -				  bytenr, (unsigned long)extent_op);
>> -		return 0;
>> +	/*
>> +	 * tree log blocks never actually go into the extent allocation
>> +	 * tree, just update pinning info and exit early.
>> +	 */
>> +	if (root_objectid == BTRFS_TREE_LOG_OBJECTID) {
>> +		printf("PINNING EXTENTS IN LOG TREE\n");
>> +		WARN_ON(owner >= BTRFS_FIRST_FREE_OBJECTID);
>> +		btrfs_pin_extent(trans->fs_info, bytenr, num_bytes);
>> +		ret = 0;
>> +	} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
>> +		BUG_ON(offset);
>> +		ret = btrfs_add_delayed_tree_ref(trans->fs_info, trans,
>> +						 bytenr, num_bytes, parent,
>> +						 root_objectid, (int)owner,
>> +						 BTRFS_DROP_DELAYED_REF,
>> +						 NULL, NULL, NULL);
>> +	} else {
>> +		ret = __free_extent(trans, bytenr, num_bytes, parent,
>> +				    root_objectid, owner, offset, 1);
>>  	}
>> -	ret = __free_extent(trans, bytenr, num_bytes, parent,
>> -			    root_objectid, owner, offset, 1);
>> -	pending_ret = del_pending_extents(trans);
>> -	return ret ? ret : pending_ret;
>> +	return ret;
>>  }
>>  
>>  static u64 stripe_align(struct btrfs_root *root, u64 val)
>> @@ -2694,6 +2684,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>  	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
>>  	struct btrfs_key ins;
>>  	bool skinny_metadata = btrfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
>> +	int ret;
>> +	u64 start, end;
>>  
>>  	ins.objectid = node->bytenr;
>>  	if (skinny_metadata) {
>> @@ -2704,10 +2696,25 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>  		ins.type = BTRFS_EXTENT_ITEM_KEY;
>>  	}
>>  
>> -	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
>> -					 extent_op->flags_to_set,
>> -					 &extent_op->key, ref->level, &ins);
>> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
>> +		ret = find_first_extent_bit(&trans->fs_info->extent_ins,
>> +					    node->bytenr, &start, &end,
>> +					    EXTENT_LOCKED);
>> +		ASSERT(!ret);
>> +		ASSERT(start == node->bytenr);
>> +		ASSERT(end == node->bytenr + node->num_bytes - 1);
>> +	}
>> +
>> +	ret = alloc_reserved_tree_block(trans, ref->root, trans->transid,
>> +					extent_op->flags_to_set,
>> +					&extent_op->key, ref->level, &ins);
>>  
>> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
>> +		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
>> +				  EXTENT_LOCKED);
>> +	}
>> +
>> +	return ret;
>>  }
>>  
>>  static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>> @@ -2772,39 +2779,50 @@ static int alloc_tree_block(struct btrfs_trans_handle *trans,
>>  			    u64 search_end, struct btrfs_key *ins)
>>  {
>>  	int ret;
>> +	u64 extent_size;
>> +	struct btrfs_delayed_extent_op *extent_op;
>> +	bool skinny_metadata = btrfs_fs_incompat(root->fs_info,
>> +						 SKINNY_METADATA);
>> +
>> +	extent_op = btrfs_alloc_delayed_extent_op();
>> +	if (!extent_op)
>> +		return -ENOMEM;
>> +
>>  	ret = btrfs_reserve_extent(trans, root, num_bytes, empty_size,
>>  				   hint_byte, search_end, ins, 0);
>>  	BUG_ON(ret);
>>  
>> +	if (key)
>> +		memcpy(&extent_op->key, key, sizeof(extent_op->key));
>> +	else
>> +		memset(&extent_op->key, 0, sizeof(extent_op->key));
>> +	extent_op->flags_to_set = flags;
>> +	extent_op->update_key = skinny_metadata ? false : true;
>> +	extent_op->update_flags = true;
>> +	extent_op->is_data = false;
>> +	extent_op->level = level;
>> +
>> +	extent_size = ins->offset;
>> +
>> +	if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>> +		ins->offset = level;
>> +		ins->type = BTRFS_METADATA_ITEM_KEY;
>> +	}
>> +
>> +	/* Ensure this reserved extent is not found by the allocator */
>>  	if (root_objectid == BTRFS_EXTENT_TREE_OBJECTID) {
>> -		struct pending_extent_op *extent_op;
>> -
>> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
>> -		BUG_ON(!extent_op);
>> -
>> -		extent_op->type = PENDING_EXTENT_INSERT;
>> -		extent_op->bytenr = ins->objectid;
>> -		extent_op->num_bytes = ins->offset;
>> -		extent_op->level = level;
>> -		extent_op->flags = flags;
>> -		memcpy(&extent_op->key, key, sizeof(*key));
>> -
>> -		set_extent_bits(&root->fs_info->extent_ins, ins->objectid,
>> -				ins->objectid + ins->offset - 1,
>> -				EXTENT_LOCKED);
>> -		set_state_private(&root->fs_info->extent_ins,
>> -				  ins->objectid, (unsigned long)extent_op);
>> -	} else {
>> -		if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>> -			ins->offset = level;
>> -			ins->type = BTRFS_METADATA_ITEM_KEY;
>> -		}
>> -		ret = alloc_reserved_tree_block(trans, root_objectid,
>> -						generation, flags,
>> -						key, level, ins);
>> -		finish_current_insert(trans);
>> -		del_pending_extents(trans);
>> +		ret = set_extent_bits(&trans->fs_info->extent_ins,
>> +				      ins->objectid,
>> +				      ins->objectid + extent_size - 1,
>> +				      EXTENT_LOCKED);
>> +
>> +		BUG_ON(ret);
>>  	}
>> +
>> +	ret = btrfs_add_delayed_tree_ref(root->fs_info, trans, ins->objectid,
>> +					 extent_size, 0, root_objectid,
>> +					 level, BTRFS_ADD_DELAYED_EXTENT,
>> +					 extent_op, NULL, NULL);
>>  	return ret;
>>  }
>>  
>> @@ -3329,11 +3347,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
>>  				sizeof(cache->item));
>>  	BUG_ON(ret);
>>  
>> -	ret = finish_current_insert(trans);
>> -	BUG_ON(ret);
>> -	ret = del_pending_extents(trans);
>> -	BUG_ON(ret);
>> -
>>  	return 0;
>>  }
>>  
>> @@ -3429,10 +3442,6 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
>>  					sizeof(cache->item));
>>  		BUG_ON(ret);
>>  
>> -		finish_current_insert(trans);
>> -		ret = del_pending_extents(trans);
>> -		BUG_ON(ret);
>> -
>>  		cur_start = cache->key.objectid + cache->key.offset;
>>  	}
>>  	return 0;
>> @@ -3814,14 +3823,9 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans)
>>  	struct btrfs_fs_info *fs_info = trans->fs_info;
>>  	struct btrfs_root *root = fs_info->extent_root;
>>  
>> -	while(extent_root_pending_ops(fs_info)) {
>> -		ret = finish_current_insert(trans);
>> -		if (ret)
>> -			return ret;
>> -		ret = del_pending_extents(trans);
>> -		if (ret)
>> -			return ret;
>> -	}
>> +	ret = btrfs_run_delayed_refs(trans, -1);
>> +	if (ret)
>> +		return ret;
>>  
>>  	while(1) {
>>  		cache = btrfs_lookup_first_block_group(fs_info, start);
>> @@ -4026,7 +4030,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>>  		} else if (ret != -EEXIST) {
>>  			goto fail;
>>  		}
>> -		btrfs_extent_post_op(trans);
>> +		btrfs_run_delayed_refs(trans, -1);
>>  		extent_bytenr = disk_bytenr;
>>  		extent_num_bytes = num_bytes;
>>  		extent_offset = 0;
>> diff --git a/transaction.c b/transaction.c
>> index 96d9891b0d1c..bfda769210ee 100644
>> --- a/transaction.c
>> +++ b/transaction.c
>> @@ -61,7 +61,6 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
>>  	u64 old_root_bytenr;
>>  	struct btrfs_root *tree_root = root->fs_info->tree_root;
>>  
>> -	btrfs_write_dirty_block_groups(trans);
>>  	while(1) {
>>  		old_root_bytenr = btrfs_root_bytenr(&root->root_item);
>>  		if (old_root_bytenr == root->node->start)
>> @@ -98,6 +97,17 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
>>  	if (ret)
>>  		return ret;
>>  
>> +	/*
>> +	 * If the above CoW is the first one to dirty the current tree_root,
>> +	 * delayed refs for it won't be run until after this function has
>> +	 * finished executing, meaning we won't process the extent tree root,
>> +	 * which will have been added to ->dirty_cowonly_roots.  So run
>> +	 * delayed refs here as well.
>> +	 */
>> +	ret = btrfs_run_delayed_refs(trans, -1);
>> +	if (ret)
>> +		return ret;
>> +
>>  	while(!list_empty(&fs_info->dirty_cowonly_roots)) {
>>  		next = fs_info->dirty_cowonly_roots.next;
>>  		list_del_init(next);
>> @@ -147,6 +157,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>  
>>  	if (trans->fs_info->transaction_aborted)
>>  		return -EROFS;
>> +	/*
>> +	 * Flush all accumulated delayed refs so that root-tree updates are
>> +	 * consistent
>> +	 */
>> +	ret = btrfs_run_delayed_refs(trans, -1);
>> +	BUG_ON(ret);
>>  
>>  	if (root->commit_root == root->node)
>>  		goto commit_tree;
>> @@ -164,11 +180,18 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>  	ret = btrfs_update_root(trans, root->fs_info->tree_root,
>>  				&root->root_key, &root->root_item);
>>  	BUG_ON(ret);
>> +
>>  commit_tree:
>>  	ret = commit_tree_roots(trans, fs_info);
>>  	BUG_ON(ret);
>> -	ret = __commit_transaction(trans, root);
>> +	/*
>> +	 * Ensure that all comitted roots are properly accounted in the
>> +	 * extent tree
>> +	 */
>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>  	BUG_ON(ret);
>> +	btrfs_write_dirty_block_groups(trans);
>> +	__commit_transaction(trans, root);
>>  	write_ctree_super(trans);
>>  	btrfs_finish_extent_commit(trans, fs_info->extent_root,
>>  			           &fs_info->pinned_extents);
>>
> 

Re: [PATCH 5/8] btrfs-progs: Wire up delayed refs

[Qu Wenruo]

On 2018/9/5 下午1:42, Nikolay Borisov wrote:
> 
> 
> On  5.09.2018 05:10, Qu Wenruo wrote:
>>
>>
>> On 2018/8/16 下午9:10, Nikolay Borisov wrote:
>>> This commit enables the delayed refs infrastructures. This entails doing
>>> the following:
>>>
>>> 1. Replacing existing calls of btrfs_extent_post_op (which is the
>>> equivalent of delayed refs) with the proper btrfs_run_delayed_refs.
>>> As well as eliminating open-coded calls to finish_current_insert and
>>> del_pending_extents which execute the delayed ops.
>>>
>>> 2. Wiring up the addition of delayed refs when freeing extents
>>> (btrfs_free_extent) and when adding new extents (alloc_tree_block).
>>>
>>> 3. Adding calls to btrfs_run_delayed refs in the transaction commit
>>> path alongside comments why every call is needed, since it's not always
>>> obvious (those call sites were derived empirically by running and
>>> debugging existing tests)
>>>
>>> 4. Correctly flagging the transaction in which we are reinitialising
>>> the extent tree.
>>>
>>> 5 Moving btrfs_write_dirty_block_groups to btrfs_write_dirty_block_groups
>>> since blockgroups should be written to disk after the last delayed refs
>>> have been run.
>>>
>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>> Signed-off-by: David Sterba <dsterba@suse.com>
>>
>> Is there something (maybe btrfs_run_delayed_refs()?) missing in btrfs-image?
>>
>> btrfs-image from devel branch can't restore image correctly, the block
>> group used bytes is not correct, thus it can't pass misc nor fsck tests.
> 
> This is really strange, all fsck/misc tests passed with those patches.
> Can you be more specific which tests exactly you mean ?

One case is fsck/020 with lowmem mode. (Original mode lacks block
group->used check).

More specifically, fsck/020/keyed_data_ref_with_shared_leaf.img

Using btrfs-image from my distribution (v4.17.1) and devel branch btrfs
check: (cwd is btrfs-progs, devel branch)

$ btrfs-image -r
tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img
$ btrfs check --mode=lowmem ~/test.img
Opening filesystem to check...
Checking filesystem on /home/adam/test.img
UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
[1/7] checking root items
[2/7] checking extents
[3/7] checking free space cache
[4/7] checking fs roots
[5/7] checking only csums items (without verifying data)
[6/7] checking root refs done with fs roots in lowmem mode, skipping
[7/7] checking quota groups skipped (not enabled on this FS)
found 1208320 bytes used, no error found
total csum bytes: 512
total tree bytes: 684032
total fs tree bytes: 638976
total extent tree bytes: 16384
btree space waste bytes: 305606
file data blocks allocated: 93847552
 referenced 1773568

But if using btrfs-image with your delayed ref patch:
$ ./btrfs-image -r
tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img

# No matter if I'm using btrfs-check from devel or 4.17.1
$ btrfs check --mode=lowmem ~/test.img
Opening filesystem to check...
Checking filesystem on /home/adam/test.img
UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
[1/7] checking root items
[2/7] checking extents
ERROR: block group[4194304 8388608] used 20480 but extent items used 24576
ERROR: block group[20971520 16777216] used 659456 but extent items used
655360
ERROR: errors found in extent allocation tree or chunk allocation
[3/7] checking free space cache
[4/7] checking fs roots
[5/7] checking only csums items (without verifying data)
[6/7] checking root refs done with fs roots in lowmem mode, skipping
[7/7] checking quota groups skipped (not enabled on this FS)
found 1208320 bytes used, error(s) found
total csum bytes: 512
total tree bytes: 684032
total fs tree bytes: 638976
total extent tree bytes: 16384
btree space waste bytes: 305606
file data blocks allocated: 93847552
 referenced 1773568

I'd say, although lowmem check is still far from perfect, it indeed has
extra checks original mode lacks, and in this case it indeed exposes
problem.

Thanks,
Qu


> 
>>
>> Thanks,
>> Qu
>>
>>> ---
>>>  check/main.c  |   3 +-
>>>  extent-tree.c | 166 ++++++++++++++++++++++++++++++----------------------------
>>>  transaction.c |  27 +++++++++-
>>>  3 files changed, 112 insertions(+), 84 deletions(-)
>>>
>>> diff --git a/check/main.c b/check/main.c
>>> index bc2ee22f7943..b361cd7e26a0 100644
>>> --- a/check/main.c
>>> +++ b/check/main.c
>>> @@ -8710,7 +8710,7 @@ static int reinit_extent_tree(struct btrfs_trans_handle *trans,
>>>  			fprintf(stderr, "Error adding block group\n");
>>>  			return ret;
>>>  		}
>>> -		btrfs_extent_post_op(trans);
>>> +		btrfs_run_delayed_refs(trans, -1);
>>>  	}
>>>  
>>>  	ret = reset_balance(trans, fs_info);
>>> @@ -9767,6 +9767,7 @@ int cmd_check(int argc, char **argv)
>>>  			goto close_out;
>>>  		}
>>>  
>>> +		trans->reinit_extent_tree = true;
>>>  		if (init_extent_tree) {
>>>  			printf("Creating a new extent tree\n");
>>>  			ret = reinit_extent_tree(trans, info,
>>> diff --git a/extent-tree.c b/extent-tree.c
>>> index 7d6c37c6b371..2fa51bbc0359 100644
>>> --- a/extent-tree.c
>>> +++ b/extent-tree.c
>>> @@ -1418,8 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>>>  		err = ret;
>>>  out:
>>>  	btrfs_free_path(path);
>>> -	finish_current_insert(trans);
>>> -	del_pending_extents(trans);
>>>  	BUG_ON(err);
>>>  	return err;
>>>  }
>>> @@ -1602,8 +1600,6 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, u64 bytenr,
>>>  	btrfs_set_extent_flags(l, item, flags);
>>>  out:
>>>  	btrfs_free_path(path);
>>> -	finish_current_insert(trans);
>>> -	del_pending_extents(trans);
>>>  	return ret;
>>>  }
>>>  
>>> @@ -1701,7 +1697,6 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>  				 struct btrfs_block_group_cache *cache)
>>>  {
>>>  	int ret;
>>> -	int pending_ret;
>>>  	struct btrfs_root *extent_root = trans->fs_info->extent_root;
>>>  	unsigned long bi;
>>>  	struct extent_buffer *leaf;
>>> @@ -1717,12 +1712,8 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>  	btrfs_mark_buffer_dirty(leaf);
>>>  	btrfs_release_path(path);
>>>  fail:
>>> -	finish_current_insert(trans);
>>> -	pending_ret = del_pending_extents(trans);
>>>  	if (ret)
>>>  		return ret;
>>> -	if (pending_ret)
>>> -		return pending_ret;
>>>  	return 0;
>>>  
>>>  }
>>> @@ -2049,6 +2040,7 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>  	int skinny_metadata =
>>>  		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
>>>  
>>> +
>>>  	while(1) {
>>>  		ret = find_first_extent_bit(&info->extent_ins, 0, &start,
>>>  					    &end, EXTENT_LOCKED);
>>> @@ -2080,6 +2072,8 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>  			BUG_ON(1);
>>>  		}
>>>  
>>> +
>>> +		printf("shouldn't be executed\n");
>>>  		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
>>>  		kfree(extent_op);
>>>  	}
>>> @@ -2379,7 +2373,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
>>>  	}
>>>  fail:
>>>  	btrfs_free_path(path);
>>> -	finish_current_insert(trans);
>>>  	return ret;
>>>  }
>>>  
>>> @@ -2462,33 +2455,30 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>>>  		      u64 bytenr, u64 num_bytes, u64 parent,
>>>  		      u64 root_objectid, u64 owner, u64 offset)
>>>  {
>>> -	struct btrfs_root *extent_root = root->fs_info->extent_root;
>>> -	int pending_ret;
>>>  	int ret;
>>>  
>>>  	WARN_ON(num_bytes < root->fs_info->sectorsize);
>>> -	if (root == extent_root) {
>>> -		struct pending_extent_op *extent_op;
>>> -
>>> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
>>> -		BUG_ON(!extent_op);
>>> -
>>> -		extent_op->type = PENDING_EXTENT_DELETE;
>>> -		extent_op->bytenr = bytenr;
>>> -		extent_op->num_bytes = num_bytes;
>>> -		extent_op->level = (int)owner;
>>> -
>>> -		set_extent_bits(&root->fs_info->pending_del,
>>> -				bytenr, bytenr + num_bytes - 1,
>>> -				EXTENT_LOCKED);
>>> -		set_state_private(&root->fs_info->pending_del,
>>> -				  bytenr, (unsigned long)extent_op);
>>> -		return 0;
>>> +	/*
>>> +	 * tree log blocks never actually go into the extent allocation
>>> +	 * tree, just update pinning info and exit early.
>>> +	 */
>>> +	if (root_objectid == BTRFS_TREE_LOG_OBJECTID) {
>>> +		printf("PINNING EXTENTS IN LOG TREE\n");
>>> +		WARN_ON(owner >= BTRFS_FIRST_FREE_OBJECTID);
>>> +		btrfs_pin_extent(trans->fs_info, bytenr, num_bytes);
>>> +		ret = 0;
>>> +	} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
>>> +		BUG_ON(offset);
>>> +		ret = btrfs_add_delayed_tree_ref(trans->fs_info, trans,
>>> +						 bytenr, num_bytes, parent,
>>> +						 root_objectid, (int)owner,
>>> +						 BTRFS_DROP_DELAYED_REF,
>>> +						 NULL, NULL, NULL);
>>> +	} else {
>>> +		ret = __free_extent(trans, bytenr, num_bytes, parent,
>>> +				    root_objectid, owner, offset, 1);
>>>  	}
>>> -	ret = __free_extent(trans, bytenr, num_bytes, parent,
>>> -			    root_objectid, owner, offset, 1);
>>> -	pending_ret = del_pending_extents(trans);
>>> -	return ret ? ret : pending_ret;
>>> +	return ret;
>>>  }
>>>  
>>>  static u64 stripe_align(struct btrfs_root *root, u64 val)
>>> @@ -2694,6 +2684,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>  	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
>>>  	struct btrfs_key ins;
>>>  	bool skinny_metadata = btrfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
>>> +	int ret;
>>> +	u64 start, end;
>>>  
>>>  	ins.objectid = node->bytenr;
>>>  	if (skinny_metadata) {
>>> @@ -2704,10 +2696,25 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>  		ins.type = BTRFS_EXTENT_ITEM_KEY;
>>>  	}
>>>  
>>> -	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
>>> -					 extent_op->flags_to_set,
>>> -					 &extent_op->key, ref->level, &ins);
>>> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
>>> +		ret = find_first_extent_bit(&trans->fs_info->extent_ins,
>>> +					    node->bytenr, &start, &end,
>>> +					    EXTENT_LOCKED);
>>> +		ASSERT(!ret);
>>> +		ASSERT(start == node->bytenr);
>>> +		ASSERT(end == node->bytenr + node->num_bytes - 1);
>>> +	}
>>> +
>>> +	ret = alloc_reserved_tree_block(trans, ref->root, trans->transid,
>>> +					extent_op->flags_to_set,
>>> +					&extent_op->key, ref->level, &ins);
>>>  
>>> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
>>> +		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
>>> +				  EXTENT_LOCKED);
>>> +	}
>>> +
>>> +	return ret;
>>>  }
>>>  
>>>  static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>>> @@ -2772,39 +2779,50 @@ static int alloc_tree_block(struct btrfs_trans_handle *trans,
>>>  			    u64 search_end, struct btrfs_key *ins)
>>>  {
>>>  	int ret;
>>> +	u64 extent_size;
>>> +	struct btrfs_delayed_extent_op *extent_op;
>>> +	bool skinny_metadata = btrfs_fs_incompat(root->fs_info,
>>> +						 SKINNY_METADATA);
>>> +
>>> +	extent_op = btrfs_alloc_delayed_extent_op();
>>> +	if (!extent_op)
>>> +		return -ENOMEM;
>>> +
>>>  	ret = btrfs_reserve_extent(trans, root, num_bytes, empty_size,
>>>  				   hint_byte, search_end, ins, 0);
>>>  	BUG_ON(ret);
>>>  
>>> +	if (key)
>>> +		memcpy(&extent_op->key, key, sizeof(extent_op->key));
>>> +	else
>>> +		memset(&extent_op->key, 0, sizeof(extent_op->key));
>>> +	extent_op->flags_to_set = flags;
>>> +	extent_op->update_key = skinny_metadata ? false : true;
>>> +	extent_op->update_flags = true;
>>> +	extent_op->is_data = false;
>>> +	extent_op->level = level;
>>> +
>>> +	extent_size = ins->offset;
>>> +
>>> +	if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>> +		ins->offset = level;
>>> +		ins->type = BTRFS_METADATA_ITEM_KEY;
>>> +	}
>>> +
>>> +	/* Ensure this reserved extent is not found by the allocator */
>>>  	if (root_objectid == BTRFS_EXTENT_TREE_OBJECTID) {
>>> -		struct pending_extent_op *extent_op;
>>> -
>>> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
>>> -		BUG_ON(!extent_op);
>>> -
>>> -		extent_op->type = PENDING_EXTENT_INSERT;
>>> -		extent_op->bytenr = ins->objectid;
>>> -		extent_op->num_bytes = ins->offset;
>>> -		extent_op->level = level;
>>> -		extent_op->flags = flags;
>>> -		memcpy(&extent_op->key, key, sizeof(*key));
>>> -
>>> -		set_extent_bits(&root->fs_info->extent_ins, ins->objectid,
>>> -				ins->objectid + ins->offset - 1,
>>> -				EXTENT_LOCKED);
>>> -		set_state_private(&root->fs_info->extent_ins,
>>> -				  ins->objectid, (unsigned long)extent_op);
>>> -	} else {
>>> -		if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>> -			ins->offset = level;
>>> -			ins->type = BTRFS_METADATA_ITEM_KEY;
>>> -		}
>>> -		ret = alloc_reserved_tree_block(trans, root_objectid,
>>> -						generation, flags,
>>> -						key, level, ins);
>>> -		finish_current_insert(trans);
>>> -		del_pending_extents(trans);
>>> +		ret = set_extent_bits(&trans->fs_info->extent_ins,
>>> +				      ins->objectid,
>>> +				      ins->objectid + extent_size - 1,
>>> +				      EXTENT_LOCKED);
>>> +
>>> +		BUG_ON(ret);
>>>  	}
>>> +
>>> +	ret = btrfs_add_delayed_tree_ref(root->fs_info, trans, ins->objectid,
>>> +					 extent_size, 0, root_objectid,
>>> +					 level, BTRFS_ADD_DELAYED_EXTENT,
>>> +					 extent_op, NULL, NULL);
>>>  	return ret;
>>>  }
>>>  
>>> @@ -3329,11 +3347,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
>>>  				sizeof(cache->item));
>>>  	BUG_ON(ret);
>>>  
>>> -	ret = finish_current_insert(trans);
>>> -	BUG_ON(ret);
>>> -	ret = del_pending_extents(trans);
>>> -	BUG_ON(ret);
>>> -
>>>  	return 0;
>>>  }
>>>  
>>> @@ -3429,10 +3442,6 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
>>>  					sizeof(cache->item));
>>>  		BUG_ON(ret);
>>>  
>>> -		finish_current_insert(trans);
>>> -		ret = del_pending_extents(trans);
>>> -		BUG_ON(ret);
>>> -
>>>  		cur_start = cache->key.objectid + cache->key.offset;
>>>  	}
>>>  	return 0;
>>> @@ -3814,14 +3823,9 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans)
>>>  	struct btrfs_fs_info *fs_info = trans->fs_info;
>>>  	struct btrfs_root *root = fs_info->extent_root;
>>>  
>>> -	while(extent_root_pending_ops(fs_info)) {
>>> -		ret = finish_current_insert(trans);
>>> -		if (ret)
>>> -			return ret;
>>> -		ret = del_pending_extents(trans);
>>> -		if (ret)
>>> -			return ret;
>>> -	}
>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>> +	if (ret)
>>> +		return ret;
>>>  
>>>  	while(1) {
>>>  		cache = btrfs_lookup_first_block_group(fs_info, start);
>>> @@ -4026,7 +4030,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>>>  		} else if (ret != -EEXIST) {
>>>  			goto fail;
>>>  		}
>>> -		btrfs_extent_post_op(trans);
>>> +		btrfs_run_delayed_refs(trans, -1);
>>>  		extent_bytenr = disk_bytenr;
>>>  		extent_num_bytes = num_bytes;
>>>  		extent_offset = 0;
>>> diff --git a/transaction.c b/transaction.c
>>> index 96d9891b0d1c..bfda769210ee 100644
>>> --- a/transaction.c
>>> +++ b/transaction.c
>>> @@ -61,7 +61,6 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
>>>  	u64 old_root_bytenr;
>>>  	struct btrfs_root *tree_root = root->fs_info->tree_root;
>>>  
>>> -	btrfs_write_dirty_block_groups(trans);
>>>  	while(1) {
>>>  		old_root_bytenr = btrfs_root_bytenr(&root->root_item);
>>>  		if (old_root_bytenr == root->node->start)
>>> @@ -98,6 +97,17 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> +	/*
>>> +	 * If the above CoW is the first one to dirty the current tree_root,
>>> +	 * delayed refs for it won't be run until after this function has
>>> +	 * finished executing, meaning we won't process the extent tree root,
>>> +	 * which will have been added to ->dirty_cowonly_roots.  So run
>>> +	 * delayed refs here as well.
>>> +	 */
>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>>  	while(!list_empty(&fs_info->dirty_cowonly_roots)) {
>>>  		next = fs_info->dirty_cowonly_roots.next;
>>>  		list_del_init(next);
>>> @@ -147,6 +157,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>  
>>>  	if (trans->fs_info->transaction_aborted)
>>>  		return -EROFS;
>>> +	/*
>>> +	 * Flush all accumulated delayed refs so that root-tree updates are
>>> +	 * consistent
>>> +	 */
>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>> +	BUG_ON(ret);
>>>  
>>>  	if (root->commit_root == root->node)
>>>  		goto commit_tree;
>>> @@ -164,11 +180,18 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>  	ret = btrfs_update_root(trans, root->fs_info->tree_root,
>>>  				&root->root_key, &root->root_item);
>>>  	BUG_ON(ret);
>>> +
>>>  commit_tree:
>>>  	ret = commit_tree_roots(trans, fs_info);
>>>  	BUG_ON(ret);
>>> -	ret = __commit_transaction(trans, root);
>>> +	/*
>>> +	 * Ensure that all comitted roots are properly accounted in the
>>> +	 * extent tree
>>> +	 */
>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>>  	BUG_ON(ret);
>>> +	btrfs_write_dirty_block_groups(trans);
>>> +	__commit_transaction(trans, root);
>>>  	write_ctree_super(trans);
>>>  	btrfs_finish_extent_commit(trans, fs_info->extent_root,
>>>  			           &fs_info->pinned_extents);
>>>
>>

Re: [PATCH 5/8] btrfs-progs: Wire up delayed refs

[Nikolay Borisov]

On  5.09.2018 08:53, Qu Wenruo wrote:
> 
> 
> On 2018/9/5 下午1:42, Nikolay Borisov wrote:
>>
>>
>> On  5.09.2018 05:10, Qu Wenruo wrote:
>>>
>>>
>>> On 2018/8/16 下午9:10, Nikolay Borisov wrote:
>>>> This commit enables the delayed refs infrastructures. This entails doing
>>>> the following:
>>>>
>>>> 1. Replacing existing calls of btrfs_extent_post_op (which is the
>>>> equivalent of delayed refs) with the proper btrfs_run_delayed_refs.
>>>> As well as eliminating open-coded calls to finish_current_insert and
>>>> del_pending_extents which execute the delayed ops.
>>>>
>>>> 2. Wiring up the addition of delayed refs when freeing extents
>>>> (btrfs_free_extent) and when adding new extents (alloc_tree_block).
>>>>
>>>> 3. Adding calls to btrfs_run_delayed refs in the transaction commit
>>>> path alongside comments why every call is needed, since it's not always
>>>> obvious (those call sites were derived empirically by running and
>>>> debugging existing tests)
>>>>
>>>> 4. Correctly flagging the transaction in which we are reinitialising
>>>> the extent tree.
>>>>
>>>> 5 Moving btrfs_write_dirty_block_groups to btrfs_write_dirty_block_groups
>>>> since blockgroups should be written to disk after the last delayed refs
>>>> have been run.
>>>>
>>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>>> Signed-off-by: David Sterba <dsterba@suse.com>
>>>
>>> Is there something (maybe btrfs_run_delayed_refs()?) missing in btrfs-image?
>>>
>>> btrfs-image from devel branch can't restore image correctly, the block
>>> group used bytes is not correct, thus it can't pass misc nor fsck tests.
>>
>> This is really strange, all fsck/misc tests passed with those patches.
>> Can you be more specific which tests exactly you mean ?
> 
> One case is fsck/020 with lowmem mode. (Original mode lacks block
> group->used check).
> 
> More specifically, fsck/020/keyed_data_ref_with_shared_leaf.img
> 
> Using btrfs-image from my distribution (v4.17.1) and devel branch btrfs
> check: (cwd is btrfs-progs, devel branch)
> 
> $ btrfs-image -r
> tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img
> $ btrfs check --mode=lowmem ~/test.img
> Opening filesystem to check...
> Checking filesystem on /home/adam/test.img
> UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
> [1/7] checking root items
> [2/7] checking extents
> [3/7] checking free space cache
> [4/7] checking fs roots
> [5/7] checking only csums items (without verifying data)
> [6/7] checking root refs done with fs roots in lowmem mode, skipping
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 1208320 bytes used, no error found
> total csum bytes: 512
> total tree bytes: 684032
> total fs tree bytes: 638976
> total extent tree bytes: 16384
> btree space waste bytes: 305606
> file data blocks allocated: 93847552
>  referenced 1773568
> 
> But if using btrfs-image with your delayed ref patch:
> $ ./btrfs-image -r
> tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img
> 
> # No matter if I'm using btrfs-check from devel or 4.17.1
> $ btrfs check --mode=lowmem ~/test.img
> Opening filesystem to check...
> Checking filesystem on /home/adam/test.img
> UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
> [1/7] checking root items
> [2/7] checking extents
> ERROR: block group[4194304 8388608] used 20480 but extent items used 24576
> ERROR: block group[20971520 16777216] used 659456 but extent items used
> 655360
> ERROR: errors found in extent allocation tree or chunk allocation
> [3/7] checking free space cache
> [4/7] checking fs roots
> [5/7] checking only csums items (without verifying data)
> [6/7] checking root refs done with fs roots in lowmem mode, skipping
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 1208320 bytes used, error(s) found
> total csum bytes: 512
> total tree bytes: 684032
> total fs tree bytes: 638976
> total extent tree bytes: 16384
> btree space waste bytes: 305606
> file data blocks allocated: 93847552
>  referenced 1773568
> 
> I'd say, although lowmem check is still far from perfect, it indeed has
> extra checks original mode lacks, and in this case it indeed exposes
> problem.


I'm not able to reproduce it: 

make TEST_ENABLE_OVERRIDE=true TEST_ARGS_CHECK="--mode=lowmem"  test-fsck
    [TEST]   fsck-tests.sh
    [TEST/fsck]   001-bad-file-extent-bytenr
    [TEST/fsck]   002-bad-transid
    [TEST/fsck]   003-shift-offsets
    [TEST/fsck]   004-no-dir-index
    [TEST/fsck]   005-bad-item-offset
    [TEST/fsck]   006-bad-root-items
    [TEST/fsck]   007-bad-offset-snapshots
    [TEST/fsck]   008-bad-dir-index-name
    [TEST/fsck]   009-no-dir-item-or-index
    [TEST/fsck]   010-no-rootdir-inode-item
    [TEST/fsck]   011-no-inode-item
    [TEST/fsck]   012-leaf-corruption
    [TEST/fsck]   013-extent-tree-rebuild
    [TEST/fsck]   014-no-extent-info
    [TEST/fsck]   015-tree-reloc-tree
    [TEST/fsck]   016-wrong-inode-nbytes
    [TEST/fsck]   017-missing-all-file-extent
    [TEST/fsck]   018-leaf-crossing-stripes
    [TEST/fsck]   019-non-skinny-false-alert
    [TEST/fsck]   020-extent-ref-cases
    [TEST/fsck]   021-partially-dropped-snapshot-case
    [TEST/fsck]   022-qgroup-rescan-halfway
    [TEST/fsck]   023-qgroup-stack-overflow
    [TEST/fsck]   024-clear-space-cache
    [TEST/fsck]   025-file-extents
    [TEST/fsck]   026-bad-dir-item-name
    [TEST/fsck]   027-bad-extent-inline-ref-type
    [TEST/fsck]   028-unaligned-super-dev-sizes
    [TEST/fsck]   029-valid-orphan-item
    [TEST/fsck]   030-reflinked-prealloc-extents
    [TEST/fsck]   031-metadatadump-check-data-csum
    [TEST/fsck]   032-corrupted-qgroup
    [TEST/fsck]   032-freespacetree-corrupted-extent-offset
    [TEST/fsck]   033-lowmem-collission-dir-items
    [TEST/fsck]   034-bad-inode-flags
    [TEST/fsck]   035-inline-bad-ram-bytes
    [TEST/fsck]   035-rescan-not-kicked-in


git bl -9 
dac6a0a6f5ca btrfs-progs: Merge alloc_reserved_tree_block(2|) (3 weeks ago) <Nikolay Borisov>
da184a3838e6 btrfs-progs: Remove __free_extent2 (3 weeks ago) <Nikolay Borisov>
76b75e40606e btrfs-progs: Remove old delayed refs infrastructure (3 months ago) <Nikolay Borisov>
870c58e08673 btrfs-progs: Wire up delayed refs (3 months ago) <Nikolay Borisov>
e045b219e397 btrfs-progs: Make btrfs_write_dirty_block_groups take only trans argument (3 weeks ago) <Nikolay Borisov>
00f8d76c8a27 btrfs-progs: Add delayed refs infrastructure (3 months ago) <Nikolay Borisov>
d03f79e8deb6 btrfs-progs: Add alloc_reserved_tree_block2 function (3 months ago) <Nikolay Borisov>
d4a4831fffe9 btrfs-progs: Add __free_extent2 function (3 months ago) <Nikolay Borisov>
7faaca0d9f78 Btrfs progs v4.17.1 (4 weeks ago) <David Sterba>

Are you sure your branch is not dirty with other changes?


> 
> Thanks,
> Qu
> 
> 
>>
>>>
>>> Thanks,
>>> Qu
>>>
>>>> ---
>>>>  check/main.c  |   3 +-
>>>>  extent-tree.c | 166 ++++++++++++++++++++++++++++++----------------------------
>>>>  transaction.c |  27 +++++++++-
>>>>  3 files changed, 112 insertions(+), 84 deletions(-)
>>>>
>>>> diff --git a/check/main.c b/check/main.c
>>>> index bc2ee22f7943..b361cd7e26a0 100644
>>>> --- a/check/main.c
>>>> +++ b/check/main.c
>>>> @@ -8710,7 +8710,7 @@ static int reinit_extent_tree(struct btrfs_trans_handle *trans,
>>>>  			fprintf(stderr, "Error adding block group\n");
>>>>  			return ret;
>>>>  		}
>>>> -		btrfs_extent_post_op(trans);
>>>> +		btrfs_run_delayed_refs(trans, -1);
>>>>  	}
>>>>  
>>>>  	ret = reset_balance(trans, fs_info);
>>>> @@ -9767,6 +9767,7 @@ int cmd_check(int argc, char **argv)
>>>>  			goto close_out;
>>>>  		}
>>>>  
>>>> +		trans->reinit_extent_tree = true;
>>>>  		if (init_extent_tree) {
>>>>  			printf("Creating a new extent tree\n");
>>>>  			ret = reinit_extent_tree(trans, info,
>>>> diff --git a/extent-tree.c b/extent-tree.c
>>>> index 7d6c37c6b371..2fa51bbc0359 100644
>>>> --- a/extent-tree.c
>>>> +++ b/extent-tree.c
>>>> @@ -1418,8 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>>>>  		err = ret;
>>>>  out:
>>>>  	btrfs_free_path(path);
>>>> -	finish_current_insert(trans);
>>>> -	del_pending_extents(trans);
>>>>  	BUG_ON(err);
>>>>  	return err;
>>>>  }
>>>> @@ -1602,8 +1600,6 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, u64 bytenr,
>>>>  	btrfs_set_extent_flags(l, item, flags);
>>>>  out:
>>>>  	btrfs_free_path(path);
>>>> -	finish_current_insert(trans);
>>>> -	del_pending_extents(trans);
>>>>  	return ret;
>>>>  }
>>>>  
>>>> @@ -1701,7 +1697,6 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>>  				 struct btrfs_block_group_cache *cache)
>>>>  {
>>>>  	int ret;
>>>> -	int pending_ret;
>>>>  	struct btrfs_root *extent_root = trans->fs_info->extent_root;
>>>>  	unsigned long bi;
>>>>  	struct extent_buffer *leaf;
>>>> @@ -1717,12 +1712,8 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>>  	btrfs_mark_buffer_dirty(leaf);
>>>>  	btrfs_release_path(path);
>>>>  fail:
>>>> -	finish_current_insert(trans);
>>>> -	pending_ret = del_pending_extents(trans);
>>>>  	if (ret)
>>>>  		return ret;
>>>> -	if (pending_ret)
>>>> -		return pending_ret;
>>>>  	return 0;
>>>>  
>>>>  }
>>>> @@ -2049,6 +2040,7 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>>  	int skinny_metadata =
>>>>  		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
>>>>  
>>>> +
>>>>  	while(1) {
>>>>  		ret = find_first_extent_bit(&info->extent_ins, 0, &start,
>>>>  					    &end, EXTENT_LOCKED);
>>>> @@ -2080,6 +2072,8 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>>  			BUG_ON(1);
>>>>  		}
>>>>  
>>>> +
>>>> +		printf("shouldn't be executed\n");
>>>>  		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
>>>>  		kfree(extent_op);
>>>>  	}
>>>> @@ -2379,7 +2373,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
>>>>  	}
>>>>  fail:
>>>>  	btrfs_free_path(path);
>>>> -	finish_current_insert(trans);
>>>>  	return ret;
>>>>  }
>>>>  
>>>> @@ -2462,33 +2455,30 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>>>>  		      u64 bytenr, u64 num_bytes, u64 parent,
>>>>  		      u64 root_objectid, u64 owner, u64 offset)
>>>>  {
>>>> -	struct btrfs_root *extent_root = root->fs_info->extent_root;
>>>> -	int pending_ret;
>>>>  	int ret;
>>>>  
>>>>  	WARN_ON(num_bytes < root->fs_info->sectorsize);
>>>> -	if (root == extent_root) {
>>>> -		struct pending_extent_op *extent_op;
>>>> -
>>>> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
>>>> -		BUG_ON(!extent_op);
>>>> -
>>>> -		extent_op->type = PENDING_EXTENT_DELETE;
>>>> -		extent_op->bytenr = bytenr;
>>>> -		extent_op->num_bytes = num_bytes;
>>>> -		extent_op->level = (int)owner;
>>>> -
>>>> -		set_extent_bits(&root->fs_info->pending_del,
>>>> -				bytenr, bytenr + num_bytes - 1,
>>>> -				EXTENT_LOCKED);
>>>> -		set_state_private(&root->fs_info->pending_del,
>>>> -				  bytenr, (unsigned long)extent_op);
>>>> -		return 0;
>>>> +	/*
>>>> +	 * tree log blocks never actually go into the extent allocation
>>>> +	 * tree, just update pinning info and exit early.
>>>> +	 */
>>>> +	if (root_objectid == BTRFS_TREE_LOG_OBJECTID) {
>>>> +		printf("PINNING EXTENTS IN LOG TREE\n");
>>>> +		WARN_ON(owner >= BTRFS_FIRST_FREE_OBJECTID);
>>>> +		btrfs_pin_extent(trans->fs_info, bytenr, num_bytes);
>>>> +		ret = 0;
>>>> +	} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
>>>> +		BUG_ON(offset);
>>>> +		ret = btrfs_add_delayed_tree_ref(trans->fs_info, trans,
>>>> +						 bytenr, num_bytes, parent,
>>>> +						 root_objectid, (int)owner,
>>>> +						 BTRFS_DROP_DELAYED_REF,
>>>> +						 NULL, NULL, NULL);
>>>> +	} else {
>>>> +		ret = __free_extent(trans, bytenr, num_bytes, parent,
>>>> +				    root_objectid, owner, offset, 1);
>>>>  	}
>>>> -	ret = __free_extent(trans, bytenr, num_bytes, parent,
>>>> -			    root_objectid, owner, offset, 1);
>>>> -	pending_ret = del_pending_extents(trans);
>>>> -	return ret ? ret : pending_ret;
>>>> +	return ret;
>>>>  }
>>>>  
>>>>  static u64 stripe_align(struct btrfs_root *root, u64 val)
>>>> @@ -2694,6 +2684,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>>  	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
>>>>  	struct btrfs_key ins;
>>>>  	bool skinny_metadata = btrfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
>>>> +	int ret;
>>>> +	u64 start, end;
>>>>  
>>>>  	ins.objectid = node->bytenr;
>>>>  	if (skinny_metadata) {
>>>> @@ -2704,10 +2696,25 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>>  		ins.type = BTRFS_EXTENT_ITEM_KEY;
>>>>  	}
>>>>  
>>>> -	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
>>>> -					 extent_op->flags_to_set,
>>>> -					 &extent_op->key, ref->level, &ins);
>>>> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
>>>> +		ret = find_first_extent_bit(&trans->fs_info->extent_ins,
>>>> +					    node->bytenr, &start, &end,
>>>> +					    EXTENT_LOCKED);
>>>> +		ASSERT(!ret);
>>>> +		ASSERT(start == node->bytenr);
>>>> +		ASSERT(end == node->bytenr + node->num_bytes - 1);
>>>> +	}
>>>> +
>>>> +	ret = alloc_reserved_tree_block(trans, ref->root, trans->transid,
>>>> +					extent_op->flags_to_set,
>>>> +					&extent_op->key, ref->level, &ins);
>>>>  
>>>> +	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
>>>> +		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
>>>> +				  EXTENT_LOCKED);
>>>> +	}
>>>> +
>>>> +	return ret;
>>>>  }
>>>>  
>>>>  static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>>>> @@ -2772,39 +2779,50 @@ static int alloc_tree_block(struct btrfs_trans_handle *trans,
>>>>  			    u64 search_end, struct btrfs_key *ins)
>>>>  {
>>>>  	int ret;
>>>> +	u64 extent_size;
>>>> +	struct btrfs_delayed_extent_op *extent_op;
>>>> +	bool skinny_metadata = btrfs_fs_incompat(root->fs_info,
>>>> +						 SKINNY_METADATA);
>>>> +
>>>> +	extent_op = btrfs_alloc_delayed_extent_op();
>>>> +	if (!extent_op)
>>>> +		return -ENOMEM;
>>>> +
>>>>  	ret = btrfs_reserve_extent(trans, root, num_bytes, empty_size,
>>>>  				   hint_byte, search_end, ins, 0);
>>>>  	BUG_ON(ret);
>>>>  
>>>> +	if (key)
>>>> +		memcpy(&extent_op->key, key, sizeof(extent_op->key));
>>>> +	else
>>>> +		memset(&extent_op->key, 0, sizeof(extent_op->key));
>>>> +	extent_op->flags_to_set = flags;
>>>> +	extent_op->update_key = skinny_metadata ? false : true;
>>>> +	extent_op->update_flags = true;
>>>> +	extent_op->is_data = false;
>>>> +	extent_op->level = level;
>>>> +
>>>> +	extent_size = ins->offset;
>>>> +
>>>> +	if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>>> +		ins->offset = level;
>>>> +		ins->type = BTRFS_METADATA_ITEM_KEY;
>>>> +	}
>>>> +
>>>> +	/* Ensure this reserved extent is not found by the allocator */
>>>>  	if (root_objectid == BTRFS_EXTENT_TREE_OBJECTID) {
>>>> -		struct pending_extent_op *extent_op;
>>>> -
>>>> -		extent_op = kmalloc(sizeof(*extent_op), GFP_NOFS);
>>>> -		BUG_ON(!extent_op);
>>>> -
>>>> -		extent_op->type = PENDING_EXTENT_INSERT;
>>>> -		extent_op->bytenr = ins->objectid;
>>>> -		extent_op->num_bytes = ins->offset;
>>>> -		extent_op->level = level;
>>>> -		extent_op->flags = flags;
>>>> -		memcpy(&extent_op->key, key, sizeof(*key));
>>>> -
>>>> -		set_extent_bits(&root->fs_info->extent_ins, ins->objectid,
>>>> -				ins->objectid + ins->offset - 1,
>>>> -				EXTENT_LOCKED);
>>>> -		set_state_private(&root->fs_info->extent_ins,
>>>> -				  ins->objectid, (unsigned long)extent_op);
>>>> -	} else {
>>>> -		if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>>> -			ins->offset = level;
>>>> -			ins->type = BTRFS_METADATA_ITEM_KEY;
>>>> -		}
>>>> -		ret = alloc_reserved_tree_block(trans, root_objectid,
>>>> -						generation, flags,
>>>> -						key, level, ins);
>>>> -		finish_current_insert(trans);
>>>> -		del_pending_extents(trans);
>>>> +		ret = set_extent_bits(&trans->fs_info->extent_ins,
>>>> +				      ins->objectid,
>>>> +				      ins->objectid + extent_size - 1,
>>>> +				      EXTENT_LOCKED);
>>>> +
>>>> +		BUG_ON(ret);
>>>>  	}
>>>> +
>>>> +	ret = btrfs_add_delayed_tree_ref(root->fs_info, trans, ins->objectid,
>>>> +					 extent_size, 0, root_objectid,
>>>> +					 level, BTRFS_ADD_DELAYED_EXTENT,
>>>> +					 extent_op, NULL, NULL);
>>>>  	return ret;
>>>>  }
>>>>  
>>>> @@ -3329,11 +3347,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
>>>>  				sizeof(cache->item));
>>>>  	BUG_ON(ret);
>>>>  
>>>> -	ret = finish_current_insert(trans);
>>>> -	BUG_ON(ret);
>>>> -	ret = del_pending_extents(trans);
>>>> -	BUG_ON(ret);
>>>> -
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -3429,10 +3442,6 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
>>>>  					sizeof(cache->item));
>>>>  		BUG_ON(ret);
>>>>  
>>>> -		finish_current_insert(trans);
>>>> -		ret = del_pending_extents(trans);
>>>> -		BUG_ON(ret);
>>>> -
>>>>  		cur_start = cache->key.objectid + cache->key.offset;
>>>>  	}
>>>>  	return 0;
>>>> @@ -3814,14 +3823,9 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans)
>>>>  	struct btrfs_fs_info *fs_info = trans->fs_info;
>>>>  	struct btrfs_root *root = fs_info->extent_root;
>>>>  
>>>> -	while(extent_root_pending_ops(fs_info)) {
>>>> -		ret = finish_current_insert(trans);
>>>> -		if (ret)
>>>> -			return ret;
>>>> -		ret = del_pending_extents(trans);
>>>> -		if (ret)
>>>> -			return ret;
>>>> -	}
>>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>>> +	if (ret)
>>>> +		return ret;
>>>>  
>>>>  	while(1) {
>>>>  		cache = btrfs_lookup_first_block_group(fs_info, start);
>>>> @@ -4026,7 +4030,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>>>>  		} else if (ret != -EEXIST) {
>>>>  			goto fail;
>>>>  		}
>>>> -		btrfs_extent_post_op(trans);
>>>> +		btrfs_run_delayed_refs(trans, -1);
>>>>  		extent_bytenr = disk_bytenr;
>>>>  		extent_num_bytes = num_bytes;
>>>>  		extent_offset = 0;
>>>> diff --git a/transaction.c b/transaction.c
>>>> index 96d9891b0d1c..bfda769210ee 100644
>>>> --- a/transaction.c
>>>> +++ b/transaction.c
>>>> @@ -61,7 +61,6 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
>>>>  	u64 old_root_bytenr;
>>>>  	struct btrfs_root *tree_root = root->fs_info->tree_root;
>>>>  
>>>> -	btrfs_write_dirty_block_groups(trans);
>>>>  	while(1) {
>>>>  		old_root_bytenr = btrfs_root_bytenr(&root->root_item);
>>>>  		if (old_root_bytenr == root->node->start)
>>>> @@ -98,6 +97,17 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
>>>>  	if (ret)
>>>>  		return ret;
>>>>  
>>>> +	/*
>>>> +	 * If the above CoW is the first one to dirty the current tree_root,
>>>> +	 * delayed refs for it won't be run until after this function has
>>>> +	 * finished executing, meaning we won't process the extent tree root,
>>>> +	 * which will have been added to ->dirty_cowonly_roots.  So run
>>>> +	 * delayed refs here as well.
>>>> +	 */
>>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>>  	while(!list_empty(&fs_info->dirty_cowonly_roots)) {
>>>>  		next = fs_info->dirty_cowonly_roots.next;
>>>>  		list_del_init(next);
>>>> @@ -147,6 +157,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>>  
>>>>  	if (trans->fs_info->transaction_aborted)
>>>>  		return -EROFS;
>>>> +	/*
>>>> +	 * Flush all accumulated delayed refs so that root-tree updates are
>>>> +	 * consistent
>>>> +	 */
>>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>>> +	BUG_ON(ret);
>>>>  
>>>>  	if (root->commit_root == root->node)
>>>>  		goto commit_tree;
>>>> @@ -164,11 +180,18 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>>  	ret = btrfs_update_root(trans, root->fs_info->tree_root,
>>>>  				&root->root_key, &root->root_item);
>>>>  	BUG_ON(ret);
>>>> +
>>>>  commit_tree:
>>>>  	ret = commit_tree_roots(trans, fs_info);
>>>>  	BUG_ON(ret);
>>>> -	ret = __commit_transaction(trans, root);
>>>> +	/*
>>>> +	 * Ensure that all comitted roots are properly accounted in the
>>>> +	 * extent tree
>>>> +	 */
>>>> +	ret = btrfs_run_delayed_refs(trans, -1);
>>>>  	BUG_ON(ret);
>>>> +	btrfs_write_dirty_block_groups(trans);
>>>> +	__commit_transaction(trans, root);
>>>>  	write_ctree_super(trans);
>>>>  	btrfs_finish_extent_commit(trans, fs_info->extent_root,
>>>>  			           &fs_info->pinned_extents);
>>>>
>>>
> 

Re: [PATCH 5/8] btrfs-progs: Wire up delayed refs

[Qu Wenruo]

On 2018/9/5 下午3:41, Nikolay Borisov wrote:
> 
> 
> On  5.09.2018 08:53, Qu Wenruo wrote:
>>
>>
>> On 2018/9/5 下午1:42, Nikolay Borisov wrote:
>>>
>>>
>>> On  5.09.2018 05:10, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2018/8/16 下午9:10, Nikolay Borisov wrote:
>>>>> This commit enables the delayed refs infrastructures. This entails doing
>>>>> the following:
>>>>>
>>>>> 1. Replacing existing calls of btrfs_extent_post_op (which is the
>>>>> equivalent of delayed refs) with the proper btrfs_run_delayed_refs.
>>>>> As well as eliminating open-coded calls to finish_current_insert and
>>>>> del_pending_extents which execute the delayed ops.
>>>>>
>>>>> 2. Wiring up the addition of delayed refs when freeing extents
>>>>> (btrfs_free_extent) and when adding new extents (alloc_tree_block).
>>>>>
>>>>> 3. Adding calls to btrfs_run_delayed refs in the transaction commit
>>>>> path alongside comments why every call is needed, since it's not always
>>>>> obvious (those call sites were derived empirically by running and
>>>>> debugging existing tests)
>>>>>
>>>>> 4. Correctly flagging the transaction in which we are reinitialising
>>>>> the extent tree.
>>>>>
>>>>> 5 Moving btrfs_write_dirty_block_groups to btrfs_write_dirty_block_groups
>>>>> since blockgroups should be written to disk after the last delayed refs
>>>>> have been run.
>>>>>
>>>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>>>> Signed-off-by: David Sterba <dsterba@suse.com>
>>>>
>>>> Is there something (maybe btrfs_run_delayed_refs()?) missing in btrfs-image?
>>>>
>>>> btrfs-image from devel branch can't restore image correctly, the block
>>>> group used bytes is not correct, thus it can't pass misc nor fsck tests.
>>>
>>> This is really strange, all fsck/misc tests passed with those patches.
>>> Can you be more specific which tests exactly you mean ?
>>
>> One case is fsck/020 with lowmem mode. (Original mode lacks block
>> group->used check).
>>
>> More specifically, fsck/020/keyed_data_ref_with_shared_leaf.img
>>
>> Using btrfs-image from my distribution (v4.17.1) and devel branch btrfs
>> check: (cwd is btrfs-progs, devel branch)
>>
>> $ btrfs-image -r
>> tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img
>> $ btrfs check --mode=wmem ~/test.img
>> Opening filesystem to check...
>> Checking filesystem on /home/adam/test.img
>> UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
>> [1/7] checking root items
>> [2/7] checking extents
>> [3/7] checking free space cache
>> [4/7] checking fs roots
>> [5/7] checking only csums items (without verifying data)
>> [6/7] checking root refs done with fs roots in lowmem mode, skipping
>> [7/7] checking quota groups skipped (not enabled on this FS)
>> found 1208320 bytes used, no error found
>> total csum bytes: 512
>> total tree bytes: 684032
>> total fs tree bytes: 638976
>> total extent tree bytes: 16384
>> btree space waste bytes: 305606
>> file data blocks allocated: 93847552
>>  referenced 1773568
>>
>> But if using btrfs-image with your delayed ref patch:
>> $ ./btrfs-image -r
>> tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img
>>
>> # No matter if I'm using btrfs-check from devel or 4.17.1
>> $ btrfs check --mode=wmem ~/test.img
>> Opening filesystem to check...
>> Checking filesystem on /home/adam/test.img
>> UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
>> [1/7] checking root items
>> [2/7] checking extents
>> ERROR: block group[4194304 8388608] used 20480 but extent items used 24576
>> ERROR: block group[20971520 16777216] used 659456 but extent items used
>> 655360
>> ERROR: errors found in extent allocation tree or chunk allocation
>> [3/7] checking free space cache
>> [4/7] checking fs roots
>> [5/7] checking only csums items (without verifying data)
>> [6/7] checking root refs done with fs roots in lowmem mode, skipping
>> [7/7] checking quota groups skipped (not enabled on this FS)
>> found 1208320 bytes used, error(s) found
>> total csum bytes: 512
>> total tree bytes: 684032
>> total fs tree bytes: 638976
>> total extent tree bytes: 16384
>> btree space waste bytes: 305606
>> file data blocks allocated: 93847552
>>  referenced 1773568
>>
>> I'd say, although lowmem check is still far from perfect, it indeed has
>> extra checks original mode lacks, and in this case it indeed exposes
>> problem.
> 
> 
> I'm not able to reproduce it: 
> 
> make TEST_ENABLE_OVERRIDE=ue TEST_ARGS_CHECK="--mode=lowmem"  test-fsck
>     [TEST]   fsck-tests.sh
>     [TEST/fsck]   001-bad-file-extent-bytenr
>     [TEST/fsck]   002-bad-transid
>     [TEST/fsck]   003-shift-offsets
>     [TEST/fsck]   004-no-dir-index
>     [TEST/fsck]   005-bad-item-offset
>     [TEST/fsck]   006-bad-root-items
>     [TEST/fsck]   007-bad-offset-snapshots
>     [TEST/fsck]   008-bad-dir-index-name
>     [TEST/fsck]   009-no-dir-item-or-index
>     [TEST/fsck]   010-no-rootdir-inode-item
>     [TEST/fsck]   011-no-inode-item
>     [TEST/fsck]   012-leaf-corruption
>     [TEST/fsck]   013-extent-tree-rebuild
>     [TEST/fsck]   014-no-extent-info
>     [TEST/fsck]   015-tree-reloc-tree
>     [TEST/fsck]   016-wrong-inode-nbytes
>     [TEST/fsck]   017-missing-all-file-extent
>     [TEST/fsck]   018-leaf-crossing-stripes
>     [TEST/fsck]   019-non-skinny-false-alert
>     [TEST/fsck]   020-extent-ref-cases
>     [TEST/fsck]   021-partially-dropped-snapshot-case
>     [TEST/fsck]   022-qgroup-rescan-halfway
>     [TEST/fsck]   023-qgroup-stack-overflow
>     [TEST/fsck]   024-clear-space-cache
>     [TEST/fsck]   025-file-extents
>     [TEST/fsck]   026-bad-dir-item-name
>     [TEST/fsck]   027-bad-extent-inline-ref-type
>     [TEST/fsck]   028-unaligned-super-dev-sizes
>     [TEST/fsck]   029-valid-orphan-item
>     [TEST/fsck]   030-reflinked-prealloc-extents
>     [TEST/fsck]   031-metadatadump-check-data-csum
>     [TEST/fsck]   032-corrupted-qgroup
>     [TEST/fsck]   032-freespacetree-corrupted-extent-offset
>     [TEST/fsck]   033-lowmem-collission-dir-items
>     [TEST/fsck]   034-bad-inode-flags
>     [TEST/fsck]   035-inline-bad-ram-bytes
>     [TEST/fsck]   035-rescan-not-kicked-in
> 
> 
> git bl -9 
> dac6a0a6f5ca btrfs-progs: Merge alloc_reserved_tree_block(2|) (3 weeks ago) <Nikolay Borisov>
> da184a3838e6 btrfs-progs: Remove __free_extent2 (3 weeks ago) <Nikolay Borisov>
> 76b75e40606e btrfs-progs: Remove old delayed refs infrastructure (3 months ago) <Nikolay Borisov>
> 870c58e08673 btrfs-progs: Wire up delayed refs (3 months ago) <Nikolay Borisov>
> e045b219e397 btrfs-progs: Make btrfs_write_dirty_block_groups take only trans argument (3 weeks ago) <Nikolay Borisov>
> 00f8d76c8a27 btrfs-progs: Add delayed refs infrastructure (3 months ago) <Nikolay Borisov>
> d03f79e8deb6 btrfs-progs: Add alloc_reserved_tree_block2 function (3 months ago) <Nikolay Borisov>
> d4a4831fffe9 btrfs-progs: Add __free_extent2 function (3 months ago) <Nikolay Borisov>
> 7faaca0d9f78 Btrfs progs v4.17.1 (4 weeks ago) <David Sterba>
> 
> Are you sure your branch is not dirty with other changes?

Oh, I'm using the (maybe out-of-data) David's devel branch.

For the "Wire up delayed refs" the commit hash is indeed different.
In that branch it indeed has some difference.

Is there any branch I could fetch from?

Thanks,
Qu

> 
> 
>>
>> Thanks,
>> Qu
>>
>>
>>>
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>> ---
>>>>>  check/main.c  |   3 +-
>>>>>  extent-tree.c | 166 ++++++++++++++++++++++++++++++----------------------------
>>>>>  transaction.c |  27 +++++++++-
>>>>>  3 files changed, 112 insertions(+), 84 deletions(-)
>>>>>
>>>>> diff --git a/check/main.c b/check/main.c
>>>>> index bc2ee22f7943..b361cd7e26a0 100644
>>>>> --- a/check/main.c
>>>>> +++ b/check/main.c
>>>>> @@ -8710,7 +8710,7 @@ static int reinit_extent_tree(struct btrfs_trans_handle *trans,
>>>>>  			fprintf(stderr, "Error adding block group\n");
>>>>>  			return ret;
>>>>>  		}
>>>>> -		btrfs_extent_post_op(trans);
>>>>> +		btrfs_run_delayed_refs(trans, -1);
>>>>>  	}
>>>>>  
>>>>>  	ret =eset_balance(trans, fs_info);
>>>>> @@ -9767,6 +9767,7 @@ int cmd_check(int argc, char **argv)
>>>>>  			goto close_out;
>>>>>  		}
>>>>>  
>>>>> +		trans->reinit_extent_tree =rue;
>>>>>  		if (init_extent_tree) {
>>>>>  			printf("Creating a new extent tree\n");
>>>>>  			ret =einit_extent_tree(trans, info,
>>>>> diff --git a/extent-tree.c b/extent-tree.c
>>>>> index 7d6c37c6b371..2fa51bbc0359 100644
>>>>> --- a/extent-tree.c
>>>>> +++ b/extent-tree.c
>>>>> @@ -1418,8 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>>>>>  		err =et;
>>>>>  out:
>>>>>  	btrfs_free_path(path);
>>>>> -	finish_current_insert(trans);
>>>>> -	del_pending_extents(trans);
>>>>>  	BUG_ON(err);
>>>>>  	return err;
>>>>>  }
>>>>> @@ -1602,8 +1600,6 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, u64 bytenr,
>>>>>  	btrfs_set_extent_flags(l, item, flags);
>>>>>  out:
>>>>>  	btrfs_free_path(path);
>>>>> -	finish_current_insert(trans);
>>>>> -	del_pending_extents(trans);
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> @@ -1701,7 +1697,6 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>>>  				 struct btrfs_block_group_cache *cache)
>>>>>  {
>>>>>  	int ret;
>>>>> -	int pending_ret;
>>>>>  	struct btrfs_root *extent_root =rans->fs_info->extent_root;
>>>>>  	unsigned long bi;
>>>>>  	struct extent_buffer *leaf;
>>>>> @@ -1717,12 +1712,8 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>>>  	btrfs_mark_buffer_dirty(leaf);
>>>>>  	btrfs_release_path(path);
>>>>>  fail:
>>>>> -	finish_current_insert(trans);
>>>>> -	pending_ret =el_pending_extents(trans);
>>>>>  	if (ret)
>>>>>  		return ret;
>>>>> -	if (pending_ret)
>>>>> -		return pending_ret;
>>>>>  	return 0;
>>>>>  
>>>>>  }
>>>>> @@ -2049,6 +2040,7 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>>>  	int skinny_metadata >>>>  		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
>>>>>  
>>>>> +
>>>>>  	while(1) {
>>>>>  		ret =ind_first_extent_bit(&info->extent_ins, 0, &start,
>>>>>  					    &end, EXTENT_LOCKED);
>>>>> @@ -2080,6 +2072,8 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>>>  			BUG_ON(1);
>>>>>  		}
>>>>>  
>>>>> +
>>>>> +		printf("shouldn't be executed\n");
>>>>>  		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
>>>>>  		kfree(extent_op);
>>>>>  	}
>>>>> @@ -2379,7 +2373,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
>>>>>  	}
>>>>>  fail:
>>>>>  	btrfs_free_path(path);
>>>>> -	finish_current_insert(trans);
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> @@ -2462,33 +2455,30 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>>>>>  		      u64 bytenr, u64 num_bytes, u64 parent,
>>>>>  		      u64 root_objectid, u64 owner, u64 offset)
>>>>>  {
>>>>> -	struct btrfs_root *extent_root =oot->fs_info->extent_root;
>>>>> -	int pending_ret;
>>>>>  	int ret;
>>>>>  
>>>>>  	WARN_ON(num_bytes < root->fs_info->sectorsize);
>>>>> -	if (root =extent_root) {
>>>>> -		struct pending_extent_op *extent_op;
>>>>> -
>>>>> -		extent_op =malloc(sizeof(*extent_op), GFP_NOFS);
>>>>> -		BUG_ON(!extent_op);
>>>>> -
>>>>> -		extent_op->type =ENDING_EXTENT_DELETE;
>>>>> -		extent_op->bytenr =ytenr;
>>>>> -		extent_op->num_bytes =um_bytes;
>>>>> -		extent_op->level =int)owner;
>>>>> -
>>>>> -		set_extent_bits(&root->fs_info->pending_del,
>>>>> -				bytenr, bytenr + num_bytes - 1,
>>>>> -				EXTENT_LOCKED);
>>>>> -		set_state_private(&root->fs_info->pending_del,
>>>>> -				  bytenr, (unsigned long)extent_op);
>>>>> -		return 0;
>>>>> +	/*
>>>>> +	 * tree log blocks never actually go into the extent allocation
>>>>> +	 * tree, just update pinning info and exit early.
>>>>> +	 */
>>>>> +	if (root_objectid =BTRFS_TREE_LOG_OBJECTID) {
>>>>> +		printf("PINNING EXTENTS IN LOG TREE\n");
>>>>> +		WARN_ON(owner >=TRFS_FIRST_FREE_OBJECTID);
>>>>> +		btrfs_pin_extent(trans->fs_info, bytenr, num_bytes);
>>>>> +		ret =;
>>>>> +	} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
>>>>> +		BUG_ON(offset);
>>>>> +		ret =trfs_add_delayed_tree_ref(trans->fs_info, trans,
>>>>> +						 bytenr, num_bytes, parent,
>>>>> +						 root_objectid, (int)owner,
>>>>> +						 BTRFS_DROP_DELAYED_REF,
>>>>> +						 NULL, NULL, NULL);
>>>>> +	} else {
>>>>> +		ret =_free_extent(trans, bytenr, num_bytes, parent,
>>>>> +				    root_objectid, owner, offset, 1);
>>>>>  	}
>>>>> -	ret =_free_extent(trans, bytenr, num_bytes, parent,
>>>>> -			    root_objectid, owner, offset, 1);
>>>>> -	pending_ret =el_pending_extents(trans);
>>>>> -	return ret ? ret : pending_ret;
>>>>> +	return ret;
>>>>>  }
>>>>>  
>>>>>  static u64 stripe_align(struct btrfs_root *root, u64 val)
>>>>> @@ -2694,6 +2684,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>>>  	struct btrfs_delayed_tree_ref *ref =trfs_delayed_node_to_tree_ref(node);
>>>>>  	struct btrfs_key ins;
>>>>>  	bool skinny_metadata =trfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
>>>>> +	int ret;
>>>>> +	u64 start, end;
>>>>>  
>>>>>  	ins.objectid =ode->bytenr;
>>>>>  	if (skinny_metadata) {
>>>>> @@ -2704,10 +2696,25 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>>>  		ins.type =TRFS_EXTENT_ITEM_KEY;
>>>>>  	}
>>>>>  
>>>>> -	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
>>>>> -					 extent_op->flags_to_set,
>>>>> -					 &extent_op->key, ref->level, &ins);
>>>>> +	if (ref->root =BTRFS_EXTENT_TREE_OBJECTID) {
>>>>> +		ret =ind_first_extent_bit(&trans->fs_info->extent_ins,
>>>>> +					    node->bytenr, &start, &end,
>>>>> +					    EXTENT_LOCKED);
>>>>> +		ASSERT(!ret);
>>>>> +		ASSERT(start =node->bytenr);
>>>>> +		ASSERT(end =node->bytenr + node->num_bytes - 1);
>>>>> +	}
>>>>> +
>>>>> +	ret =lloc_reserved_tree_block(trans, ref->root, trans->transid,
>>>>> +					extent_op->flags_to_set,
>>>>> +					&extent_op->key, ref->level, &ins);
>>>>>  
>>>>> +	if (ref->root =BTRFS_EXTENT_TREE_OBJECTID) {
>>>>> +		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
>>>>> +				  EXTENT_LOCKED);
>>>>> +	}
>>>>> +
>>>>> +	return ret;
>>>>>  }
>>>>>  
>>>>>  static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>>>>> @@ -2772,39 +2779,50 @@ static int alloc_tree_block(struct btrfs_trans_handle *trans,
>>>>>  			    u64 search_end, struct btrfs_key *ins)
>>>>>  {
>>>>>  	int ret;
>>>>> +	u64 extent_size;
>>>>> +	struct btrfs_delayed_extent_op *extent_op;
>>>>> +	bool skinny_metadata =trfs_fs_incompat(root->fs_info,
>>>>> +						 SKINNY_METADATA);
>>>>> +
>>>>> +	extent_op =trfs_alloc_delayed_extent_op();
>>>>> +	if (!extent_op)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>>  	ret =trfs_reserve_extent(trans, root, num_bytes, empty_size,
>>>>>  				   hint_byte, search_end, ins, 0);
>>>>>  	BUG_ON(ret);
>>>>>  
>>>>> +	if (key)
>>>>> +		memcpy(&extent_op->key, key, sizeof(extent_op->key));
>>>>> +	else
>>>>> +		memset(&extent_op->key, 0, sizeof(extent_op->key));
>>>>> +	extent_op->flags_to_set =lags;
>>>>> +	extent_op->update_key =kinny_metadata ? false : true;
>>>>> +	extent_op->update_flags =rue;
>>>>> +	extent_op->is_data =alse;
>>>>> +	extent_op->level =evel;
>>>>> +
>>>>> +	extent_size =ns->offset;
>>>>> +
>>>>> +	if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>>>> +		ins->offset =evel;
>>>>> +		ins->type =TRFS_METADATA_ITEM_KEY;
>>>>> +	}
>>>>> +
>>>>> +	/* Ensure this reserved extent is not found by the allocator */
>>>>>  	if (root_objectid =BTRFS_EXTENT_TREE_OBJECTID) {
>>>>> -		struct pending_extent_op *extent_op;
>>>>> -
>>>>> -		extent_op =malloc(sizeof(*extent_op), GFP_NOFS);
>>>>> -		BUG_ON(!extent_op);
>>>>> -
>>>>> -		extent_op->type =ENDING_EXTENT_INSERT;
>>>>> -		extent_op->bytenr =ns->objectid;
>>>>> -		extent_op->num_bytes =ns->offset;
>>>>> -		extent_op->level =evel;
>>>>> -		extent_op->flags =lags;
>>>>> -		memcpy(&extent_op->key, key, sizeof(*key));
>>>>> -
>>>>> -		set_extent_bits(&root->fs_info->extent_ins, ins->objectid,
>>>>> -				ins->objectid + ins->offset - 1,
>>>>> -				EXTENT_LOCKED);
>>>>> -		set_state_private(&root->fs_info->extent_ins,
>>>>> -				  ins->objectid, (unsigned long)extent_op);
>>>>> -	} else {
>>>>> -		if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>>>> -			ins->offset =evel;
>>>>> -			ins->type =TRFS_METADATA_ITEM_KEY;
>>>>> -		}
>>>>> -		ret =lloc_reserved_tree_block(trans, root_objectid,
>>>>> -						generation, flags,
>>>>> -						key, level, ins);
>>>>> -		finish_current_insert(trans);
>>>>> -		del_pending_extents(trans);
>>>>> +		ret =et_extent_bits(&trans->fs_info->extent_ins,
>>>>> +				      ins->objectid,
>>>>> +				      ins->objectid + extent_size - 1,
>>>>> +				      EXTENT_LOCKED);
>>>>> +
>>>>> +		BUG_ON(ret);
>>>>>  	}
>>>>> +
>>>>> +	ret =trfs_add_delayed_tree_ref(root->fs_info, trans, ins->objectid,
>>>>> +					 extent_size, 0, root_objectid,
>>>>> +					 level, BTRFS_ADD_DELAYED_EXTENT,
>>>>> +					 extent_op, NULL, NULL);
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> @@ -3329,11 +3347,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
>>>>>  				sizeof(cache->item));
>>>>>  	BUG_ON(ret);
>>>>>  
>>>>> -	ret =inish_current_insert(trans);
>>>>> -	BUG_ON(ret);
>>>>> -	ret =el_pending_extents(trans);
>>>>> -	BUG_ON(ret);
>>>>> -
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> @@ -3429,10 +3442,6 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
>>>>>  					sizeof(cache->item));
>>>>>  		BUG_ON(ret);
>>>>>  
>>>>> -		finish_current_insert(trans);
>>>>> -		ret =el_pending_extents(trans);
>>>>> -		BUG_ON(ret);
>>>>> -
>>>>>  		cur_start =ache->key.objectid + cache->key.offset;
>>>>>  	}
>>>>>  	return 0;
>>>>> @@ -3814,14 +3823,9 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans)
>>>>>  	struct btrfs_fs_info *fs_info =rans->fs_info;
>>>>>  	struct btrfs_root *root =s_info->extent_root;
>>>>>  
>>>>> -	while(extent_root_pending_ops(fs_info)) {
>>>>> -		ret =inish_current_insert(trans);
>>>>> -		if (ret)
>>>>> -			return ret;
>>>>> -		ret =el_pending_extents(trans);
>>>>> -		if (ret)
>>>>> -			return ret;
>>>>> -	}
>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>> +	if (ret)
>>>>> +		return ret;
>>>>>  
>>>>>  	while(1) {
>>>>>  		cache =trfs_lookup_first_block_group(fs_info, start);
>>>>> @@ -4026,7 +4030,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>>>>>  		} else if (ret !=EEXIST) {
>>>>>  			goto fail;
>>>>>  		}
>>>>> -		btrfs_extent_post_op(trans);
>>>>> +		btrfs_run_delayed_refs(trans, -1);
>>>>>  		extent_bytenr =isk_bytenr;
>>>>>  		extent_num_bytes =um_bytes;
>>>>>  		extent_offset =;
>>>>> diff --git a/transaction.c b/transaction.c
>>>>> index 96d9891b0d1c..bfda769210ee 100644
>>>>> --- a/transaction.c
>>>>> +++ b/transaction.c
>>>>> @@ -61,7 +61,6 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
>>>>>  	u64 old_root_bytenr;
>>>>>  	struct btrfs_root *tree_root =oot->fs_info->tree_root;
>>>>>  
>>>>> -	btrfs_write_dirty_block_groups(trans);
>>>>>  	while(1) {
>>>>>  		old_root_bytenr =trfs_root_bytenr(&root->root_item);
>>>>>  		if (old_root_bytenr =root->node->start)
>>>>> @@ -98,6 +97,17 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
>>>>>  	if (ret)
>>>>>  		return ret;
>>>>>  
>>>>> +	/*
>>>>> +	 * If the above CoW is the first one to dirty the current tree_root,
>>>>> +	 * delayed refs for it won't be run until after this function has
>>>>> +	 * finished executing, meaning we won't process the extent tree root,
>>>>> +	 * which will have been added to ->dirty_cowonly_roots.  So run
>>>>> +	 * delayed refs here as well.
>>>>> +	 */
>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>> +	if (ret)
>>>>> +		return ret;
>>>>> +
>>>>>  	while(!list_empty(&fs_info->dirty_cowonly_roots)) {
>>>>>  		next =s_info->dirty_cowonly_roots.next;
>>>>>  		list_del_init(next);
>>>>> @@ -147,6 +157,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>>>  
>>>>>  	if (trans->fs_info->transaction_aborted)
>>>>>  		return -EROFS;
>>>>> +	/*
>>>>> +	 * Flush all accumulated delayed refs so that root-tree updates are
>>>>> +	 * consistent
>>>>> +	 */
>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>> +	BUG_ON(ret);
>>>>>  
>>>>>  	if (root->commit_root =root->node)
>>>>>  		goto commit_tree;
>>>>> @@ -164,11 +180,18 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>>>  	ret =trfs_update_root(trans, root->fs_info->tree_root,
>>>>>  				&root->root_key, &root->root_item);
>>>>>  	BUG_ON(ret);
>>>>> +
>>>>>  commit_tree:
>>>>>  	ret =ommit_tree_roots(trans, fs_info);
>>>>>  	BUG_ON(ret);
>>>>> -	ret =_commit_transaction(trans, root);
>>>>> +	/*
>>>>> +	 * Ensure that all comitted roots are properly accounted in the
>>>>> +	 * extent tree
>>>>> +	 */
>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>>  	BUG_ON(ret);
>>>>> +	btrfs_write_dirty_block_groups(trans);
>>>>> +	__commit_transaction(trans, root);
>>>>>  	write_ctree_super(trans);
>>>>>  	btrfs_finish_extent_commit(trans, fs_info->extent_root,
>>>>>  			           &fs_info->pinned_extents);
>>>>>
>>>>
>>

Re: [PATCH 5/8] btrfs-progs: Wire up delayed refs

[Nikolay Borisov]

On  5.09.2018 10:46, Qu Wenruo wrote:
> 
> 
> On 2018/9/5 下午3:41, Nikolay Borisov wrote:
>>
>>
>> On  5.09.2018 08:53, Qu Wenruo wrote:
>>>
>>>
>>> On 2018/9/5 下午1:42, Nikolay Borisov wrote:
>>>>
>>>>
>>>> On  5.09.2018 05:10, Qu Wenruo wrote:
>>>>>
>>>>>
>>>>> On 2018/8/16 下午9:10, Nikolay Borisov wrote:
>>>>>> This commit enables the delayed refs infrastructures. This entails doing
>>>>>> the following:
>>>>>>
>>>>>> 1. Replacing existing calls of btrfs_extent_post_op (which is the
>>>>>> equivalent of delayed refs) with the proper btrfs_run_delayed_refs.
>>>>>> As well as eliminating open-coded calls to finish_current_insert and
>>>>>> del_pending_extents which execute the delayed ops.
>>>>>>
>>>>>> 2. Wiring up the addition of delayed refs when freeing extents
>>>>>> (btrfs_free_extent) and when adding new extents (alloc_tree_block).
>>>>>>
>>>>>> 3. Adding calls to btrfs_run_delayed refs in the transaction commit
>>>>>> path alongside comments why every call is needed, since it's not always
>>>>>> obvious (those call sites were derived empirically by running and
>>>>>> debugging existing tests)
>>>>>>
>>>>>> 4. Correctly flagging the transaction in which we are reinitialising
>>>>>> the extent tree.
>>>>>>
>>>>>> 5 Moving btrfs_write_dirty_block_groups to btrfs_write_dirty_block_groups
>>>>>> since blockgroups should be written to disk after the last delayed refs
>>>>>> have been run.
>>>>>>
>>>>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>>>>> Signed-off-by: David Sterba <dsterba@suse.com>
>>>>>
>>>>> Is there something (maybe btrfs_run_delayed_refs()?) missing in btrfs-image?
>>>>>
>>>>> btrfs-image from devel branch can't restore image correctly, the block
>>>>> group used bytes is not correct, thus it can't pass misc nor fsck tests.
>>>>
>>>> This is really strange, all fsck/misc tests passed with those patches.
>>>> Can you be more specific which tests exactly you mean ?
>>>
>>> One case is fsck/020 with lowmem mode. (Original mode lacks block
>>> group->used check).
>>>
>>> More specifically, fsck/020/keyed_data_ref_with_shared_leaf.img
>>>
>>> Using btrfs-image from my distribution (v4.17.1) and devel branch btrfs
>>> check: (cwd is btrfs-progs, devel branch)
>>>
>>> $ btrfs-image -r
>>> tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img
>>> $ btrfs check --mode=wmem ~/test.img
>>> Opening filesystem to check...
>>> Checking filesystem on /home/adam/test.img
>>> UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
>>> [1/7] checking root items
>>> [2/7] checking extents
>>> [3/7] checking free space cache
>>> [4/7] checking fs roots
>>> [5/7] checking only csums items (without verifying data)
>>> [6/7] checking root refs done with fs roots in lowmem mode, skipping
>>> [7/7] checking quota groups skipped (not enabled on this FS)
>>> found 1208320 bytes used, no error found
>>> total csum bytes: 512
>>> total tree bytes: 684032
>>> total fs tree bytes: 638976
>>> total extent tree bytes: 16384
>>> btree space waste bytes: 305606
>>> file data blocks allocated: 93847552
>>>  referenced 1773568
>>>
>>> But if using btrfs-image with your delayed ref patch:
>>> $ ./btrfs-image -r
>>> tests/fsck-tests/020-extent-ref-cases/keyed_data_ref_with_shared_leaf.img ~/test.img
>>>
>>> # No matter if I'm using btrfs-check from devel or 4.17.1
>>> $ btrfs check --mode=wmem ~/test.img
>>> Opening filesystem to check...
>>> Checking filesystem on /home/adam/test.img
>>> UUID: 12dabcf2-d4da-4a70-9701-9f3d48074e73
>>> [1/7] checking root items
>>> [2/7] checking extents
>>> ERROR: block group[4194304 8388608] used 20480 but extent items used 24576
>>> ERROR: block group[20971520 16777216] used 659456 but extent items used
>>> 655360
>>> ERROR: errors found in extent allocation tree or chunk allocation
>>> [3/7] checking free space cache
>>> [4/7] checking fs roots
>>> [5/7] checking only csums items (without verifying data)
>>> [6/7] checking root refs done with fs roots in lowmem mode, skipping
>>> [7/7] checking quota groups skipped (not enabled on this FS)
>>> found 1208320 bytes used, error(s) found
>>> total csum bytes: 512
>>> total tree bytes: 684032
>>> total fs tree bytes: 638976
>>> total extent tree bytes: 16384
>>> btree space waste bytes: 305606
>>> file data blocks allocated: 93847552
>>>  referenced 1773568
>>>
>>> I'd say, although lowmem check is still far from perfect, it indeed has
>>> extra checks original mode lacks, and in this case it indeed exposes
>>> problem.
>>
>>
>> I'm not able to reproduce it: 
>>
>> make TEST_ENABLE_OVERRIDE=ue TEST_ARGS_CHECK="--mode=lowmem"  test-fsck
>>     [TEST]   fsck-tests.sh
>>     [TEST/fsck]   001-bad-file-extent-bytenr
>>     [TEST/fsck]   002-bad-transid
>>     [TEST/fsck]   003-shift-offsets
>>     [TEST/fsck]   004-no-dir-index
>>     [TEST/fsck]   005-bad-item-offset
>>     [TEST/fsck]   006-bad-root-items
>>     [TEST/fsck]   007-bad-offset-snapshots
>>     [TEST/fsck]   008-bad-dir-index-name
>>     [TEST/fsck]   009-no-dir-item-or-index
>>     [TEST/fsck]   010-no-rootdir-inode-item
>>     [TEST/fsck]   011-no-inode-item
>>     [TEST/fsck]   012-leaf-corruption
>>     [TEST/fsck]   013-extent-tree-rebuild
>>     [TEST/fsck]   014-no-extent-info
>>     [TEST/fsck]   015-tree-reloc-tree
>>     [TEST/fsck]   016-wrong-inode-nbytes
>>     [TEST/fsck]   017-missing-all-file-extent
>>     [TEST/fsck]   018-leaf-crossing-stripes
>>     [TEST/fsck]   019-non-skinny-false-alert
>>     [TEST/fsck]   020-extent-ref-cases
>>     [TEST/fsck]   021-partially-dropped-snapshot-case
>>     [TEST/fsck]   022-qgroup-rescan-halfway
>>     [TEST/fsck]   023-qgroup-stack-overflow
>>     [TEST/fsck]   024-clear-space-cache
>>     [TEST/fsck]   025-file-extents
>>     [TEST/fsck]   026-bad-dir-item-name
>>     [TEST/fsck]   027-bad-extent-inline-ref-type
>>     [TEST/fsck]   028-unaligned-super-dev-sizes
>>     [TEST/fsck]   029-valid-orphan-item
>>     [TEST/fsck]   030-reflinked-prealloc-extents
>>     [TEST/fsck]   031-metadatadump-check-data-csum
>>     [TEST/fsck]   032-corrupted-qgroup
>>     [TEST/fsck]   032-freespacetree-corrupted-extent-offset
>>     [TEST/fsck]   033-lowmem-collission-dir-items
>>     [TEST/fsck]   034-bad-inode-flags
>>     [TEST/fsck]   035-inline-bad-ram-bytes
>>     [TEST/fsck]   035-rescan-not-kicked-in
>>
>>
>> git bl -9 
>> dac6a0a6f5ca btrfs-progs: Merge alloc_reserved_tree_block(2|) (3 weeks ago) <Nikolay Borisov>
>> da184a3838e6 btrfs-progs: Remove __free_extent2 (3 weeks ago) <Nikolay Borisov>
>> 76b75e40606e btrfs-progs: Remove old delayed refs infrastructure (3 months ago) <Nikolay Borisov>
>> 870c58e08673 btrfs-progs: Wire up delayed refs (3 months ago) <Nikolay Borisov>
>> e045b219e397 btrfs-progs: Make btrfs_write_dirty_block_groups take only trans argument (3 weeks ago) <Nikolay Borisov>
>> 00f8d76c8a27 btrfs-progs: Add delayed refs infrastructure (3 months ago) <Nikolay Borisov>
>> d03f79e8deb6 btrfs-progs: Add alloc_reserved_tree_block2 function (3 months ago) <Nikolay Borisov>
>> d4a4831fffe9 btrfs-progs: Add __free_extent2 function (3 months ago) <Nikolay Borisov>
>> 7faaca0d9f78 Btrfs progs v4.17.1 (4 weeks ago) <David Sterba>
>>
>> Are you sure your branch is not dirty with other changes?
> 
> Oh, I'm using the (maybe out-of-data) David's devel branch.
> 
> For the "Wire up delayed refs" the commit hash is indeed different.
> In that branch it indeed has some difference.
> 
> Is there any branch I could fetch from?


I've just pushed a delayed-refs-v2 branch to :
https://github.com/lorddoskias/btrfs-progs/tree/delayed-refs-v2

> 
> Thanks,
> Qu
> 
>>
>>
>>>
>>> Thanks,
>>> Qu
>>>
>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> Qu
>>>>>
>>>>>> ---
>>>>>>  check/main.c  |   3 +-
>>>>>>  extent-tree.c | 166 ++++++++++++++++++++++++++++++----------------------------
>>>>>>  transaction.c |  27 +++++++++-
>>>>>>  3 files changed, 112 insertions(+), 84 deletions(-)
>>>>>>
>>>>>> diff --git a/check/main.c b/check/main.c
>>>>>> index bc2ee22f7943..b361cd7e26a0 100644
>>>>>> --- a/check/main.c
>>>>>> +++ b/check/main.c
>>>>>> @@ -8710,7 +8710,7 @@ static int reinit_extent_tree(struct btrfs_trans_handle *trans,
>>>>>>  			fprintf(stderr, "Error adding block group\n");
>>>>>>  			return ret;
>>>>>>  		}
>>>>>> -		btrfs_extent_post_op(trans);
>>>>>> +		btrfs_run_delayed_refs(trans, -1);
>>>>>>  	}
>>>>>>  
>>>>>>  	ret =eset_balance(trans, fs_info);
>>>>>> @@ -9767,6 +9767,7 @@ int cmd_check(int argc, char **argv)
>>>>>>  			goto close_out;
>>>>>>  		}
>>>>>>  
>>>>>> +		trans->reinit_extent_tree =rue;
>>>>>>  		if (init_extent_tree) {
>>>>>>  			printf("Creating a new extent tree\n");
>>>>>>  			ret =einit_extent_tree(trans, info,
>>>>>> diff --git a/extent-tree.c b/extent-tree.c
>>>>>> index 7d6c37c6b371..2fa51bbc0359 100644
>>>>>> --- a/extent-tree.c
>>>>>> +++ b/extent-tree.c
>>>>>> @@ -1418,8 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>>>>>>  		err =et;
>>>>>>  out:
>>>>>>  	btrfs_free_path(path);
>>>>>> -	finish_current_insert(trans);
>>>>>> -	del_pending_extents(trans);
>>>>>>  	BUG_ON(err);
>>>>>>  	return err;
>>>>>>  }
>>>>>> @@ -1602,8 +1600,6 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, u64 bytenr,
>>>>>>  	btrfs_set_extent_flags(l, item, flags);
>>>>>>  out:
>>>>>>  	btrfs_free_path(path);
>>>>>> -	finish_current_insert(trans);
>>>>>> -	del_pending_extents(trans);
>>>>>>  	return ret;
>>>>>>  }
>>>>>>  
>>>>>> @@ -1701,7 +1697,6 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>>>>  				 struct btrfs_block_group_cache *cache)
>>>>>>  {
>>>>>>  	int ret;
>>>>>> -	int pending_ret;
>>>>>>  	struct btrfs_root *extent_root =rans->fs_info->extent_root;
>>>>>>  	unsigned long bi;
>>>>>>  	struct extent_buffer *leaf;
>>>>>> @@ -1717,12 +1712,8 @@ static int write_one_cache_group(struct btrfs_trans_handle *trans,
>>>>>>  	btrfs_mark_buffer_dirty(leaf);
>>>>>>  	btrfs_release_path(path);
>>>>>>  fail:
>>>>>> -	finish_current_insert(trans);
>>>>>> -	pending_ret =el_pending_extents(trans);
>>>>>>  	if (ret)
>>>>>>  		return ret;
>>>>>> -	if (pending_ret)
>>>>>> -		return pending_ret;
>>>>>>  	return 0;
>>>>>>  
>>>>>>  }
>>>>>> @@ -2049,6 +2040,7 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>>>>  	int skinny_metadata >>>>  		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
>>>>>>  
>>>>>> +
>>>>>>  	while(1) {
>>>>>>  		ret =ind_first_extent_bit(&info->extent_ins, 0, &start,
>>>>>>  					    &end, EXTENT_LOCKED);
>>>>>> @@ -2080,6 +2072,8 @@ static int finish_current_insert(struct btrfs_trans_handle *trans)
>>>>>>  			BUG_ON(1);
>>>>>>  		}
>>>>>>  
>>>>>> +
>>>>>> +		printf("shouldn't be executed\n");
>>>>>>  		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
>>>>>>  		kfree(extent_op);
>>>>>>  	}
>>>>>> @@ -2379,7 +2373,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
>>>>>>  	}
>>>>>>  fail:
>>>>>>  	btrfs_free_path(path);
>>>>>> -	finish_current_insert(trans);
>>>>>>  	return ret;
>>>>>>  }
>>>>>>  
>>>>>> @@ -2462,33 +2455,30 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>>>>>>  		      u64 bytenr, u64 num_bytes, u64 parent,
>>>>>>  		      u64 root_objectid, u64 owner, u64 offset)
>>>>>>  {
>>>>>> -	struct btrfs_root *extent_root =oot->fs_info->extent_root;
>>>>>> -	int pending_ret;
>>>>>>  	int ret;
>>>>>>  
>>>>>>  	WARN_ON(num_bytes < root->fs_info->sectorsize);
>>>>>> -	if (root =extent_root) {
>>>>>> -		struct pending_extent_op *extent_op;
>>>>>> -
>>>>>> -		extent_op =malloc(sizeof(*extent_op), GFP_NOFS);
>>>>>> -		BUG_ON(!extent_op);
>>>>>> -
>>>>>> -		extent_op->type =ENDING_EXTENT_DELETE;
>>>>>> -		extent_op->bytenr =ytenr;
>>>>>> -		extent_op->num_bytes =um_bytes;
>>>>>> -		extent_op->level =int)owner;
>>>>>> -
>>>>>> -		set_extent_bits(&root->fs_info->pending_del,
>>>>>> -				bytenr, bytenr + num_bytes - 1,
>>>>>> -				EXTENT_LOCKED);
>>>>>> -		set_state_private(&root->fs_info->pending_del,
>>>>>> -				  bytenr, (unsigned long)extent_op);
>>>>>> -		return 0;
>>>>>> +	/*
>>>>>> +	 * tree log blocks never actually go into the extent allocation
>>>>>> +	 * tree, just update pinning info and exit early.
>>>>>> +	 */
>>>>>> +	if (root_objectid =BTRFS_TREE_LOG_OBJECTID) {
>>>>>> +		printf("PINNING EXTENTS IN LOG TREE\n");
>>>>>> +		WARN_ON(owner >=TRFS_FIRST_FREE_OBJECTID);
>>>>>> +		btrfs_pin_extent(trans->fs_info, bytenr, num_bytes);
>>>>>> +		ret =;
>>>>>> +	} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
>>>>>> +		BUG_ON(offset);
>>>>>> +		ret =trfs_add_delayed_tree_ref(trans->fs_info, trans,
>>>>>> +						 bytenr, num_bytes, parent,
>>>>>> +						 root_objectid, (int)owner,
>>>>>> +						 BTRFS_DROP_DELAYED_REF,
>>>>>> +						 NULL, NULL, NULL);
>>>>>> +	} else {
>>>>>> +		ret =_free_extent(trans, bytenr, num_bytes, parent,
>>>>>> +				    root_objectid, owner, offset, 1);
>>>>>>  	}
>>>>>> -	ret =_free_extent(trans, bytenr, num_bytes, parent,
>>>>>> -			    root_objectid, owner, offset, 1);
>>>>>> -	pending_ret =el_pending_extents(trans);
>>>>>> -	return ret ? ret : pending_ret;
>>>>>> +	return ret;
>>>>>>  }
>>>>>>  
>>>>>>  static u64 stripe_align(struct btrfs_root *root, u64 val)
>>>>>> @@ -2694,6 +2684,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>>>>  	struct btrfs_delayed_tree_ref *ref =trfs_delayed_node_to_tree_ref(node);
>>>>>>  	struct btrfs_key ins;
>>>>>>  	bool skinny_metadata =trfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
>>>>>> +	int ret;
>>>>>> +	u64 start, end;
>>>>>>  
>>>>>>  	ins.objectid =ode->bytenr;
>>>>>>  	if (skinny_metadata) {
>>>>>> @@ -2704,10 +2696,25 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
>>>>>>  		ins.type =TRFS_EXTENT_ITEM_KEY;
>>>>>>  	}
>>>>>>  
>>>>>> -	return alloc_reserved_tree_block(trans, ref->root, trans->transid,
>>>>>> -					 extent_op->flags_to_set,
>>>>>> -					 &extent_op->key, ref->level, &ins);
>>>>>> +	if (ref->root =BTRFS_EXTENT_TREE_OBJECTID) {
>>>>>> +		ret =ind_first_extent_bit(&trans->fs_info->extent_ins,
>>>>>> +					    node->bytenr, &start, &end,
>>>>>> +					    EXTENT_LOCKED);
>>>>>> +		ASSERT(!ret);
>>>>>> +		ASSERT(start =node->bytenr);
>>>>>> +		ASSERT(end =node->bytenr + node->num_bytes - 1);
>>>>>> +	}
>>>>>> +
>>>>>> +	ret =lloc_reserved_tree_block(trans, ref->root, trans->transid,
>>>>>> +					extent_op->flags_to_set,
>>>>>> +					&extent_op->key, ref->level, &ins);
>>>>>>  
>>>>>> +	if (ref->root =BTRFS_EXTENT_TREE_OBJECTID) {
>>>>>> +		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
>>>>>> +				  EXTENT_LOCKED);
>>>>>> +	}
>>>>>> +
>>>>>> +	return ret;
>>>>>>  }
>>>>>>  
>>>>>>  static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>>>>>> @@ -2772,39 +2779,50 @@ static int alloc_tree_block(struct btrfs_trans_handle *trans,
>>>>>>  			    u64 search_end, struct btrfs_key *ins)
>>>>>>  {
>>>>>>  	int ret;
>>>>>> +	u64 extent_size;
>>>>>> +	struct btrfs_delayed_extent_op *extent_op;
>>>>>> +	bool skinny_metadata =trfs_fs_incompat(root->fs_info,
>>>>>> +						 SKINNY_METADATA);
>>>>>> +
>>>>>> +	extent_op =trfs_alloc_delayed_extent_op();
>>>>>> +	if (!extent_op)
>>>>>> +		return -ENOMEM;
>>>>>> +
>>>>>>  	ret =trfs_reserve_extent(trans, root, num_bytes, empty_size,
>>>>>>  				   hint_byte, search_end, ins, 0);
>>>>>>  	BUG_ON(ret);
>>>>>>  
>>>>>> +	if (key)
>>>>>> +		memcpy(&extent_op->key, key, sizeof(extent_op->key));
>>>>>> +	else
>>>>>> +		memset(&extent_op->key, 0, sizeof(extent_op->key));
>>>>>> +	extent_op->flags_to_set =lags;
>>>>>> +	extent_op->update_key =kinny_metadata ? false : true;
>>>>>> +	extent_op->update_flags =rue;
>>>>>> +	extent_op->is_data =alse;
>>>>>> +	extent_op->level =evel;
>>>>>> +
>>>>>> +	extent_size =ns->offset;
>>>>>> +
>>>>>> +	if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>>>>> +		ins->offset =evel;
>>>>>> +		ins->type =TRFS_METADATA_ITEM_KEY;
>>>>>> +	}
>>>>>> +
>>>>>> +	/* Ensure this reserved extent is not found by the allocator */
>>>>>>  	if (root_objectid =BTRFS_EXTENT_TREE_OBJECTID) {
>>>>>> -		struct pending_extent_op *extent_op;
>>>>>> -
>>>>>> -		extent_op =malloc(sizeof(*extent_op), GFP_NOFS);
>>>>>> -		BUG_ON(!extent_op);
>>>>>> -
>>>>>> -		extent_op->type =ENDING_EXTENT_INSERT;
>>>>>> -		extent_op->bytenr =ns->objectid;
>>>>>> -		extent_op->num_bytes =ns->offset;
>>>>>> -		extent_op->level =evel;
>>>>>> -		extent_op->flags =lags;
>>>>>> -		memcpy(&extent_op->key, key, sizeof(*key));
>>>>>> -
>>>>>> -		set_extent_bits(&root->fs_info->extent_ins, ins->objectid,
>>>>>> -				ins->objectid + ins->offset - 1,
>>>>>> -				EXTENT_LOCKED);
>>>>>> -		set_state_private(&root->fs_info->extent_ins,
>>>>>> -				  ins->objectid, (unsigned long)extent_op);
>>>>>> -	} else {
>>>>>> -		if (btrfs_fs_incompat(root->fs_info, SKINNY_METADATA)) {
>>>>>> -			ins->offset =evel;
>>>>>> -			ins->type =TRFS_METADATA_ITEM_KEY;
>>>>>> -		}
>>>>>> -		ret =lloc_reserved_tree_block(trans, root_objectid,
>>>>>> -						generation, flags,
>>>>>> -						key, level, ins);
>>>>>> -		finish_current_insert(trans);
>>>>>> -		del_pending_extents(trans);
>>>>>> +		ret =et_extent_bits(&trans->fs_info->extent_ins,
>>>>>> +				      ins->objectid,
>>>>>> +				      ins->objectid + extent_size - 1,
>>>>>> +				      EXTENT_LOCKED);
>>>>>> +
>>>>>> +		BUG_ON(ret);
>>>>>>  	}
>>>>>> +
>>>>>> +	ret =trfs_add_delayed_tree_ref(root->fs_info, trans, ins->objectid,
>>>>>> +					 extent_size, 0, root_objectid,
>>>>>> +					 level, BTRFS_ADD_DELAYED_EXTENT,
>>>>>> +					 extent_op, NULL, NULL);
>>>>>>  	return ret;
>>>>>>  }
>>>>>>  
>>>>>> @@ -3329,11 +3347,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
>>>>>>  				sizeof(cache->item));
>>>>>>  	BUG_ON(ret);
>>>>>>  
>>>>>> -	ret =inish_current_insert(trans);
>>>>>> -	BUG_ON(ret);
>>>>>> -	ret =el_pending_extents(trans);
>>>>>> -	BUG_ON(ret);
>>>>>> -
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> @@ -3429,10 +3442,6 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
>>>>>>  					sizeof(cache->item));
>>>>>>  		BUG_ON(ret);
>>>>>>  
>>>>>> -		finish_current_insert(trans);
>>>>>> -		ret =el_pending_extents(trans);
>>>>>> -		BUG_ON(ret);
>>>>>> -
>>>>>>  		cur_start =ache->key.objectid + cache->key.offset;
>>>>>>  	}
>>>>>>  	return 0;
>>>>>> @@ -3814,14 +3823,9 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans)
>>>>>>  	struct btrfs_fs_info *fs_info =rans->fs_info;
>>>>>>  	struct btrfs_root *root =s_info->extent_root;
>>>>>>  
>>>>>> -	while(extent_root_pending_ops(fs_info)) {
>>>>>> -		ret =inish_current_insert(trans);
>>>>>> -		if (ret)
>>>>>> -			return ret;
>>>>>> -		ret =el_pending_extents(trans);
>>>>>> -		if (ret)
>>>>>> -			return ret;
>>>>>> -	}
>>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>>> +	if (ret)
>>>>>> +		return ret;
>>>>>>  
>>>>>>  	while(1) {
>>>>>>  		cache =trfs_lookup_first_block_group(fs_info, start);
>>>>>> @@ -4026,7 +4030,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>>>>>>  		} else if (ret !=EEXIST) {
>>>>>>  			goto fail;
>>>>>>  		}
>>>>>> -		btrfs_extent_post_op(trans);
>>>>>> +		btrfs_run_delayed_refs(trans, -1);
>>>>>>  		extent_bytenr =isk_bytenr;
>>>>>>  		extent_num_bytes =um_bytes;
>>>>>>  		extent_offset =;
>>>>>> diff --git a/transaction.c b/transaction.c
>>>>>> index 96d9891b0d1c..bfda769210ee 100644
>>>>>> --- a/transaction.c
>>>>>> +++ b/transaction.c
>>>>>> @@ -61,7 +61,6 @@ static int update_cowonly_root(struct btrfs_trans_handle *trans,
>>>>>>  	u64 old_root_bytenr;
>>>>>>  	struct btrfs_root *tree_root =oot->fs_info->tree_root;
>>>>>>  
>>>>>> -	btrfs_write_dirty_block_groups(trans);
>>>>>>  	while(1) {
>>>>>>  		old_root_bytenr =trfs_root_bytenr(&root->root_item);
>>>>>>  		if (old_root_bytenr =root->node->start)
>>>>>> @@ -98,6 +97,17 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
>>>>>>  	if (ret)
>>>>>>  		return ret;
>>>>>>  
>>>>>> +	/*
>>>>>> +	 * If the above CoW is the first one to dirty the current tree_root,
>>>>>> +	 * delayed refs for it won't be run until after this function has
>>>>>> +	 * finished executing, meaning we won't process the extent tree root,
>>>>>> +	 * which will have been added to ->dirty_cowonly_roots.  So run
>>>>>> +	 * delayed refs here as well.
>>>>>> +	 */
>>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>>> +	if (ret)
>>>>>> +		return ret;
>>>>>> +
>>>>>>  	while(!list_empty(&fs_info->dirty_cowonly_roots)) {
>>>>>>  		next =s_info->dirty_cowonly_roots.next;
>>>>>>  		list_del_init(next);
>>>>>> @@ -147,6 +157,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>>>>  
>>>>>>  	if (trans->fs_info->transaction_aborted)
>>>>>>  		return -EROFS;
>>>>>> +	/*
>>>>>> +	 * Flush all accumulated delayed refs so that root-tree updates are
>>>>>> +	 * consistent
>>>>>> +	 */
>>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>>> +	BUG_ON(ret);
>>>>>>  
>>>>>>  	if (root->commit_root =root->node)
>>>>>>  		goto commit_tree;
>>>>>> @@ -164,11 +180,18 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>>>>>>  	ret =trfs_update_root(trans, root->fs_info->tree_root,
>>>>>>  				&root->root_key, &root->root_item);
>>>>>>  	BUG_ON(ret);
>>>>>> +
>>>>>>  commit_tree:
>>>>>>  	ret =ommit_tree_roots(trans, fs_info);
>>>>>>  	BUG_ON(ret);
>>>>>> -	ret =_commit_transaction(trans, root);
>>>>>> +	/*
>>>>>> +	 * Ensure that all comitted roots are properly accounted in the
>>>>>> +	 * extent tree
>>>>>> +	 */
>>>>>> +	ret =trfs_run_delayed_refs(trans, -1);
>>>>>>  	BUG_ON(ret);
>>>>>> +	btrfs_write_dirty_block_groups(trans);
>>>>>> +	__commit_transaction(trans, root);
>>>>>>  	write_ctree_super(trans);
>>>>>>  	btrfs_finish_extent_commit(trans, fs_info->extent_root,
>>>>>>  			           &fs_info->pinned_extents);
>>>>>>
>>>>>
>>>
> 

[PATCH 6/8] btrfs-progs: Remove old delayed refs infrastructure

[Nikolay Borisov]

Given that the new delayed refs infrastructure is implemented and
wired up, there is no point in keeping the old code. So just remove it.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 ctree.h       |   2 -
 disk-io.c     |   2 -
 extent-tree.c | 137 ----------------------------------------------------------
 3 files changed, 141 deletions(-)

diff --git a/ctree.h b/ctree.h
index 75675ef3f781..49f0f5181512 100644
--- a/ctree.h
+++ b/ctree.h
@@ -1098,7 +1098,6 @@ struct btrfs_fs_info {
 	struct extent_io_tree free_space_cache;
 	struct extent_io_tree block_group_cache;
 	struct extent_io_tree pinned_extents;
-	struct extent_io_tree pending_del;
 	struct extent_io_tree extent_ins;
 	struct extent_io_tree *excluded_extents;
 
@@ -2481,7 +2480,6 @@ int btrfs_fix_block_accounting(struct btrfs_trans_handle *trans);
 void btrfs_pin_extent(struct btrfs_fs_info *fs_info, u64 bytenr, u64 num_bytes);
 void btrfs_unpin_extent(struct btrfs_fs_info *fs_info,
 			u64 bytenr, u64 num_bytes);
-int btrfs_extent_post_op(struct btrfs_trans_handle *trans);
 struct btrfs_block_group_cache *btrfs_lookup_block_group(struct
 							 btrfs_fs_info *info,
 							 u64 bytenr);
diff --git a/disk-io.c b/disk-io.c
index 26e4f6e93ed6..2e6d56a36af9 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -730,7 +730,6 @@ struct btrfs_fs_info *btrfs_new_fs_info(int writable, u64 sb_bytenr)
 	extent_io_tree_init(&fs_info->free_space_cache);
 	extent_io_tree_init(&fs_info->block_group_cache);
 	extent_io_tree_init(&fs_info->pinned_extents);
-	extent_io_tree_init(&fs_info->pending_del);
 	extent_io_tree_init(&fs_info->extent_ins);
 	fs_info->excluded_extents = NULL;
 
@@ -988,7 +987,6 @@ void btrfs_cleanup_all_caches(struct btrfs_fs_info *fs_info)
 	extent_io_tree_cleanup(&fs_info->free_space_cache);
 	extent_io_tree_cleanup(&fs_info->block_group_cache);
 	extent_io_tree_cleanup(&fs_info->pinned_extents);
-	extent_io_tree_cleanup(&fs_info->pending_del);
 	extent_io_tree_cleanup(&fs_info->extent_ins);
 }
 
diff --git a/extent-tree.c b/extent-tree.c
index 2fa51bbc0359..6893b4c07019 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -52,8 +52,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 			 u64 bytenr, u64 num_bytes, u64 parent,
 			 u64 root_objectid, u64 owner_objectid,
 			 u64 owner_offset, int refs_to_drop);
-static int finish_current_insert(struct btrfs_trans_handle *trans);
-static int del_pending_extents(struct btrfs_trans_handle *trans);
 static struct btrfs_block_group_cache *
 btrfs_find_block_group(struct btrfs_root *root, struct btrfs_block_group_cache
 		       *hint, u64 search_start, int data, int owner);
@@ -1422,13 +1420,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
 	return err;
 }
 
-int btrfs_extent_post_op(struct btrfs_trans_handle *trans)
-{
-	finish_current_insert(trans);
-	del_pending_extents(trans);
-	return 0;
-}
-
 int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 			     struct btrfs_fs_info *fs_info, u64 bytenr,
 			     u64 offset, int metadata, u64 *refs, u64 *flags)
@@ -2012,74 +2003,6 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans,
 	return 0;
 }
 
-static int extent_root_pending_ops(struct btrfs_fs_info *info)
-{
-	u64 start;
-	u64 end;
-	int ret;
-
-	ret = find_first_extent_bit(&info->extent_ins, 0, &start,
-				    &end, EXTENT_LOCKED);
-	if (!ret) {
-		ret = find_first_extent_bit(&info->pending_del, 0, &start, &end,
-					    EXTENT_LOCKED);
-	}
-	return ret == 0;
-
-}
-static int finish_current_insert(struct btrfs_trans_handle *trans)
-{
-	u64 start;
-	u64 end;
-	u64 priv;
-	struct btrfs_fs_info *info = trans->fs_info;
-	struct btrfs_root *extent_root = info->extent_root;
-	struct pending_extent_op *extent_op;
-	struct btrfs_key key;
-	int ret;
-	int skinny_metadata =
-		btrfs_fs_incompat(extent_root->fs_info, SKINNY_METADATA);
-
-
-	while(1) {
-		ret = find_first_extent_bit(&info->extent_ins, 0, &start,
-					    &end, EXTENT_LOCKED);
-		if (ret)
-			break;
-
-		ret = get_state_private(&info->extent_ins, start, &priv);
-		BUG_ON(ret);
-		extent_op = (struct pending_extent_op *)(unsigned long)priv;
-
-		if (extent_op->type == PENDING_EXTENT_INSERT) {
-			key.objectid = start;
-			if (skinny_metadata) {
-				key.offset = extent_op->level;
-				key.type = BTRFS_METADATA_ITEM_KEY;
-			} else {
-				key.offset = extent_op->num_bytes;
-				key.type = BTRFS_EXTENT_ITEM_KEY;
-			}
-
-			ret = alloc_reserved_tree_block(trans,
-						extent_root->root_key.objectid,
-						trans->transid,
-						extent_op->flags,
-						&extent_op->key,
-						extent_op->level, &key);
-			BUG_ON(ret);
-		} else {
-			BUG_ON(1);
-		}
-
-
-		printf("shouldn't be executed\n");
-		clear_extent_bits(&info->extent_ins, start, end, EXTENT_LOCKED);
-		kfree(extent_op);
-	}
-	return 0;
-}
-
 static int pin_down_bytes(struct btrfs_trans_handle *trans, u64 bytenr,
 			  u64 num_bytes, int is_data)
 {
@@ -2376,66 +2299,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-/*
- * find all the blocks marked as pending in the radix tree and remove
- * them from the extent map
- */
-static int del_pending_extents(struct btrfs_trans_handle *trans)
-{
-	int ret;
-	int err = 0;
-	u64 start;
-	u64 end;
-	u64 priv;
-	struct extent_io_tree *pending_del;
-	struct extent_io_tree *extent_ins;
-	struct pending_extent_op *extent_op;
-	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *extent_root = fs_info->extent_root;
-
-	extent_ins = &extent_root->fs_info->extent_ins;
-	pending_del = &extent_root->fs_info->pending_del;
-
-	while(1) {
-		ret = find_first_extent_bit(pending_del, 0, &start, &end,
-					    EXTENT_LOCKED);
-		if (ret)
-			break;
-
-		ret = get_state_private(pending_del, start, &priv);
-		BUG_ON(ret);
-		extent_op = (struct pending_extent_op *)(unsigned long)priv;
-
-		clear_extent_bits(pending_del, start, end, EXTENT_LOCKED);
-
-		if (!test_range_bit(extent_ins, start, end,
-				    EXTENT_LOCKED, 0)) {
-			ret = __free_extent(trans, start, end + 1 - start, 0,
-					    extent_root->root_key.objectid,
-					    extent_op->level, 0, 1);
-			kfree(extent_op);
-		} else {
-			kfree(extent_op);
-			ret = get_state_private(extent_ins, start, &priv);
-			BUG_ON(ret);
-			extent_op = (struct pending_extent_op *)
-							(unsigned long)priv;
-
-			clear_extent_bits(extent_ins, start, end,
-					  EXTENT_LOCKED);
-
-			if (extent_op->type == PENDING_BACKREF_UPDATE)
-				BUG_ON(1);
-
-			kfree(extent_op);
-		}
-		if (ret)
-			err = ret;
-	}
-	return err;
-}
-
-
 int btrfs_free_tree_block(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root,
 			  struct extent_buffer *buf,
-- 
2.7.4

[PATCH 7/8] btrfs-progs: Remove __free_extent2

[Nikolay Borisov]

Now that delayed refs have been all wired up clean up the __free_extent2
adapter function since it's no longer needed. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 extent-tree.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/extent-tree.c b/extent-tree.c
index 6893b4c07019..1a63efdd9681 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -2052,17 +2052,6 @@ void btrfs_unpin_extent(struct btrfs_fs_info *fs_info,
 	update_pinned_extents(fs_info, bytenr, num_bytes, 0);
 }
 
-static int __free_extent2(struct btrfs_trans_handle *trans,
-			  struct btrfs_delayed_ref_node *node,
-			  struct btrfs_delayed_extent_op *extent_op)
-{
-
-	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
-
-	return __free_extent(trans, node->bytenr, node->num_bytes,
-			     ref->parent, ref->root, ref->level, 0, 1);
-}
-
 /*
  * remove an extent from the root, returns 0 on success
  */
@@ -4183,7 +4172,9 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle *trans,
 		BUG_ON(!extent_op || !extent_op->update_flags);
 		ret = alloc_reserved_tree_block2(trans, node, extent_op);
 	} else if (node->action == BTRFS_DROP_DELAYED_REF) {
-		ret = __free_extent2(trans, node, extent_op);
+		struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
+		ret =  __free_extent(trans, node->bytenr, node->num_bytes,
+			     ref->parent, ref->root, ref->level, 0, 1);
 	} else {
 		BUG();
 	}
-- 
2.7.4

[PATCH 8/8] btrfs-progs: Merge alloc_reserved_tree_block(2|)

[Nikolay Borisov]

Now that delayed refs have been wired let's merge the two function. In
the process also remove one BUG_ON since alloc_reserved_tree_block's
callers can handle errors. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 extent-tree.c | 77 +++++++++++++++++++++++------------------------------------
 1 file changed, 30 insertions(+), 47 deletions(-)

diff --git a/extent-tree.c b/extent-tree.c
index 1a63efdd9681..b9a30644720b 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -44,10 +44,6 @@ struct pending_extent_op {
 	int level;
 };
 
-static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
-				     u64 root_objectid, u64 generation,
-				     u64 flags, struct btrfs_disk_key *key,
-				     int level, struct btrfs_key *ins);
 static int __free_extent(struct btrfs_trans_handle *trans,
 			 u64 bytenr, u64 num_bytes, u64 parent,
 			 u64 root_objectid, u64 owner_objectid,
@@ -2528,16 +2524,22 @@ int btrfs_reserve_extent(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
+static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
 				      struct btrfs_delayed_ref_node *node,
 				      struct btrfs_delayed_extent_op *extent_op)
 {
 
 	struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
-	struct btrfs_key ins;
 	bool skinny_metadata = btrfs_fs_incompat(trans->fs_info, SKINNY_METADATA);
-	int ret;
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_extent_item *extent_item;
+	struct btrfs_extent_inline_ref *iref;
+	struct extent_buffer *leaf;
+	struct btrfs_path *path;
+	struct btrfs_key ins;
+	u32 size = sizeof(*extent_item) + sizeof(*iref);
 	u64 start, end;
+	int ret;
 
 	ins.objectid = node->bytenr;
 	if (skinny_metadata) {
@@ -2546,6 +2548,8 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
 	} else {
 		ins.offset = node->num_bytes;
 		ins.type = BTRFS_EXTENT_ITEM_KEY;
+
+		size += sizeof(struct btrfs_tree_block_info);
 	}
 
 	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
@@ -2557,69 +2561,48 @@ static int alloc_reserved_tree_block2(struct btrfs_trans_handle *trans,
 		ASSERT(end == node->bytenr + node->num_bytes - 1);
 	}
 
-	ret = alloc_reserved_tree_block(trans, ref->root, trans->transid,
-					extent_op->flags_to_set,
-					&extent_op->key, ref->level, &ins);
-
-	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
-		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
-				  EXTENT_LOCKED);
-	}
-
-	return ret;
-}
-
-static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
-				     u64 root_objectid, u64 generation,
-				     u64 flags, struct btrfs_disk_key *key,
-				     int level, struct btrfs_key *ins)
-{
-	int ret;
-	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_extent_item *extent_item;
-	struct btrfs_tree_block_info *block_info;
-	struct btrfs_extent_inline_ref *iref;
-	struct btrfs_path *path;
-	struct extent_buffer *leaf;
-	u32 size = sizeof(*extent_item) + sizeof(*iref);
-	int skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
-
-	if (!skinny_metadata)
-		size += sizeof(*block_info);
-
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
 
 	ret = btrfs_insert_empty_item(trans, fs_info->extent_root, path,
-				      ins, size);
-	BUG_ON(ret);
+				      &ins, size);
+	if (ret)
+		return ret;
 
 	leaf = path->nodes[0];
 	extent_item = btrfs_item_ptr(leaf, path->slots[0],
 				     struct btrfs_extent_item);
 	btrfs_set_extent_refs(leaf, extent_item, 1);
-	btrfs_set_extent_generation(leaf, extent_item, generation);
+	btrfs_set_extent_generation(leaf, extent_item, trans->transid);
 	btrfs_set_extent_flags(leaf, extent_item,
-			       flags | BTRFS_EXTENT_FLAG_TREE_BLOCK);
+			       extent_op->flags_to_set |
+			       BTRFS_EXTENT_FLAG_TREE_BLOCK);
 
 	if (skinny_metadata) {
 		iref = (struct btrfs_extent_inline_ref *)(extent_item + 1);
 	} else {
+		struct btrfs_tree_block_info *block_info;
 		block_info = (struct btrfs_tree_block_info *)(extent_item + 1);
-		btrfs_set_tree_block_key(leaf, block_info, key);
-		btrfs_set_tree_block_level(leaf, block_info, level);
+		btrfs_set_tree_block_key(leaf, block_info, &extent_op->key);
+		btrfs_set_tree_block_level(leaf, block_info, ref->level);
 		iref = (struct btrfs_extent_inline_ref *)(block_info + 1);
 	}
 
 	btrfs_set_extent_inline_ref_type(leaf, iref, BTRFS_TREE_BLOCK_REF_KEY);
-	btrfs_set_extent_inline_ref_offset(leaf, iref, root_objectid);
+	btrfs_set_extent_inline_ref_offset(leaf, iref, ref->root);
 
 	btrfs_mark_buffer_dirty(leaf);
 	btrfs_free_path(path);
 
-	ret = update_block_group(fs_info, ins->objectid, fs_info->nodesize,
-				 1, 0);
+	ret = update_block_group(fs_info, ins.objectid, fs_info->nodesize, 1,
+				 0);
+
+	if (ref->root == BTRFS_EXTENT_TREE_OBJECTID) {
+		clear_extent_bits(&trans->fs_info->extent_ins, start, end,
+				  EXTENT_LOCKED);
+	}
+
 	return ret;
 }
 
@@ -4170,7 +4153,7 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle *trans,
 	}
 	if (node->action == BTRFS_ADD_DELAYED_REF && insert_reserved) {
 		BUG_ON(!extent_op || !extent_op->update_flags);
-		ret = alloc_reserved_tree_block2(trans, node, extent_op);
+		ret = alloc_reserved_tree_block(trans, node, extent_op);
 	} else if (node->action == BTRFS_DROP_DELAYED_REF) {
 		struct btrfs_delayed_tree_ref *ref = btrfs_delayed_node_to_tree_ref(node);
 		ret =  __free_extent(trans, node->bytenr, node->num_bytes,
-- 
2.7.4

Re: [PATCH 0/8 V2] Add delayed-refs support to btrfs-progs

[David Sterba]

On Thu, Aug 16, 2018 at 04:10:27PM +0300, Nikolay Borisov wrote:
> Hello, 
> 
> Here is the second version of the delayed refs for progs support. The first 
> version can be found here [1]. I've taken into account all the feedback from 
> Misono and have verified the code is working and rebased it atop btrfs-progs
> 4.17.1.
> 
> Changes since v1: 
>  * Removed num_entries variable from delayed ref root
>  
>  * Added a patch to refactor btrfs_write_dirty_block_groups and subsequently 
>  changed when this function is called to fix an issue reported by Misono. I
>  verified that 'make test-fsck TEST_ENABLE_OVERRIDE=true TEST_ARGS_CHECK=--mode=lowmem'
>  produces no errors
> 
>  * Added 2 patches which remove the newly added adapter functions at the 
>  beggining of the series, following the wiring up of the delayed refs 
>  infrastructured. The first one (dealing with __free_extent2) is trivial, while
>  the second one (for alloc_reserved_tree_block2) is a bit more involved, since
>  I've opted to merge the two functions. 
> 
>  * Rebased atop latest btrfs-progs release - 4.17.1
> 
>  * Dropped patches which have been merged in the mean time
> 
> 
> [1] https://www.spinics.net/lists/linux-btrfs/msg79173.html
> 
> Nikolay Borisov (8):
>   btrfs-progs: Add __free_extent2 function
>   btrfs-progs: Add alloc_reserved_tree_block2 function
>   btrfs-progs: Add delayed refs infrastructure
>   btrfs-progs: Make btrfs_write_dirty_block_groups take only trans
>     argument
>   btrfs-progs: Wire up delayed refs
>   btrfs-progs: Remove old delayed refs infrastructure
>   btrfs-progs: Remove __free_extent2
>   btrfs-progs: Merge alloc_reserved_tree_block(2|)

For the record, the v2 patchset has been merged to devel.