[PATCH 0/6] btrfs-progs: better space_cache=v2 support

[PATCH 0/6] btrfs-progs: better space_cache=v2 support

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

Hi,

This series implements some support for space_cache=v2 in btrfs-progs.
In particular, this adds support for `btrfs check --clear-space-cache v2`,
proper printing of the free space tree flags for `btrfs inspect-internal
dump-super`, and better documentation.

We'd previously talked about always making btrfs-progs always invalidate
the free space tree when doing write operations, but I decided that this
should be an action explicitly requested by the user. It'd also be
unsafe if using a kernel without the free space tree valid bit support,
which is why I didn't implement a `btrfs check --invalidate-free-space-cache`
option. Doing the full clear is always safe.

Still missing is full read-write support, but this should hopefully
cover most btrfs-progs usage.

Thanks!

Omar Sandoval (6):
  btrfs-progs: add the FREE_SPACE_TREE_VALID compat_ro bit definition
  btrfs-progs: format FREE_SPACE_TREE{,_VALID} nicely in dump-super
  btrfs-progs: add OPEN_CTREE_INVALIDATE_FST flag
  btrfs-progs: add btrfs_clear_free_space_tree() from the kernel
  btrfs-progs: implement btrfs check --clear-space-cache v2
  btrfs-progs: document space_cache=v2 more thoroughly

 Documentation/btrfs-check.asciidoc | 14 +++---
 Documentation/btrfs-man5.asciidoc  | 43 +++++++++++--------
 chunk-recover.c                    |  2 +-
 cmds-check.c                       | 34 +++++++++++----
 cmds-inspect-dump-super.c          | 24 +++++++++++
 ctree.h                            | 19 +++++++++
 disk-io.c                          | 28 +++++++-----
 disk-io.h                          |  8 +++-
 extent-tree.c                      | 10 +++++
 free-space-tree.c                  | 87 ++++++++++++++++++++++++++++++++++++++
 free-space-tree.h                  |  1 +
 root-tree.c                        | 22 ++++++++++
 12 files changed, 248 insertions(+), 44 deletions(-)

-- 
2.10.2

[PATCH 1/6] btrfs-progs: add the FREE_SPACE_TREE_VALID compat_ro bit definition

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

This is just the definition; we don't support it yet.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 ctree.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/ctree.h b/ctree.h
index d47f0ae..d67b852 100644
--- a/ctree.h
+++ b/ctree.h
@@ -467,6 +467,14 @@ struct btrfs_super_block {
  * ones specified below then we will fail to mount
  */
 #define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE	(1ULL << 0)
+/*
+ * Older kernels on big-endian systems produced broken free space tree bitmaps,
+ * and btrfs-progs also used to corrupt the free space tree. If this bit is
+ * clear, then the free space tree cannot be trusted. btrfs-progs can also
+ * intentionally clear this bit to ask the kernel to rebuild the free space
+ * tree.
+ */
+#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID	(1ULL << 1)
 
 #define BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF	(1ULL << 0)
 #define BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL	(1ULL << 1)
@@ -493,6 +501,11 @@ struct btrfs_super_block {
 
 #define BTRFS_FEATURE_COMPAT_SUPP		0ULL
 
+/*
+ * The FREE_SPACE_TREE and FREE_SPACE_TREE_VALID compat_ro bits must not be
+ * added here until read-write support for the free space tree is implemented in
+ * btrfs-progs.
+ */
 #define BTRFS_FEATURE_COMPAT_RO_SUPP		0ULL
 
 #define BTRFS_FEATURE_INCOMPAT_SUPP			\
-- 
2.10.2

[PATCH 2/6] btrfs-progs: format FREE_SPACE_TREE{,_VALID} nicely in dump-super

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 cmds-inspect-dump-super.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
index d9f7bfb..ba0d708 100644
--- a/cmds-inspect-dump-super.c
+++ b/cmds-inspect-dump-super.c
@@ -197,6 +197,16 @@ struct readable_flag_entry {
 	char *output;
 };
 
+#define DEF_COMPAT_RO_FLAG_ENTRY(bit_name)		\
+	{BTRFS_FEATURE_COMPAT_RO_##bit_name, #bit_name}
+
+static struct readable_flag_entry compat_ro_flags_array[] = {
+	DEF_COMPAT_RO_FLAG_ENTRY(FREE_SPACE_TREE),
+	DEF_COMPAT_RO_FLAG_ENTRY(FREE_SPACE_TREE_VALID),
+};
+static const int compat_ro_flags_num = sizeof(compat_ro_flags_array) /
+				       sizeof(struct readable_flag_entry);
+
 #define DEF_INCOMPAT_FLAG_ENTRY(bit_name)		\
 	{BTRFS_FEATURE_INCOMPAT_##bit_name, #bit_name}
 
@@ -268,6 +278,19 @@ static void __print_readable_flag(u64 flag, struct readable_flag_entry *array,
 	printf(")\n");
 }
 
+static void print_readable_compat_ro_flag(u64 flag)
+{
+	/*
+	 * We know about the FREE_SPACE_TREE{,_VALID} bits, but we don't
+	 * actually support them yet.
+	 */
+	return __print_readable_flag(flag, compat_ro_flags_array,
+				     compat_ro_flags_num,
+				     BTRFS_FEATURE_COMPAT_RO_SUPP |
+				     BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |
+				     BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID);
+}
+
 static void print_readable_incompat_flag(u64 flag)
 {
 	return __print_readable_flag(flag, incompat_flags_array,
@@ -377,6 +400,7 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
 	       (unsigned long long)btrfs_super_compat_flags(sb));
 	printf("compat_ro_flags\t\t0x%llx\n",
 	       (unsigned long long)btrfs_super_compat_ro_flags(sb));
+	print_readable_compat_ro_flag(btrfs_super_compat_ro_flags(sb));
 	printf("incompat_flags\t\t0x%llx\n",
 	       (unsigned long long)btrfs_super_incompat_flags(sb));
 	print_readable_incompat_flag(btrfs_super_incompat_flags(sb));
-- 
2.10.2

[PATCH 3/6] btrfs-progs: add OPEN_CTREE_INVALIDATE_FST flag

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

If this flag is passed to open_ctree(), we'll clear the
FREE_SPACE_TREE_VALID compat_ro bit. The kernel will then reconstruct
the free space tree the next time the filesystem is mounted.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 chunk-recover.c |  2 +-
 disk-io.c       | 28 ++++++++++++++++++----------
 disk-io.h       |  8 +++++++-
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index e33ee8b..e6b26ac 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -1477,7 +1477,7 @@ open_ctree_with_broken_chunk(struct recover_control *rc)
 
 	memcpy(fs_info->fsid, &disk_super->fsid, BTRFS_FSID_SIZE);
 
-	ret = btrfs_check_fs_compatibility(disk_super, 1);
+	ret = btrfs_check_fs_compatibility(disk_super, OPEN_CTREE_WRITES);
 	if (ret)
 		goto out_devices;
 
diff --git a/disk-io.c b/disk-io.c
index a576300..a55fcc1 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -904,7 +904,7 @@ free_all:
 	return NULL;
 }
 
-int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, int writable)
+int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, unsigned flags)
 {
 	u64 features;
 
@@ -923,13 +923,22 @@ int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, int writable)
 		btrfs_set_super_incompat_flags(sb, features);
 	}
 
-	features = btrfs_super_compat_ro_flags(sb) &
-		~BTRFS_FEATURE_COMPAT_RO_SUPP;
-	if (writable && features) {
-		printk("couldn't open RDWR because of unsupported "
-		       "option features (%Lx).\n",
-		       (unsigned long long)features);
-		return -ENOTSUP;
+	features = btrfs_super_compat_ro_flags(sb);
+	if (flags & OPEN_CTREE_WRITES) {
+		if (flags & OPEN_CTREE_INVALIDATE_FST) {
+			/* Clear the FREE_SPACE_TREE_VALID bit on disk... */
+			features &= ~BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID;
+			btrfs_set_super_compat_ro_flags(sb, features);
+			/* ... and ignore the free space tree bit. */
+			features &= ~BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE;
+		}
+		if (features & ~BTRFS_FEATURE_COMPAT_RO_SUPP) {
+			printk("couldn't open RDWR because of unsupported "
+			       "option features (%Lx).\n",
+			       (unsigned long long)features);
+			return -ENOTSUP;
+		}
+
 	}
 	return 0;
 }
@@ -1320,8 +1329,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 
 	memcpy(fs_info->fsid, &disk_super->fsid, BTRFS_FSID_SIZE);
 
-	ret = btrfs_check_fs_compatibility(fs_info->super_copy,
-					   flags & OPEN_CTREE_WRITES);
+	ret = btrfs_check_fs_compatibility(fs_info->super_copy, flags);
 	if (ret)
 		goto out_devices;
 
diff --git a/disk-io.h b/disk-io.h
index 8ab36aa..5d4b281 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -74,6 +74,12 @@ enum btrfs_open_ctree_flags {
 
 	/* Allow to open a partially created filesystem */
 	OPEN_CTREE_FS_PARTIAL = (1U << 12),
+
+	/*
+	 * Invalidate the free space tree (i.e., clear the FREE_SPACE_TREE_VALID
+	 * compat_ro bit).
+	 */
+	OPEN_CTREE_INVALIDATE_FST = (1U << 13),
 };
 
 /*
@@ -128,7 +134,7 @@ int clean_tree_block(struct btrfs_trans_handle *trans,
 
 void btrfs_free_fs_info(struct btrfs_fs_info *fs_info);
 struct btrfs_fs_info *btrfs_new_fs_info(int writable, u64 sb_bytenr);
-int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, int writable);
+int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, unsigned flags);
 int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
 			  unsigned flags);
 void btrfs_release_all_roots(struct btrfs_fs_info *fs_info);
-- 
2.10.2

[PATCH 4/6] btrfs-progs: add btrfs_clear_free_space_tree() from the kernel

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 ctree.h           |  6 ++++
 extent-tree.c     | 10 +++++++
 free-space-tree.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 free-space-tree.h |  1 +
 root-tree.c       | 22 ++++++++++++++
 5 files changed, 126 insertions(+)

diff --git a/ctree.h b/ctree.h
index d67b852..90193ad 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2504,6 +2504,10 @@ int btrfs_inc_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		  struct extent_buffer *buf, int record_parent);
 int btrfs_dec_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		  struct extent_buffer *buf, int record_parent);
+void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
+			   struct btrfs_root *root,
+			   struct extent_buffer *buf,
+			   u64 parent, int last_ref);
 int btrfs_free_extent(struct btrfs_trans_handle *trans,
 		      struct btrfs_root *root,
 		      u64 bytenr, u64 num_bytes, u64 parent,
@@ -2664,6 +2668,8 @@ int btrfs_add_root_ref(struct btrfs_trans_handle *trans,
 int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root
 		      *root, struct btrfs_key *key, struct btrfs_root_item
 		      *item);
+int btrfs_del_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		   struct btrfs_key *key);
 int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
 		      *root, struct btrfs_key *key, struct btrfs_root_item
 		      *item);
diff --git a/extent-tree.c b/extent-tree.c
index 3b1577e..d445723 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -2467,6 +2467,16 @@ static int del_pending_extents(struct btrfs_trans_handle *trans, struct
 	return err;
 }
 
+
+void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
+			   struct btrfs_root *root,
+			   struct extent_buffer *buf,
+			   u64 parent, int last_ref)
+{
+	btrfs_free_extent(trans, root, buf->start, buf->len, parent,
+			  root->root_key.objectid, btrfs_header_level(buf), 0);
+}
+
 /*
  * remove an extent from the root, returns 0 on success
  */
diff --git a/free-space-tree.c b/free-space-tree.c
index 3c7a246..d972f26 100644
--- a/free-space-tree.c
+++ b/free-space-tree.c
@@ -20,6 +20,7 @@
 #include "disk-io.h"
 #include "free-space-cache.h"
 #include "free-space-tree.h"
+#include "transaction.h"
 
 static struct btrfs_free_space_info *
 search_free_space_info(struct btrfs_trans_handle *trans,
@@ -67,6 +68,92 @@ static int free_space_test_bit(struct btrfs_block_group_cache *block_group,
 	return !!extent_buffer_test_bit(leaf, ptr, i);
 }
 
+static int clear_free_space_tree(struct btrfs_trans_handle *trans,
+				 struct btrfs_root *root)
+{
+	struct btrfs_path *path;
+	struct btrfs_key key;
+	int nr;
+	int ret;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	key.objectid = 0;
+	key.type = 0;
+	key.offset = 0;
+
+	while (1) {
+		ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
+		if (ret < 0)
+			goto out;
+
+		nr = btrfs_header_nritems(path->nodes[0]);
+		if (!nr)
+			break;
+
+		path->slots[0] = 0;
+		ret = btrfs_del_items(trans, root, path, 0, nr);
+		if (ret)
+			goto out;
+
+		btrfs_release_path(path);
+	}
+
+	ret = 0;
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
+int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_root *free_space_root = fs_info->free_space_root;
+	int ret;
+	u64 features;
+
+	trans = btrfs_start_transaction(tree_root, 0);
+	if (IS_ERR(trans))
+		return PTR_ERR(trans);
+
+
+	features = btrfs_super_compat_ro_flags(fs_info->super_copy);
+	features &= ~(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID |
+		      BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE);
+	btrfs_set_super_compat_ro_flags(fs_info->super_copy, features);
+	fs_info->free_space_root = NULL;
+
+	ret = clear_free_space_tree(trans, free_space_root);
+	if (ret)
+		goto abort;
+
+	ret = btrfs_del_root(trans, tree_root, &free_space_root->root_key);
+	if (ret)
+		goto abort;
+
+	list_del(&free_space_root->dirty_list);
+
+	clean_tree_block(trans, tree_root, free_space_root->node);
+	btrfs_free_tree_block(trans, free_space_root, free_space_root->node,
+			      0, 1);
+
+	free_extent_buffer(free_space_root->node);
+	free_extent_buffer(free_space_root->commit_root);
+	kfree(free_space_root);
+
+	ret = btrfs_commit_transaction(trans, tree_root);
+	if (ret)
+		return ret;
+
+	return 0;
+
+abort:
+	return ret;
+}
+
 static int load_free_space_bitmaps(struct btrfs_fs_info *fs_info,
 				   struct btrfs_block_group_cache *block_group,
 				   struct btrfs_path *path,
diff --git a/free-space-tree.h b/free-space-tree.h
index 7529a46..4845f13 100644
--- a/free-space-tree.h
+++ b/free-space-tree.h
@@ -19,6 +19,7 @@
 #ifndef __BTRFS_FREE_SPACE_TREE_H__
 #define __BTRFS_FREE_SPACE_TREE_H__
 
+int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info);
 int load_free_space_tree(struct btrfs_fs_info *fs_info,
 			 struct btrfs_block_group_cache *block_group);
 
diff --git a/root-tree.c b/root-tree.c
index cca424e..c660447 100644
--- a/root-tree.c
+++ b/root-tree.c
@@ -143,6 +143,28 @@ int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root
 	return ret;
 }
 
+/* drop the root item for 'key' from 'root' */
+int btrfs_del_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		   struct btrfs_key *key)
+{
+	struct btrfs_path *path;
+	int ret;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+	ret = btrfs_search_slot(trans, root, key, path, -1, 1);
+	if (ret < 0)
+		goto out;
+
+	BUG_ON(ret != 0);
+
+	ret = btrfs_del_item(trans, root, path);
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 /*
  * add a btrfs_root_ref item.  type is either BTRFS_ROOT_REF_KEY
  * or BTRFS_ROOT_BACKREF_KEY.
-- 
2.10.2

[PATCH 5/6] btrfs-progs: implement btrfs check --clear-space-cache v2

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 Documentation/btrfs-check.asciidoc | 14 +++++++++-----
 cmds-check.c                       | 34 +++++++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/Documentation/btrfs-check.asciidoc b/Documentation/btrfs-check.asciidoc
index 5ef414e..633cbbf 100644
--- a/Documentation/btrfs-check.asciidoc
+++ b/Documentation/btrfs-check.asciidoc
@@ -80,12 +80,16 @@ superblock is damaged.
 
 --clear-space-cache v1|v2::
 completely wipe all free space cache of given type
-
-NOTE: Only v1 free space cache supported is implemented.
 +
-Kernel mount option 'clear_cache' is only designed to rebuild free space cache
-which is modified during the lifetime of that mount option.  It doesn't rebuild
-all free space cache, nor clear them out.
+For free space cache 'v1', the 'clear_cache' kernel mount option only rebuilds
+the free space cache for block groups that are modified while the filesystem is
+mounted with that option. Thus, using this option with 'v1' makes it possible
+to actually clear the entire free space cache.
++
+For free space cache 'v2', the 'clear_cache' kernel mount option does destroy
+the entire free space cache. This option with 'v2' provides an alternative
+method of clearing the free space cache that doesn't require mounting the
+filesystem.
 
 
 DANGEROUS OPTIONS
diff --git a/cmds-check.c b/cmds-check.c
index 57c4300..0eca102 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -11170,7 +11170,6 @@ const char * const cmd_check_usage[] = {
 	"--chunk-root <bytenr>       use the given bytenr for the chunk tree root",
 	"-p|--progress               indicate progress",
 	"--clear-space-cache v1|v2   clear space cache for v1 or v2",
-	"                            NOTE: v1 support implemented",
 	NULL
 };
 
@@ -11292,13 +11291,16 @@ int cmd_check(int argc, char **argv)
 				}
 				break;
 			case GETOPT_VAL_CLEAR_SPACE_CACHE:
-				if (strcmp(optarg, "v1") != 0) {
-					error(
-			"only v1 support implmented, unrecognized value %s",
-			optarg);
+				if (strcmp(optarg, "v1") == 0) {
+					clear_space_cache = 1;
+				} else if (strcmp(optarg, "v2") == 0) {
+					clear_space_cache = 2;
+					ctree_flags |= OPEN_CTREE_INVALIDATE_FST;
+				} else {
+					error("invalid argument to --clear-space-cache; "
+					      "must be v1 or v2");
 					exit(1);
 				}
-				clear_space_cache = 1;
 				ctree_flags |= OPEN_CTREE_WRITES;
 				break;
 		}
@@ -11352,11 +11354,10 @@ int cmd_check(int argc, char **argv)
 
 	global_info = info;
 	root = info->fs_root;
-	if (clear_space_cache) {
+	if (clear_space_cache == 1) {
 		if (btrfs_fs_compat_ro(info,
 				BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE)) {
-			error(
-			"free space cache v2 detected, clearing not implemented");
+			error("free space cache v2 detected; use --clear-space-cache v2");
 			ret = 1;
 			goto close_out;
 		}
@@ -11369,6 +11370,21 @@ int cmd_check(int argc, char **argv)
 			printf("Free space cache cleared\n");
 		}
 		goto close_out;
+	} else if (clear_space_cache == 2) {
+		if (!btrfs_fs_compat_ro(info, BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE)) {
+			printf("No free space cache v2 to clear\n");
+			ret = 0;
+			goto close_out;
+		}
+		printf("Clear free space cache v2\n");
+		ret = btrfs_clear_free_space_tree(info);
+		if (ret) {
+			error("Failed to clear free space cache v2");
+			ret = 1;
+		} else {
+			printf("Free space cache v2 cleared\n");
+		}
+		goto close_out;
 	}
 
 	/*
-- 
2.10.2

[PATCH 6/6] btrfs-progs: document space_cache=v2 more thoroughly

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 Documentation/btrfs-man5.asciidoc | 43 +++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/Documentation/btrfs-man5.asciidoc b/Documentation/btrfs-man5.asciidoc
index d12b059..3b003d3 100644
--- a/Documentation/btrfs-man5.asciidoc
+++ b/Documentation/btrfs-man5.asciidoc
@@ -319,25 +319,32 @@ May be resumed with *btrfs balance resume* or the paused state can be removed
 by *btrfs balance cancel*. The default behaviour is to start interrutpd balance.
 
 *space_cache*::
-*space_cache=v2*::
+*space_cache='version'*::
 *nospace_cache*::
-('nospace_cache' since: 3.2, 'space_cache=v2' since 4.5, default: on)
-+
-Options to control the free space cache.  This affects performance as searching
-for new free blocks could take longer if the space cache is not enabled. On the
-other hand, managing the space cache consumes some resources.  It can be
-disabled without clearing at mount time.
-+
-There are two implementations of how the space is tracked. The safe default is
-'v1'.  On large filesystems (many-terabytes) and certain workloads the 'v1'
-performance may degrade.  This problem is addressed by 'v2', that is based on
-b-trees, sometimes referred to as 'free-space-tree'.
-+
-'Compatibility notes:'
-+
-* the 'v2' has to be enabled manually at mount time, once
-* kernel without 'v2' support will be able to mount the filesystem in read-only mode
-* 'v2' can be removed by mounting with 'clear_cache'
+('nospace_cache' since: 3.2, 'space_cache=v1' and 'space_cache=v2' since 4.5, default: 'space_cache=v1')
++
+Options to control the free space cache. The free space cache greatly improves
+performance when reading block group free space into memory. However, managing
+the space cache consumes some resources, including a small amount of disk
+space.
++
+There are two implementations of the free space cache. The original
+implementation, 'v1', is the safe default. The 'v1' space cache can be disabled
+at mount time with 'nospace_cache' without clearing.
++
+On very large filesystems (many terabytes) and certain workloads, the
+performance of the 'v1' space cache may degrade drastically. The 'v2'
+implementation, which adds a new B-tree called the free space tree, addresses
+this issue. Once enabled, the 'v2' space cache will always be used and cannot
+be disabled unless it is cleared. Use 'clear_cache,space_cache=v1' or
+'clear_cache,nospace_cache' to do so. If 'v2' is enabled, kernels without 'v2'
+support will only be able to mount the filesystem in read-only mode. The
+`btrfs(8)` command currently only has read-only support for 'v2'. A read-write
+command may be run on a 'v2' filesystem by clearing the cache, running the
+command, and then remounting with 'space_cache=v2'.
++
+If a version is not explicitly specified, the default implementation will be
+chosen, which is 'v1' as of 4.9.
 
 *ssd*::
 *nossd*::
-- 
2.10.2

Re: [PATCH 3/6] btrfs-progs: add OPEN_CTREE_INVALIDATE_FST flag

[Qu Wenruo]

At 11/14/2016 03:35 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> If this flag is passed to open_ctree(), we'll clear the
> FREE_SPACE_TREE_VALID compat_ro bit. The kernel will then reconstruct
> the free space tree the next time the filesystem is mounted.

This feature is really helpful.

Especially for users reporting failed to mount v2 space_cache fs.

Despite a small nit commented below, feel free to add my reviewed by tag.

Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>  chunk-recover.c |  2 +-
>  disk-io.c       | 28 ++++++++++++++++++----------
>  disk-io.h       |  8 +++++++-
>  3 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/chunk-recover.c b/chunk-recover.c
> index e33ee8b..e6b26ac 100644
> --- a/chunk-recover.c
> +++ b/chunk-recover.c
> @@ -1477,7 +1477,7 @@ open_ctree_with_broken_chunk(struct recover_control *rc)
>
>  	memcpy(fs_info->fsid, &disk_super->fsid, BTRFS_FSID_SIZE);
>
> -	ret = btrfs_check_fs_compatibility(disk_super, 1);
> +	ret = btrfs_check_fs_compatibility(disk_super, OPEN_CTREE_WRITES);
>  	if (ret)
>  		goto out_devices;
>
> diff --git a/disk-io.c b/disk-io.c
> index a576300..a55fcc1 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -904,7 +904,7 @@ free_all:
>  	return NULL;
>  }
>
> -int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, int writable)
> +int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, unsigned flags)

IIRC kernel checkpatch will warn on single "unsigned", as it's 
recommended to use "unsigned int".

Thanks,
Qu

>  {
>  	u64 features;
>
> @@ -923,13 +923,22 @@ int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, int writable)
>  		btrfs_set_super_incompat_flags(sb, features);
>  	}
>
> -	features = btrfs_super_compat_ro_flags(sb) &
> -		~BTRFS_FEATURE_COMPAT_RO_SUPP;
> -	if (writable && features) {
> -		printk("couldn't open RDWR because of unsupported "
> -		       "option features (%Lx).\n",
> -		       (unsigned long long)features);
> -		return -ENOTSUP;
> +	features = btrfs_super_compat_ro_flags(sb);
> +	if (flags & OPEN_CTREE_WRITES) {
> +		if (flags & OPEN_CTREE_INVALIDATE_FST) {
> +			/* Clear the FREE_SPACE_TREE_VALID bit on disk... */
> +			features &= ~BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID;
> +			btrfs_set_super_compat_ro_flags(sb, features);
> +			/* ... and ignore the free space tree bit. */
> +			features &= ~BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE;
> +		}
> +		if (features & ~BTRFS_FEATURE_COMPAT_RO_SUPP) {
> +			printk("couldn't open RDWR because of unsupported "
> +			       "option features (%Lx).\n",
> +			       (unsigned long long)features);
> +			return -ENOTSUP;
> +		}
> +
>  	}
>  	return 0;
>  }
> @@ -1320,8 +1329,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
>
>  	memcpy(fs_info->fsid, &disk_super->fsid, BTRFS_FSID_SIZE);
>
> -	ret = btrfs_check_fs_compatibility(fs_info->super_copy,
> -					   flags & OPEN_CTREE_WRITES);
> +	ret = btrfs_check_fs_compatibility(fs_info->super_copy, flags);
>  	if (ret)
>  		goto out_devices;
>
> diff --git a/disk-io.h b/disk-io.h
> index 8ab36aa..5d4b281 100644
> --- a/disk-io.h
> +++ b/disk-io.h
> @@ -74,6 +74,12 @@ enum btrfs_open_ctree_flags {
>
>  	/* Allow to open a partially created filesystem */
>  	OPEN_CTREE_FS_PARTIAL = (1U << 12),
> +
> +	/*
> +	 * Invalidate the free space tree (i.e., clear the FREE_SPACE_TREE_VALID
> +	 * compat_ro bit).
> +	 */
> +	OPEN_CTREE_INVALIDATE_FST = (1U << 13),
>  };
>
>  /*
> @@ -128,7 +134,7 @@ int clean_tree_block(struct btrfs_trans_handle *trans,
>
>  void btrfs_free_fs_info(struct btrfs_fs_info *fs_info);
>  struct btrfs_fs_info *btrfs_new_fs_info(int writable, u64 sb_bytenr);
> -int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, int writable);
> +int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, unsigned flags);
>  int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
>  			  unsigned flags);
>  void btrfs_release_all_roots(struct btrfs_fs_info *fs_info);
>

Re: [PATCH 4/6] btrfs-progs: add btrfs_clear_free_space_tree() from the kernel

[Qu Wenruo]

At 11/14/2016 03:35 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Only small nits about the BUG_ON() and return value.
Despite that, feel free to add my reviewed-by:

Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  ctree.h           |  6 ++++
>  extent-tree.c     | 10 +++++++
>  free-space-tree.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  free-space-tree.h |  1 +
>  root-tree.c       | 22 ++++++++++++++
>  5 files changed, 126 insertions(+)
>
> diff --git a/ctree.h b/ctree.h
> index d67b852..90193ad 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -2504,6 +2504,10 @@ int btrfs_inc_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>  		  struct extent_buffer *buf, int record_parent);
>  int btrfs_dec_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>  		  struct extent_buffer *buf, int record_parent);
> +void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> +			   struct btrfs_root *root,
> +			   struct extent_buffer *buf,
> +			   u64 parent, int last_ref);
>  int btrfs_free_extent(struct btrfs_trans_handle *trans,
>  		      struct btrfs_root *root,
>  		      u64 bytenr, u64 num_bytes, u64 parent,
> @@ -2664,6 +2668,8 @@ int btrfs_add_root_ref(struct btrfs_trans_handle *trans,
>  int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root
>  		      *root, struct btrfs_key *key, struct btrfs_root_item
>  		      *item);
> +int btrfs_del_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> +		   struct btrfs_key *key);
>  int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
>  		      *root, struct btrfs_key *key, struct btrfs_root_item
>  		      *item);
> diff --git a/extent-tree.c b/extent-tree.c
> index 3b1577e..d445723 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -2467,6 +2467,16 @@ static int del_pending_extents(struct btrfs_trans_handle *trans, struct
>  	return err;
>  }
>
> +
> +void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> +			   struct btrfs_root *root,
> +			   struct extent_buffer *buf,
> +			   u64 parent, int last_ref)
> +{
> +	btrfs_free_extent(trans, root, buf->start, buf->len, parent,
> +			  root->root_key.objectid, btrfs_header_level(buf), 0);
> +}

btrfs_free_extent() will return int.

Better return it.

> +
>  /*
>   * remove an extent from the root, returns 0 on success
>   */
> diff --git a/free-space-tree.c b/free-space-tree.c
> index 3c7a246..d972f26 100644
> --- a/free-space-tree.c
> +++ b/free-space-tree.c
> @@ -20,6 +20,7 @@
>  #include "disk-io.h"
>  #include "free-space-cache.h"
>  #include "free-space-tree.h"
> +#include "transaction.h"
>
>  static struct btrfs_free_space_info *
>  search_free_space_info(struct btrfs_trans_handle *trans,
> @@ -67,6 +68,92 @@ static int free_space_test_bit(struct btrfs_block_group_cache *block_group,
>  	return !!extent_buffer_test_bit(leaf, ptr, i);
>  }
>
> +static int clear_free_space_tree(struct btrfs_trans_handle *trans,
> +				 struct btrfs_root *root)
> +{
> +	struct btrfs_path *path;
> +	struct btrfs_key key;
> +	int nr;
> +	int ret;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	key.objectid = 0;
> +	key.type = 0;
> +	key.offset = 0;
> +
> +	while (1) {
> +		ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
> +		if (ret < 0)
> +			goto out;
> +
> +		nr = btrfs_header_nritems(path->nodes[0]);
> +		if (!nr)
> +			break;
> +
> +		path->slots[0] = 0;
> +		ret = btrfs_del_items(trans, root, path, 0, nr);
> +		if (ret)
> +			goto out;
> +
> +		btrfs_release_path(path);
> +	}
> +
> +	ret = 0;
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
> +int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info)
> +{
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_root *tree_root = fs_info->tree_root;
> +	struct btrfs_root *free_space_root = fs_info->free_space_root;
> +	int ret;
> +	u64 features;
> +
> +	trans = btrfs_start_transaction(tree_root, 0);
> +	if (IS_ERR(trans))
> +		return PTR_ERR(trans);
> +
> +
> +	features = btrfs_super_compat_ro_flags(fs_info->super_copy);
> +	features &= ~(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID |
> +		      BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE);
> +	btrfs_set_super_compat_ro_flags(fs_info->super_copy, features);
> +	fs_info->free_space_root = NULL;
> +
> +	ret = clear_free_space_tree(trans, free_space_root);
> +	if (ret)
> +		goto abort;
> +
> +	ret = btrfs_del_root(trans, tree_root, &free_space_root->root_key);
> +	if (ret)
> +		goto abort;
> +
> +	list_del(&free_space_root->dirty_list);
> +
> +	clean_tree_block(trans, tree_root, free_space_root->node);

Better catch the return value.

> +	btrfs_free_tree_block(trans, free_space_root, free_space_root->node,
> +			      0, 1);

Here too.

> +
> +	free_extent_buffer(free_space_root->node);
> +	free_extent_buffer(free_space_root->commit_root);
> +	kfree(free_space_root);
> +
> +	ret = btrfs_commit_transaction(trans, tree_root);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +
> +abort:

This reminds me again that we really need a btrfs_abort_transaction() in 
btrfs-progs.
(And I always forget to implement it)

> +	return ret;
> +}
> +
>  static int load_free_space_bitmaps(struct btrfs_fs_info *fs_info,
>  				   struct btrfs_block_group_cache *block_group,
>  				   struct btrfs_path *path,
> diff --git a/free-space-tree.h b/free-space-tree.h
> index 7529a46..4845f13 100644
> --- a/free-space-tree.h
> +++ b/free-space-tree.h
> @@ -19,6 +19,7 @@
>  #ifndef __BTRFS_FREE_SPACE_TREE_H__
>  #define __BTRFS_FREE_SPACE_TREE_H__
>
> +int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info);
>  int load_free_space_tree(struct btrfs_fs_info *fs_info,
>  			 struct btrfs_block_group_cache *block_group);
>
> diff --git a/root-tree.c b/root-tree.c
> index cca424e..c660447 100644
> --- a/root-tree.c
> +++ b/root-tree.c
> @@ -143,6 +143,28 @@ int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root
>  	return ret;
>  }
>
> +/* drop the root item for 'key' from 'root' */
> +int btrfs_del_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> +		   struct btrfs_key *key)
> +{
> +	struct btrfs_path *path;
> +	int ret;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +	ret = btrfs_search_slot(trans, root, key, path, -1, 1);
> +	if (ret < 0)
> +		goto out;
> +
> +	BUG_ON(ret != 0);

Better to avoid BUG_ON().

Return -ENOENT seems good enough.

Thanks,
Qu

> +
> +	ret = btrfs_del_item(trans, root, path);
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
>  /*
>   * add a btrfs_root_ref item.  type is either BTRFS_ROOT_REF_KEY
>   * or BTRFS_ROOT_BACKREF_KEY.
>

Re: [PATCH 5/6] btrfs-progs: implement btrfs check --clear-space-cache v2

[Qu Wenruo]

At 11/14/2016 03:35 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  Documentation/btrfs-check.asciidoc | 14 +++++++++-----
>  cmds-check.c                       | 34 +++++++++++++++++++++++++---------
>  2 files changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/btrfs-check.asciidoc b/Documentation/btrfs-check.asciidoc
> index 5ef414e..633cbbf 100644
> --- a/Documentation/btrfs-check.asciidoc
> +++ b/Documentation/btrfs-check.asciidoc
> @@ -80,12 +80,16 @@ superblock is damaged.
>
>  --clear-space-cache v1|v2::
>  completely wipe all free space cache of given type
> -
> -NOTE: Only v1 free space cache supported is implemented.
>  +
> -Kernel mount option 'clear_cache' is only designed to rebuild free space cache
> -which is modified during the lifetime of that mount option.  It doesn't rebuild
> -all free space cache, nor clear them out.
> +For free space cache 'v1', the 'clear_cache' kernel mount option only rebuilds
> +the free space cache for block groups that are modified while the filesystem is
> +mounted with that option. Thus, using this option with 'v1' makes it possible
> +to actually clear the entire free space cache.
> ++
> +For free space cache 'v2', the 'clear_cache' kernel mount option does destroy
> +the entire free space cache. This option with 'v2' provides an alternative
> +method of clearing the free space cache that doesn't require mounting the
> +filesystem.

This is quite helpful.

The behavior of clear_cache should be the correct one.

Thanks,
Qu

>
>
>  DANGEROUS OPTIONS
> diff --git a/cmds-check.c b/cmds-check.c
> index 57c4300..0eca102 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -11170,7 +11170,6 @@ const char * const cmd_check_usage[] = {
>  	"--chunk-root <bytenr>       use the given bytenr for the chunk tree root",
>  	"-p|--progress               indicate progress",
>  	"--clear-space-cache v1|v2   clear space cache for v1 or v2",
> -	"                            NOTE: v1 support implemented",
>  	NULL
>  };
>
> @@ -11292,13 +11291,16 @@ int cmd_check(int argc, char **argv)
>  				}
>  				break;
>  			case GETOPT_VAL_CLEAR_SPACE_CACHE:
> -				if (strcmp(optarg, "v1") != 0) {
> -					error(
> -			"only v1 support implmented, unrecognized value %s",
> -			optarg);
> +				if (strcmp(optarg, "v1") == 0) {
> +					clear_space_cache = 1;
> +				} else if (strcmp(optarg, "v2") == 0) {
> +					clear_space_cache = 2;
> +					ctree_flags |= OPEN_CTREE_INVALIDATE_FST;
> +				} else {
> +					error("invalid argument to --clear-space-cache; "
> +					      "must be v1 or v2");
>  					exit(1);
>  				}
> -				clear_space_cache = 1;
>  				ctree_flags |= OPEN_CTREE_WRITES;
>  				break;
>  		}
> @@ -11352,11 +11354,10 @@ int cmd_check(int argc, char **argv)
>
>  	global_info = info;
>  	root = info->fs_root;
> -	if (clear_space_cache) {
> +	if (clear_space_cache == 1) {
>  		if (btrfs_fs_compat_ro(info,
>  				BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE)) {
> -			error(
> -			"free space cache v2 detected, clearing not implemented");
> +			error("free space cache v2 detected; use --clear-space-cache v2");
>  			ret = 1;
>  			goto close_out;
>  		}
> @@ -11369,6 +11370,21 @@ int cmd_check(int argc, char **argv)
>  			printf("Free space cache cleared\n");
>  		}
>  		goto close_out;
> +	} else if (clear_space_cache == 2) {
> +		if (!btrfs_fs_compat_ro(info, BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE)) {
> +			printf("No free space cache v2 to clear\n");
> +			ret = 0;
> +			goto close_out;
> +		}
> +		printf("Clear free space cache v2\n");
> +		ret = btrfs_clear_free_space_tree(info);
> +		if (ret) {
> +			error("Failed to clear free space cache v2");
> +			ret = 1;
> +		} else {
> +			printf("Free space cache v2 cleared\n");
> +		}
> +		goto close_out;
>  	}
>
>  	/*
>

Re: [PATCH 4/6] btrfs-progs: add btrfs_clear_free_space_tree() from the kernel

[Omar Sandoval]

On Mon, Nov 14, 2016 at 09:38:19AM +0800, Qu Wenruo wrote:
> 
> 
> At 11/14/2016 03:35 AM, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> 
> Only small nits about the BUG_ON() and return value.
> Despite that, feel free to add my reviewed-by:
> 
> Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> > ---
> >  ctree.h           |  6 ++++
> >  extent-tree.c     | 10 +++++++
> >  free-space-tree.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  free-space-tree.h |  1 +
> >  root-tree.c       | 22 ++++++++++++++
> >  5 files changed, 126 insertions(+)
> > 
> > diff --git a/ctree.h b/ctree.h
> > index d67b852..90193ad 100644
> > --- a/ctree.h
> > +++ b/ctree.h
> > @@ -2504,6 +2504,10 @@ int btrfs_inc_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >  		  struct extent_buffer *buf, int record_parent);
> >  int btrfs_dec_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >  		  struct extent_buffer *buf, int record_parent);
> > +void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> > +			   struct btrfs_root *root,
> > +			   struct extent_buffer *buf,
> > +			   u64 parent, int last_ref);
> >  int btrfs_free_extent(struct btrfs_trans_handle *trans,
> >  		      struct btrfs_root *root,
> >  		      u64 bytenr, u64 num_bytes, u64 parent,
> > @@ -2664,6 +2668,8 @@ int btrfs_add_root_ref(struct btrfs_trans_handle *trans,
> >  int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root
> >  		      *root, struct btrfs_key *key, struct btrfs_root_item
> >  		      *item);
> > +int btrfs_del_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> > +		   struct btrfs_key *key);
> >  int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
> >  		      *root, struct btrfs_key *key, struct btrfs_root_item
> >  		      *item);
> > diff --git a/extent-tree.c b/extent-tree.c
> > index 3b1577e..d445723 100644
> > --- a/extent-tree.c
> > +++ b/extent-tree.c
> > @@ -2467,6 +2467,16 @@ static int del_pending_extents(struct btrfs_trans_handle *trans, struct
> >  	return err;
> >  }
> > 
> > +
> > +void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> > +			   struct btrfs_root *root,
> > +			   struct extent_buffer *buf,
> > +			   u64 parent, int last_ref)
> > +{
> > +	btrfs_free_extent(trans, root, buf->start, buf->len, parent,
> > +			  root->root_key.objectid, btrfs_header_level(buf), 0);
> > +}
> 
> btrfs_free_extent() will return int.
> 
> Better return it.

Will fix.

> > +
> >  /*
> >   * remove an extent from the root, returns 0 on success
> >   */
> > diff --git a/free-space-tree.c b/free-space-tree.c
> > index 3c7a246..d972f26 100644
> > --- a/free-space-tree.c
> > +++ b/free-space-tree.c
> > @@ -20,6 +20,7 @@
> >  #include "disk-io.h"
> >  #include "free-space-cache.h"
> >  #include "free-space-tree.h"
> > +#include "transaction.h"
> > 
> >  static struct btrfs_free_space_info *
> >  search_free_space_info(struct btrfs_trans_handle *trans,
> > @@ -67,6 +68,92 @@ static int free_space_test_bit(struct btrfs_block_group_cache *block_group,
> >  	return !!extent_buffer_test_bit(leaf, ptr, i);
> >  }
> > 
> > +static int clear_free_space_tree(struct btrfs_trans_handle *trans,
> > +				 struct btrfs_root *root)
> > +{
> > +	struct btrfs_path *path;
> > +	struct btrfs_key key;
> > +	int nr;
> > +	int ret;
> > +
> > +	path = btrfs_alloc_path();
> > +	if (!path)
> > +		return -ENOMEM;
> > +
> > +	key.objectid = 0;
> > +	key.type = 0;
> > +	key.offset = 0;
> > +
> > +	while (1) {
> > +		ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
> > +		if (ret < 0)
> > +			goto out;
> > +
> > +		nr = btrfs_header_nritems(path->nodes[0]);
> > +		if (!nr)
> > +			break;
> > +
> > +		path->slots[0] = 0;
> > +		ret = btrfs_del_items(trans, root, path, 0, nr);
> > +		if (ret)
> > +			goto out;
> > +
> > +		btrfs_release_path(path);
> > +	}
> > +
> > +	ret = 0;
> > +out:
> > +	btrfs_free_path(path);
> > +	return ret;
> > +}
> > +
> > +int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info)
> > +{
> > +	struct btrfs_trans_handle *trans;
> > +	struct btrfs_root *tree_root = fs_info->tree_root;
> > +	struct btrfs_root *free_space_root = fs_info->free_space_root;
> > +	int ret;
> > +	u64 features;
> > +
> > +	trans = btrfs_start_transaction(tree_root, 0);
> > +	if (IS_ERR(trans))
> > +		return PTR_ERR(trans);
> > +
> > +
> > +	features = btrfs_super_compat_ro_flags(fs_info->super_copy);
> > +	features &= ~(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID |
> > +		      BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE);
> > +	btrfs_set_super_compat_ro_flags(fs_info->super_copy, features);
> > +	fs_info->free_space_root = NULL;
> > +
> > +	ret = clear_free_space_tree(trans, free_space_root);
> > +	if (ret)
> > +		goto abort;
> > +
> > +	ret = btrfs_del_root(trans, tree_root, &free_space_root->root_key);
> > +	if (ret)
> > +		goto abort;
> > +
> > +	list_del(&free_space_root->dirty_list);
> > +
> > +	clean_tree_block(trans, tree_root, free_space_root->node);
> 
> Better catch the return value.
> 
> > +	btrfs_free_tree_block(trans, free_space_root, free_space_root->node,
> > +			      0, 1);
> 
> Here too.

Oh yeah these two return void in the kernel, but I'll fix it here.

> > +
> > +	free_extent_buffer(free_space_root->node);
> > +	free_extent_buffer(free_space_root->commit_root);
> > +	kfree(free_space_root);
> > +
> > +	ret = btrfs_commit_transaction(trans, tree_root);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +
> > +abort:
> 
> This reminds me again that we really need a btrfs_abort_transaction() in
> btrfs-progs.
> (And I always forget to implement it)
> 
> > +	return ret;
> > +}
> > +
> >  static int load_free_space_bitmaps(struct btrfs_fs_info *fs_info,
> >  				   struct btrfs_block_group_cache *block_group,
> >  				   struct btrfs_path *path,
> > diff --git a/free-space-tree.h b/free-space-tree.h
> > index 7529a46..4845f13 100644
> > --- a/free-space-tree.h
> > +++ b/free-space-tree.h
> > @@ -19,6 +19,7 @@
> >  #ifndef __BTRFS_FREE_SPACE_TREE_H__
> >  #define __BTRFS_FREE_SPACE_TREE_H__
> > 
> > +int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info);
> >  int load_free_space_tree(struct btrfs_fs_info *fs_info,
> >  			 struct btrfs_block_group_cache *block_group);
> > 
> > diff --git a/root-tree.c b/root-tree.c
> > index cca424e..c660447 100644
> > --- a/root-tree.c
> > +++ b/root-tree.c
> > @@ -143,6 +143,28 @@ int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root
> >  	return ret;
> >  }
> > 
> > +/* drop the root item for 'key' from 'root' */
> > +int btrfs_del_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> > +		   struct btrfs_key *key)
> > +{
> > +	struct btrfs_path *path;
> > +	int ret;
> > +
> > +	path = btrfs_alloc_path();
> > +	if (!path)
> > +		return -ENOMEM;
> > +	ret = btrfs_search_slot(trans, root, key, path, -1, 1);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	BUG_ON(ret != 0);
> 
> Better to avoid BUG_ON().
> 
> Return -ENOENT seems good enough.

Copied this straight from the kernel, but you're right, no reason to
crash progs.

Thanks for the review!

> Thanks,
> Qu
> 
> > +
> > +	ret = btrfs_del_item(trans, root, path);
> > +out:
> > +	btrfs_free_path(path);
> > +	return ret;
> > +}
> > +
> >  /*
> >   * add a btrfs_root_ref item.  type is either BTRFS_ROOT_REF_KEY
> >   * or BTRFS_ROOT_BACKREF_KEY.
> > 
> 
> 

-- 
Omar

Re: [PATCH 3/6] btrfs-progs: add OPEN_CTREE_INVALIDATE_FST flag

[David Sterba]

On Mon, Nov 14, 2016 at 08:22:56AM -0800, Omar Sandoval wrote:
> > > --- a/disk-io.c
> > > +++ b/disk-io.c
> > > @@ -904,7 +904,7 @@ free_all:
> > >  	return NULL;
> > >  }
> > > 
> > > -int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, int writable)
> > > +int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, unsigned flags)
> > 
> > IIRC kernel checkpatch will warn on single "unsigned", as it's recommended
> > to use "unsigned int".
> 
> I was going for consistency with the rest of disk-io.c, but I can fix
> it.

Not needed IMHO, we don't use unsinged int/unsigned consistently.