btrfs-progs: better space_cache=v2 support
Subsystems (2)
cover btrfs-progs: better space_cache=v2 support
From: Omar Sandoval <[email protected]>
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(-)
Signature · 1 line
2.10.2
PATCH 1/6 btrfs-progs: add the FREE_SPACE_TREE_VALID compat_ro bit definition
From: Omar Sandoval <[email protected]>
This is just the definition; we don't support it yet.
Signed-off-by Omar Sandoval <[email protected]>
--- ctree.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
ctree.h +13 -0
@@ -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 \
Signature · 1 line
2.10.2
PATCH 2/6 btrfs-progs: format FREE_SPACE_TREE{,_VALID} nicely in dump-super
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
Signed-off-by Omar Sandoval <[email protected]>
--- cmds-inspect-dump-super.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
cmds-inspect-dump-super.c +24 -0
@@ -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));
Signature · 1 line
2.10.2
PATCH 3/6 btrfs-progs: add OPEN_CTREE_INVALIDATE_FST flag
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
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 <[email protected]>
--- chunk-recover.c | 2 +- disk-io.c | 28 ++++++++++++++++++---------- disk-io.h | 8 +++++++- 3 files changed, 26 insertions(+), 12 deletions(-)
chunk-recover.c +1 -1
@@ -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;
disk-io.c +18 -10
@@ -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;
disk-io.h +7 -1
@@ -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);
Signature · 1 line
2.10.2
Re: [PATCH 3/6] btrfs-progs: add OPEN_CTREE_INVALIDATE_FST flag
Quoted 5 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
If this flag is passed to
open_ctree(), we'll clear theFREE_SPACE_TREE_VALIDcompat_robit. 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 <[email protected]>
Quoted 26 lines
Signed-off-by Omar Sandoval <[email protected]>--- chunk-recover.c | 2 +- disk-io.c | 28 ++++++++++++++++++---------- disk-io.h | 8 +++++++- 3 files changed, 26 insertions(+), 12 deletions(-)chunk-recover.c +1
-1@@ -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;disk-io.c +0
-0
disk-io.c free_all: · line 907
Quoted 5 lines
disk-io.c +1
-1@@ -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
Quoted 70 lines
disk-io.c +17
-9{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;disk-io.h +7
-1@@ -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);
A message that is not in the archive (sent off-list or not yet received): 20161114162256.GA22223@vader
Re: [PATCH 3/6] btrfs-progs: add OPEN_CTREE_INVALIDATE_FST flag
Quoted 6 lines · Omar Sandoval
Quoted 6 lines
Quoted 6 lines
disk-io.c +0
-0@@ -904,7 +904,7 @@ free_all:return NULL ; }
Quoted 8 lines
Quoted 5 lines
disk-io.c free_all: · line 907
Quoted 2 lines
disk-io.c +1
-1-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.
PATCH 4/6 btrfs-progs: add btrfs_clear_free_space_tree() from the kernel
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
Signed-off-by Omar Sandoval <[email protected]>
--- 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(+)
ctree.h +6 -0
@@ -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);
extent-tree.c +10 -0
@@ -2467,6 +2467,16 @@ static int del_pending_extents(struct btrfs_trans_handle *trans, structreturn 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 */
free-space-tree.c +87 -0
99 lines not on this page · raw message
free-space-tree.h +1 -0
@@ -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);
root-tree.c +22 -0
@@ -143,6 +143,28 @@ int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_rootreturn 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.
Signature · 1 line
2.10.2
Re: [PATCH 4/6] btrfs-progs: add btrfs_clear_free_space_tree() from the kernel
Quoted 3 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
Signed-off-by Omar Sandoval <[email protected]>
Only small nits about the BUG_ON() and return value. Despite that, feel free to add my reviewed-by:
Reviewed-by Qu Wenruo <[email protected]>
Quoted 39 lines
--- 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(+)ctree.h +6
-0@@ -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);extent-tree.c +0
-0@@ -2467,6 +2467,16 @@ static int del_pending_extents(struct btrfs_trans_handle *trans, structreturn err; }
extent-tree.c static int del_pending_extents(struct btrfs_trans_handle *trans, struct · line 2478
Quoted 10 lines
extent-tree.c +9
-0+ +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.
Quoted 79 lines
extent-tree.c +1
-0+/* * remove an extent from the root, returns 0 on success */ free-space-tree.c +60
-0@@ -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);
free-space-tree.c static int free_space_test_bit(struct btrfs_block_group_cache *block_group, · line 139
Quoted 10 lines
free-space-tree.c +10
-0+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.
free-space-tree.c static int free_space_test_bit(struct btrfs_block_group_cache *block_group, · line 141
Quoted 2 lines
free-space-tree.c +2
-0+btrfs_free_tree_block (trans, free_space_root, free_space_root->node, +0 ,1 );
Here too.
free-space-tree.c static int free_space_test_bit(struct btrfs_block_group_cache *block_group, · line 153
Quoted 12 lines
free-space-tree.c +12
-0+ +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)
Quoted 31 lines
free-space-tree.c +3
-0+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,free-space-tree.h +1
-0@@ -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);root-tree.c +5
-0@@ -143,6 +143,28 @@ int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_rootreturn 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;
root-tree.c int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root · line 160
Quoted 10 lines
root-tree.c +10
-0+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
Quoted 11 lines
root-tree.c +7
-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.
Re: [PATCH 4/6] btrfs-progs: add btrfs_clear_free_space_tree() from the kernel
Quoted 63 lines · Qu Wenruo
Quoted 3 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
Signed-off-by Omar Sandoval <[email protected]>Only small nits about the
BUG_ON()and return value. Despite that, feel free to add my reviewed-by:Reviewed-by Qu Wenruo <[email protected]>Quoted 39 lines
--- 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(+)ctree.h +6
-0@@ -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);extent-tree.c +0
-0@@ -2467,6 +2467,16 @@ static int del_pending_extents(struct btrfs_trans_handle *trans, structreturn err; }extent-tree.c static int del_pending_extents(struct btrfs_trans_handle *trans, struct · line 2478
Quoted 10 lines
extent-tree.c +9
-0+ +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.
Quoted 1 line
Better return it.
Will fix.
Quoted 92 lines
Quoted 79 lines
extent-tree.c +1
-0+/* * remove an extent from the root, returns 0 on success */ free-space-tree.c +60
-0@@ -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);free-space-tree.c static int free_space_test_bit(struct btrfs_block_group_cache *block_group, · line 139
Quoted 10 lines
free-space-tree.c +10
-0+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.
Quoted 4 lines
free-space-tree.c static int free_space_test_bit(struct btrfs_block_group_cache *block_group, · line 141
Quoted 2 lines
free-space-tree.c +2
-0+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.
Quoted 61 lines
free-space-tree.c static int free_space_test_bit(struct btrfs_block_group_cache *block_group, · line 153
Quoted 12 lines
free-space-tree.c +12
-0+ +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)Quoted 31 lines
free-space-tree.c +3
-0+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,free-space-tree.h +1
-0@@ -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);root-tree.c +5
-0@@ -143,6 +143,28 @@ int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_rootreturn 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;root-tree.c int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root · line 160
Quoted 10 lines
root-tree.c +10
-0+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().
Quoted 1 line
Return
-ENOENTseems good enough.
Copied this straight from the kernel, but you're right, no reason to crash progs.
Thanks for the review!
Quoted 16 lines
Thanks,
QuQuoted 11 lines
root-tree.c +7
-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.
Signature · 1 line
Omar
PATCH 5/6 btrfs-progs: implement btrfs check --clear-space-cache v2
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
Signed-off-by Omar Sandoval <[email protected]>
--- Documentation/btrfs-check.asciidoc | 14 +++++++++----- cmds-check.c | 34 +++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 14 deletions(-)
Documentation/btrfs-check.asciidoc +9 -5
@@ -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
cmds-check.c +25 -9
@@ -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; }/*
Signature · 1 line
2.10.2
Re: [PATCH 5/6] btrfs-progs: implement btrfs check --clear-space-cache v2
Quoted 3 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
Signed-off-by Omar Sandoval <[email protected]>
Reviewed-by Qu Wenruo <[email protected]>
Quoted 18 lines
--- Documentation/btrfs-check.asciidoc | 14 +++++++++----- cmds-check.c | 34 +++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 14 deletions(-)Documentation/btrfs-check.asciidoc +0
-4@@ -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
Documentation/btrfs-check.asciidoc superblock is damaged. · line 92
Quoted 10 lines
Documentation/btrfs-check.asciidoc +9
-1-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
Quoted 74 lines
Documentation/btrfs-check.asciidoc +0
-0DANGEROUS OPTIONScmds-check.c +25
-9@@ -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; }/*
PATCH 6/6 btrfs-progs: document space_cache=v2 more thoroughly
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
Signed-off-by Omar Sandoval <[email protected]>
--- Documentation/btrfs-man5.asciidoc | 43 +++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-)
Documentation/btrfs-man5.asciidoc +25 -18
@@ -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*::
Signature · 1 line
2.10.2