Re: Btrfs: add a extent ref verify tool

[Geert Uytterhoeven]

On Tue, Nov 14, 2017 at 11:46 PM, Linux Kernel Mailing List
<linux-ker...@vger.kernel.org> wrote:
> Web:
> https://git.kernel.org/torvalds/c/fd708b81d972a0714b02a60eb4792fdbf15868c4
> Commit: fd708b81d972a0714b02a60eb4792fdbf15868c4
> Parent: 84f7d8e6242ceb377c7af10a7133c653cc7fea5f
> Refname:refs/heads/master
> Author: Josef Bacik <jo...@toxicpanda.com>
> AuthorDate: Fri Sep 29 15:43:50 2017 -0400
> Committer:  David Sterba <dste...@suse.com>
> CommitDate: Mon Oct 30 12:28:00 2017 +0100
>
> Btrfs: add a extent ref verify tool
>
> We were having corruption issues that were tied back to problems with
> the extent tree.  In order to track them down I built this tool to try
> and find the culprit, which was pretty successful.  If you compile with
> this tool on it will live verify every ref update that the fs makes and
> make sure it is consistent and valid.  I've run this through with
> xfstests and haven't gotten any false positives.  Thanks,
>
> Signed-off-by: Josef Bacik <jba...@fb.com>
> Reviewed-by: David Sterba <dste...@suse.com>
> [ update error messages, add fixup from Dan Carpenter to handle errors
>   of read_tree_block ]
> Signed-off-by: David Sterba <dste...@suse.com>

> --- /dev/null
> +++ b/fs/btrfs/ref-verify.c

> +static int process_extent_item(struct btrfs_fs_info *fs_info,
> +  struct btrfs_path *path, struct btrfs_key *key,
> +  int slot, int *tree_block_level)
> +{
> +   struct btrfs_extent_item *ei;
> +   struct btrfs_extent_inline_ref *iref;
> +   struct btrfs_extent_data_ref *dref;
> +   struct btrfs_shared_data_ref *sref;
> +   struct extent_buffer *leaf = path->nodes[0];
> +   u32 item_size = btrfs_item_size_nr(leaf, slot);
> +   unsigned long end, ptr;
> +   u64 offset, flags, count;
> +   int type, ret;

With gcc-4.1.2:

warning: ‘ret’ may be used uninitialized in this function

> +
> +   ei = btrfs_item_ptr(leaf, slot, struct btrfs_extent_item);
> +   flags = btrfs_extent_flags(leaf, ei);
> +
> +   if ((key->type == BTRFS_EXTENT_ITEM_KEY) &&
> +   flags & BTRFS_EXTENT_FLAG_TREE_BLOCK) {
> +   struct btrfs_tree_block_info *info;
> +
> +   info = (struct btrfs_tree_block_info *)(ei + 1);
> +   *tree_block_level = btrfs_tree_block_level(leaf, info);
> +   iref = (struct btrfs_extent_inline_ref *)(info + 1);
> +   } else {
> +   if (key->type == BTRFS_METADATA_ITEM_KEY)
> +   *tree_block_level = key->offset;
> +   iref = (struct btrfs_extent_inline_ref *)(ei + 1);
> +   }
> +
> +   ptr = (unsigned long)iref;
> +   end = (unsigned long)ei + item_size;
> +   while (ptr < end) {

Is this loop guaranteed to run at least once?
If not:
  1. uninitialized ret will be returned,
  2. To which value should ret be preinitialized? 0 or an error?

As I understand this is a verification tool, it should be implemented
using defensive programming.

> +   iref = (struct btrfs_extent_inline_ref *)ptr;
> +   type = btrfs_extent_inline_ref_type(leaf, iref);
> +   offset = btrfs_extent_inline_ref_offset(leaf, iref);
> +   switch (type) {
> +   case BTRFS_TREE_BLOCK_REF_KEY:
> +   ret = add_tree_block(fs_info, offset, 0, 
> key->objectid,
> +*tree_block_level);
> +   break;
> +   case BTRFS_SHARED_BLOCK_REF_KEY:
> +   ret = add_tree_block(fs_info, 0, offset, 
> key->objectid,
> +*tree_block_level);
> +   break;
> +   case BTRFS_EXTENT_DATA_REF_KEY:
> +   dref = (struct btrfs_extent_data_ref 
> *)(>offset);
> +   ret = add_extent_data_ref(fs_info, leaf, dref,
> + key->objectid, key->offset);
> +   break;
> +   case BTRFS_SHARED_DATA_REF_KEY:
> +   sref = (struct btrfs_shared_data_ref *)(iref + 1);
> +   count = btrfs_shared_data_ref_count(leaf, sref);
> +   ret = add_shared_data_ref(fs_info, offset, count,
> + key->objectid, key->offset);
> +   break;
> +   default:
> +   btrfs_err(fs_info, "invalid key type in iref");
> +

Re: [PATCH 06/21] Btrfs: add a extent ref verify tool

[David Sterba]

On Fri, Sep 29, 2017 at 03:43:50PM -0400, Josef Bacik wrote:
> We were having corruption issues that were tied back to problems with the 
> extent
> tree.  In order to track them down I built this tool to try and find the
> culprit, which was pretty successful.  If you compile with this tool on it 
> will
> live verify every ref update that the fs makes and make sure it is consistent
> and valid.  I've run this through with xfstests and haven't gotten any false
> positives.  Thanks,
> 
> Signed-off-by: Josef Bacik 

Reviewed-by: David Sterba 

I've fixed the error messages, they should not start with an uppercase
letter and must not end with \n, and can be un-indented so they fit as
much as possible under 80.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/21] Btrfs: add a extent ref verify tool

[Josef Bacik]

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  I've run this through with xfstests and haven't gotten any false
positives.  Thanks,

Signed-off-by: Josef Bacik 
---
 fs/btrfs/Kconfig   |   11 +
 fs/btrfs/Makefile  |1 +
 fs/btrfs/ctree.h   |5 +
 fs/btrfs/disk-io.c |6 +
 fs/btrfs/extent-tree.c |   22 ++
 fs/btrfs/ref-verify.c  | 1012 
 fs/btrfs/ref-verify.h  |   59 +++
 7 files changed, 1116 insertions(+)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index a26c63b4ad68..2e558227931a 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -91,3 +91,14 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool "Btrfs with the ref verify tool compiled in"
+   depends on BTRFS_FS
+   default n
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 962a95aefb81..72c60f54f962 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a7484a744ef0..4ffbe9f07cf7 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1096,6 +1096,11 @@ struct btrfs_fs_info {
u32 nodesize;
u32 sectorsize;
u32 stripesize;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+#endif
 };
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1307907e19d8..778dc7682966 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -50,6 +50,7 @@
 #include "sysfs.h"
 #include "qgroup.h"
 #include "compression.h"
+#include "ref-verify.h"
 
 #ifdef CONFIG_X86
 #include 
@@ -2776,6 +2777,7 @@ int open_ctree(struct super_block *sb,
/* readahead state */
INIT_RADIX_TREE(_info->reada_tree, GFP_NOFS & ~__GFP_DIRECT_RECLAIM);
spin_lock_init(_info->reada_lock);
+   btrfs_init_ref_verify(fs_info);
 
fs_info->thread_pool_size = min_t(unsigned long,
  num_online_cpus() + 2, 8);
@@ -3195,6 +3197,9 @@ int open_ctree(struct super_block *sb,
if (ret)
goto fail_trans_kthread;
 
+   if (btrfs_build_ref_tree(fs_info))
+   btrfs_err(fs_info, "BTRFS: couldn't build ref tree\n");
+
/* do not make disk changes in broken FS or nologreplay is given */
if (btrfs_super_log_root(disk_super) != 0 &&
!btrfs_test_opt(fs_info, NOLOGREPLAY)) {
@@ -4060,6 +4065,7 @@ void close_ctree(struct btrfs_fs_info *fs_info)
cleanup_srcu_struct(_info->subvol_srcu);
 
btrfs_free_stripe_hash_table(fs_info);
+   btrfs_free_ref_cache(fs_info);
 
__btrfs_free_block_rsv(root->orphan_block_rsv);
root->orphan_block_rsv = NULL;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 2df22cae45b1..00d86c8afaef 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -39,6 +39,7 @@
 #include "math.h"
 #include "sysfs.h"
 #include "qgroup.h"
+#include "ref-verify.h"
 
 #undef SCRAMBLE_DELAYED_REFS
 
@@ -2189,6 +2190,9 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
BUG_ON(owner < BTRFS_FIRST_FREE_OBJECTID &&
   root_objectid == BTRFS_TREE_LOG_OBJECTID);
 
+   btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, root_objectid,
+  owner, offset, BTRFS_ADD_DELAYED_REF);
+
if (owner < BTRFS_FIRST_FREE_OBJECTID) {
ret = btrfs_add_delayed_tree_ref(fs_info, trans, bytenr,
 num_bytes, parent,
@@ -7223,6 +7227,10 @@ void btrfs_free_tree_block(struct btrfs_trans_handle 
*trans,
if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
int old_ref_mod, new_ref_mod;
 
+   btrfs_ref_tree_mod(root, buf->start, buf->len, 

Re: [PATCH][v3] Btrfs: add a extent ref verify tool

[David Sterba]

On Fri, Sep 01, 2017 at 03:09:30AM -0400, jo...@toxicpanda.com wrote:
> From: Josef Bacik 
> 
> We were having corruption issues that were tied back to problems with the 
> extent
> tree.  In order to track them down I built this tool to try and find the
> culprit, which was pretty successful.  If you compile with this tool on it 
> will
> live verify every ref update that the fs makes and make sure it is consistent
> and valid.  I've run this through with xfstests and haven't gotten any false
> positives.  Thanks,
> 
> Signed-off-by: Josef Bacik 
> ---
> v2->v3:
> - fix use after free in an error case

Can you please split the patch into more parts? This is just too big.

* the parameter changes, one per patch
* add ref-veriy.*
* new mount option and enabling the ref-verify

Apart from the specific comments written inline, here's list of thing
that I saw repeatd in several places:

printk instead of the btrfs_* error helpers - the bare printk will not
tell youw which filesystem is affected so it's not helpful when there
are several btrfs filesytems active

please don't split long strings

please don't use %Lu or %Ld format string, %llu

GFP_NOFS -- it's used on the open_ctree path so GFP_KERNEL is the right
and safe flag

misc small coding style issues


I'm half way through reviewing it from the functional side, so far it
looks good.

>  fs/btrfs/Kconfig   |   10 +
>  fs/btrfs/Makefile  |1 +
>  fs/btrfs/ctree.c   |2 +-
>  fs/btrfs/ctree.h   |   14 +-
>  fs/btrfs/disk-io.c |   15 +
>  fs/btrfs/extent-tree.c |   44 ++-
>  fs/btrfs/file.c|   10 +-
>  fs/btrfs/inode.c   |9 +-
>  fs/btrfs/ioctl.c   |2 +-
>  fs/btrfs/ref-verify.c  | 1019 
> 
>  fs/btrfs/ref-verify.h  |   51 +++
>  fs/btrfs/relocation.c  |   14 +-
>  fs/btrfs/super.c   |   17 +
>  fs/btrfs/tree-log.c|2 +-
>  14 files changed, 1178 insertions(+), 32 deletions(-)
>  create mode 100644 fs/btrfs/ref-verify.c
>  create mode 100644 fs/btrfs/ref-verify.h
> 
> diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
> index 80e9c18..77d7f74 100644
> --- a/fs/btrfs/Kconfig
> +++ b/fs/btrfs/Kconfig
> @@ -89,3 +89,13 @@ config BTRFS_ASSERT
> any of the assertions trip.  This is meant for btrfs developers only.
>  
> If unsure, say N.
> +
> +config BTRFS_FS_REF_VERIFY
> + bool "Btrfs with the ref verify tool compiled in"
> + depends on BTRFS_FS

must be N by default

> + help
> +   Enable run-time extent reference verification instrumentation.  This
> +   is meant to be used by btrfs developers for tracking down extent
> +   reference problems or verifying they didn't break something.
> +
> +   If unsure, say N.
> diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
> index 128ce17..3172751 100644
> --- a/fs/btrfs/Makefile
> +++ b/fs/btrfs/Makefile
> @@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
> root-tree.o dir-item.o \
>  
>  btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
>  btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
> +btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
>  
>  btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
>   tests/extent-buffer-tests.o tests/btrfs-tests.o \
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 6d49db7..a4812ce 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -192,7 +192,7 @@ struct extent_buffer *btrfs_lock_root_node(struct 
> btrfs_root *root)
>   * tree until you end up with a lock on the root.  A locked buffer
>   * is returned, with a reference held.
>   */
> -static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root 
> *root)
> +struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
>  {
>   struct extent_buffer *eb;
>  
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index d49b045..4fa3ddd 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1098,6 +1098,12 @@ struct btrfs_fs_info {
>   u32 nodesize;
>   u32 sectorsize;
>   u32 stripesize;
> +
> +#ifdef CONFIG_BTRFS_FS_REF_VERIFY
> + spinlock_t ref_verify_lock;
> + struct rb_root block_tree;
> + bool ref_verify_enabled;

the on/off bit can be added to the fs_info::flags instead

> +#endif
>  };
>  
>  static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
> @@ -1336,6 +1342,7 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct 
> btrfs_fs_info *info)
>  #define BTRFS_MOUNT_FRAGMENT_METADATA(1 << 25)
>  #define BTRFS_MOUNT_FREE_SPACE_TREE  (1 << 26)
>  #define BTRFS_MOUNT_NOLOGREPLAY  (1 << 27)
> +#define BTRFS_MOUNT_REF_VERIFY   (1 << 28)
>  
>  #define BTRFS_DEFAULT_COMMIT_INTERVAL(30)
>  #define BTRFS_DEFAULT_MAX_INLINE (2048)
> @@ -2627,7 +2634,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle 
> *trans,
>  struct extent_buffer 

[PATCH][v3] Btrfs: add a extent ref verify tool

[josef]

From: Josef Bacik 

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  I've run this through with xfstests and haven't gotten any false
positives.  Thanks,

Signed-off-by: Josef Bacik 
---
v2->v3:
- fix use after free in an error case

 fs/btrfs/Kconfig   |   10 +
 fs/btrfs/Makefile  |1 +
 fs/btrfs/ctree.c   |2 +-
 fs/btrfs/ctree.h   |   14 +-
 fs/btrfs/disk-io.c |   15 +
 fs/btrfs/extent-tree.c |   44 ++-
 fs/btrfs/file.c|   10 +-
 fs/btrfs/inode.c   |9 +-
 fs/btrfs/ioctl.c   |2 +-
 fs/btrfs/ref-verify.c  | 1019 
 fs/btrfs/ref-verify.h  |   51 +++
 fs/btrfs/relocation.c  |   14 +-
 fs/btrfs/super.c   |   17 +
 fs/btrfs/tree-log.c|2 +-
 14 files changed, 1178 insertions(+), 32 deletions(-)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index 80e9c18..77d7f74 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -89,3 +89,13 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool "Btrfs with the ref verify tool compiled in"
+   depends on BTRFS_FS
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 128ce17..3172751 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6d49db7..a4812ce 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -192,7 +192,7 @@ struct extent_buffer *btrfs_lock_root_node(struct 
btrfs_root *root)
  * tree until you end up with a lock on the root.  A locked buffer
  * is returned, with a reference held.
  */
-static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 {
struct extent_buffer *eb;
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d49b045..4fa3ddd 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1098,6 +1098,12 @@ struct btrfs_fs_info {
u32 nodesize;
u32 sectorsize;
u32 stripesize;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+   bool ref_verify_enabled;
+#endif
 };
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
@@ -1336,6 +1342,7 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct 
btrfs_fs_info *info)
 #define BTRFS_MOUNT_FRAGMENT_METADATA  (1 << 25)
 #define BTRFS_MOUNT_FREE_SPACE_TREE(1 << 26)
 #define BTRFS_MOUNT_NOLOGREPLAY(1 << 27)
+#define BTRFS_MOUNT_REF_VERIFY (1 << 28)
 
 #define BTRFS_DEFAULT_COMMIT_INTERVAL  (30)
 #define BTRFS_DEFAULT_MAX_INLINE   (2048)
@@ -2627,7 +2634,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle 
*trans,
   struct extent_buffer *buf,
   u64 parent, int last_ref);
 int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
-u64 root_objectid, u64 owner,
+struct btrfs_root *root, u64 owner,
 u64 offset, u64 ram_bytes,
 struct btrfs_key *ins);
 int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,
@@ -2646,7 +2653,7 @@ int btrfs_set_disk_extent_flags(struct btrfs_trans_handle 
*trans,
u64 bytenr, u64 num_bytes, u64 flags,
int level, int is_data);
 int btrfs_free_extent(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info,
+ struct btrfs_root *root,
  u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
  u64 owner, u64 offset);
 
@@ -2658,7 +2665,7 @@ void btrfs_prepare_extent_commit(struct btrfs_fs_info 
*fs_info);
 int btrfs_finish_extent_commit(struct 

[PATCH][v2] Btrfs: add a extent ref verify tool

[josef]

From: Josef Bacik 

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  I've run this through with xfstests and haven't gotten any false
positives.  Thanks,

Signed-off-by: Josef Bacik 
---
v1->v2:
- fix compile errors caught by buildbot

 fs/btrfs/Kconfig   |   10 +
 fs/btrfs/Makefile  |1 +
 fs/btrfs/ctree.c   |2 +-
 fs/btrfs/ctree.h   |   14 +-
 fs/btrfs/disk-io.c |   15 +
 fs/btrfs/extent-tree.c |   44 ++-
 fs/btrfs/file.c|   10 +-
 fs/btrfs/inode.c   |9 +-
 fs/btrfs/ioctl.c   |2 +-
 fs/btrfs/ref-verify.c  | 1019 
 fs/btrfs/ref-verify.h  |   51 +++
 fs/btrfs/relocation.c  |   14 +-
 fs/btrfs/super.c   |   17 +
 fs/btrfs/tree-log.c|2 +-
 14 files changed, 1178 insertions(+), 32 deletions(-)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index 80e9c18..77d7f74 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -89,3 +89,13 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool "Btrfs with the ref verify tool compiled in"
+   depends on BTRFS_FS
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 128ce17..3172751 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6d49db7..a4812ce 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -192,7 +192,7 @@ struct extent_buffer *btrfs_lock_root_node(struct 
btrfs_root *root)
  * tree until you end up with a lock on the root.  A locked buffer
  * is returned, with a reference held.
  */
-static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 {
struct extent_buffer *eb;
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d49b045..4fa3ddd 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1098,6 +1098,12 @@ struct btrfs_fs_info {
u32 nodesize;
u32 sectorsize;
u32 stripesize;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+   bool ref_verify_enabled;
+#endif
 };
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
@@ -1336,6 +1342,7 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct 
btrfs_fs_info *info)
 #define BTRFS_MOUNT_FRAGMENT_METADATA  (1 << 25)
 #define BTRFS_MOUNT_FREE_SPACE_TREE(1 << 26)
 #define BTRFS_MOUNT_NOLOGREPLAY(1 << 27)
+#define BTRFS_MOUNT_REF_VERIFY (1 << 28)
 
 #define BTRFS_DEFAULT_COMMIT_INTERVAL  (30)
 #define BTRFS_DEFAULT_MAX_INLINE   (2048)
@@ -2627,7 +2634,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle 
*trans,
   struct extent_buffer *buf,
   u64 parent, int last_ref);
 int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
-u64 root_objectid, u64 owner,
+struct btrfs_root *root, u64 owner,
 u64 offset, u64 ram_bytes,
 struct btrfs_key *ins);
 int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,
@@ -2646,7 +2653,7 @@ int btrfs_set_disk_extent_flags(struct btrfs_trans_handle 
*trans,
u64 bytenr, u64 num_bytes, u64 flags,
int level, int is_data);
 int btrfs_free_extent(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info,
+ struct btrfs_root *root,
  u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
  u64 owner, u64 offset);
 
@@ -2658,7 +2665,7 @@ void btrfs_prepare_extent_commit(struct btrfs_fs_info 
*fs_info);
 int btrfs_finish_extent_commit(struct 

[PATCH] Btrfs: add a extent ref verify tool

[josef]

From: Josef Bacik 

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  I've run this through with xfstests and haven't gotten any false
positives.  Thanks,

Signed-off-by: Josef Bacik 
---
 fs/btrfs/Kconfig   |   10 +
 fs/btrfs/Makefile  |1 +
 fs/btrfs/ctree.h   |   13 +-
 fs/btrfs/disk-io.c |   15 +
 fs/btrfs/extent-tree.c |   44 ++-
 fs/btrfs/file.c|   10 +-
 fs/btrfs/inode.c   |9 +-
 fs/btrfs/ioctl.c   |2 +-
 fs/btrfs/ref-verify.c  | 1006 
 fs/btrfs/ref-verify.h  |   51 +++
 fs/btrfs/relocation.c  |   14 +-
 fs/btrfs/super.c   |   17 +
 fs/btrfs/tree-log.c|2 +-
 13 files changed, 1163 insertions(+), 31 deletions(-)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index 80e9c18..77d7f74 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -89,3 +89,13 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool "Btrfs with the ref verify tool compiled in"
+   depends on BTRFS_FS
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 128ce17..3172751 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d49b045..d50c21a 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1098,6 +1098,12 @@ struct btrfs_fs_info {
u32 nodesize;
u32 sectorsize;
u32 stripesize;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+   bool ref_verify_enabled;
+#endif
 };
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
@@ -1336,6 +1342,7 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct 
btrfs_fs_info *info)
 #define BTRFS_MOUNT_FRAGMENT_METADATA  (1 << 25)
 #define BTRFS_MOUNT_FREE_SPACE_TREE(1 << 26)
 #define BTRFS_MOUNT_NOLOGREPLAY(1 << 27)
+#define BTRFS_MOUNT_REF_VERIFY (1 << 28)
 
 #define BTRFS_DEFAULT_COMMIT_INTERVAL  (30)
 #define BTRFS_DEFAULT_MAX_INLINE   (2048)
@@ -2627,7 +2634,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle 
*trans,
   struct extent_buffer *buf,
   u64 parent, int last_ref);
 int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
-u64 root_objectid, u64 owner,
+struct btrfs_root *root, u64 owner,
 u64 offset, u64 ram_bytes,
 struct btrfs_key *ins);
 int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,
@@ -2646,7 +2653,7 @@ int btrfs_set_disk_extent_flags(struct btrfs_trans_handle 
*trans,
u64 bytenr, u64 num_bytes, u64 flags,
int level, int is_data);
 int btrfs_free_extent(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info,
+ struct btrfs_root *root,
  u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
  u64 owner, u64 offset);
 
@@ -2658,7 +2665,7 @@ void btrfs_prepare_extent_commit(struct btrfs_fs_info 
*fs_info);
 int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans,
   struct btrfs_fs_info *fs_info);
 int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
-struct btrfs_fs_info *fs_info,
+struct btrfs_root *root,
 u64 bytenr, u64 num_bytes, u64 parent,
 u64 root_objectid, u64 owner, u64 offset);
 
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4a41158..32215e5 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -50,6 +50,7 @@
 #include "sysfs.h"
 #include "qgroup.h"
 #include "compression.h"

Re: [PATCH] Btrfs: add a extent ref verify tool V2

[Josef Bacik]

On 05/08/2014 07:34 PM, Zach Brown wrote:

+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+int btrfs_build_ref_tree(struct btrfs_fs_info *fs_info);
+void btrfs_free_ref_cache(struct btrfs_fs_info *fs_info);
+int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
+  u64 parent, u64 ref_root, u64 owner, u64 offset,
+  int action);
+void btrfs_free_ref_tree_range(struct btrfs_fs_info *fs_info, u64 start,
+  u64 len);
+#else
+
+#define btrfs_free_ref_cache(fs_info) do { } while (0)
+#define btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, ref_root,  \
+  owner, offset, action) do { } while (0)
+#define btrfs_free_ref_tree_range(fs_info, start, len) do { } while (0)
+
+#endif /* CONFIG_BTRFS_FS_REF_VERIFY */
+#endif /* _REF_VERIFY__ */


Don't just omit the arguments when the config isn't enabled.  That can
let these calls bit rot over time as everyone build tests their changes
with the config disabled, as they will.

int derp;

...

btrfs_free_ref_cache(derp);

...

-   derp++;


Will have gcc warn that derp is unused.  They'll delete it and send out
the patch.  You'll later turn on the config and get undefined derp
warnings.

inline stubs are the best way out.  They'll still warn if the arg is
used undefined.  Putting (void)arg; in the macros doesn't warn in that
case.


Gcc doesn't complain when I build with it off but I can switch to inline 
stubs if that makes you feel better.  Thanks,


Josef

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Btrfs: add a extent ref verify tool V3

[Josef Bacik]

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  I've run this through with xfstests and haven't gotten any false
positives.  Thanks,

Signed-off-by: Josef Bacik jba...@fb.com
---
V2-V3:
-use inline stubs instead of defines for the off case

V1-V2:
-fixed unlock problems when hitting errors
-handle shared refs properly when building cache
-deal with tree log replay properly

 fs/btrfs/Kconfig   |  10 +
 fs/btrfs/Makefile  |   1 +
 fs/btrfs/ctree.c   |   2 +-
 fs/btrfs/ctree.h   |   7 +
 fs/btrfs/disk-io.c |  14 +
 fs/btrfs/extent-tree.c |  18 +
 fs/btrfs/ref-verify.c  | 994 +
 fs/btrfs/ref-verify.h  |  51 +++
 fs/btrfs/relocation.c  |   1 +
 9 files changed, 1097 insertions(+), 1 deletion(-)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index a66768e..1dfd411 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -88,3 +88,13 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool Btrfs with the ref verify tool compiled in
+   depends on BTRFS_FS
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 6d1d0b9..b566ef3 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cb1f56f..aa849e0 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -202,7 +202,7 @@ struct extent_buffer *btrfs_lock_root_node(struct 
btrfs_root *root)
  * tree until you end up with a lock on the root.  A locked buffer
  * is returned, with a reference held.
  */
-static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 {
struct extent_buffer *eb;
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 88d9504..96dae25 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1691,6 +1691,12 @@ struct btrfs_fs_info {
 
struct semaphore uuid_tree_rescan_sem;
unsigned int update_uuid_tree_gen:1;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+   bool ref_verify_enabled;
+#endif
 };
 
 struct btrfs_subvolume_writers {
@@ -3404,6 +3410,7 @@ void btrfs_set_item_key_safe(struct btrfs_root *root, 
struct btrfs_path *path,
 struct btrfs_key *new_key);
 struct extent_buffer *btrfs_root_node(struct btrfs_root *root);
 struct extent_buffer *btrfs_lock_root_node(struct btrfs_root *root);
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root);
 int btrfs_find_next_key(struct btrfs_root *root, struct btrfs_path *path,
struct btrfs_key *key, int lowest_level,
u64 min_trans);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3824c85..a822b62 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -50,6 +50,7 @@
 #include raid56.h
 #include sysfs.h
 #include qgroup.h
+#include ref-verify.h
 
 #ifdef CONFIG_X86
 #include asm/cpufeature.h
@@ -2295,6 +2296,11 @@ int open_ctree(struct super_block *sb,
 #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
fs_info-check_integrity_print_mask = 0;
 #endif
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spin_lock_init(fs_info-ref_verify_lock);
+   fs_info-block_tree = RB_ROOT;
+   fs_info-ref_verify_enabled = true;
+#endif
 
spin_lock_init(fs_info-balance_lock);
mutex_init(fs_info-balance_mutex);
@@ -2843,6 +2849,13 @@ retry_root_backup:
if (ret)
goto fail_trans_kthread;
 
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   if (btrfs_build_ref_tree(fs_info)) {
+   fs_info-ref_verify_enabled = false;
+   printk(KERN_ERR BTRFS: couldn't build ref tree\n);
+   }
+#endif
+
/* do not make disk changes in broken FS */
if (btrfs_super_log_root(disk_super) != 0) {
u64 bytenr = 

Re: [PATCH] Btrfs: add a extent ref verify tool V2

[Zach Brown]

On Fri, May 09, 2014 at 04:45:05PM -0400, Josef Bacik wrote:
 On 05/08/2014 07:34 PM, Zach Brown wrote:
 +#ifdef CONFIG_BTRFS_FS_REF_VERIFY
 +int btrfs_build_ref_tree(struct btrfs_fs_info *fs_info);
 +void btrfs_free_ref_cache(struct btrfs_fs_info *fs_info);
 +int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
 +  u64 parent, u64 ref_root, u64 owner, u64 offset,
 +  int action);
 +void btrfs_free_ref_tree_range(struct btrfs_fs_info *fs_info, u64 start,
 +  u64 len);
 +#else
 +
 +#define btrfs_free_ref_cache(fs_info) do { } while (0)
 +#define btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, ref_root,  
 \
 +  owner, offset, action) do { } while (0)
 +#define btrfs_free_ref_tree_range(fs_info, start, len) do { } while (0)
 +
 +#endif /* CONFIG_BTRFS_FS_REF_VERIFY */
 +#endif /* _REF_VERIFY__ */
 
 Don't just omit the arguments when the config isn't enabled.  That can
 let these calls bit rot over time as everyone build tests their changes
 with the config disabled, as they will.
 
  int derp;
 
  ...
 
  btrfs_free_ref_cache(derp);
 
  ...
 
 -derp++;
 
 
 Will have gcc warn that derp is unused.  They'll delete it and send out
 the patch.  You'll later turn on the config and get undefined derp
 warnings.
 
 inline stubs are the best way out.  They'll still warn if the arg is
 used undefined.  Putting (void)arg; in the macros doesn't warn in that
 case.
 
 Gcc doesn't complain when I build with it off but I can switch to inline
 stubs if that makes you feel better.  Thanks,

It's not about the code now.  It's about the code changing over time as
people only build it it disabled and break the build with it enabled.
This has happened to lots of interfaces in the kernel in the past.
This is why, for example, pr_devel() calls no_printk() instead of just
dropping the arguments on the floor.

- z
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Btrfs: add a extent ref verify tool V2

[Josef Bacik]

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  I've run this through with xfstests and haven't gotten any false
positives.  Thanks,

Signed-off-by: Josef Bacik jba...@fb.com
---
V1-V2:
-fixed unlock problems when hitting errors
-handle shared refs properly when building cache
-deal with tree log replay properly

 fs/btrfs/Kconfig   |  10 +
 fs/btrfs/Makefile  |   1 +
 fs/btrfs/ctree.c   |   2 +-
 fs/btrfs/ctree.h   |   7 +
 fs/btrfs/disk-io.c |  14 +
 fs/btrfs/extent-tree.c |  18 +
 fs/btrfs/ref-verify.c  | 994 +
 fs/btrfs/ref-verify.h  |  37 ++
 fs/btrfs/relocation.c  |   1 +
 9 files changed, 1083 insertions(+), 1 deletion(-)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index a66768e..1dfd411 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -88,3 +88,13 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool Btrfs with the ref verify tool compiled in
+   depends on BTRFS_FS
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 6d1d0b9..b566ef3 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cb1f56f..aa849e0 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -202,7 +202,7 @@ struct extent_buffer *btrfs_lock_root_node(struct 
btrfs_root *root)
  * tree until you end up with a lock on the root.  A locked buffer
  * is returned, with a reference held.
  */
-static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 {
struct extent_buffer *eb;
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 88d9504..96dae25 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1691,6 +1691,12 @@ struct btrfs_fs_info {
 
struct semaphore uuid_tree_rescan_sem;
unsigned int update_uuid_tree_gen:1;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+   bool ref_verify_enabled;
+#endif
 };
 
 struct btrfs_subvolume_writers {
@@ -3404,6 +3410,7 @@ void btrfs_set_item_key_safe(struct btrfs_root *root, 
struct btrfs_path *path,
 struct btrfs_key *new_key);
 struct extent_buffer *btrfs_root_node(struct btrfs_root *root);
 struct extent_buffer *btrfs_lock_root_node(struct btrfs_root *root);
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root);
 int btrfs_find_next_key(struct btrfs_root *root, struct btrfs_path *path,
struct btrfs_key *key, int lowest_level,
u64 min_trans);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3824c85..a822b62 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -50,6 +50,7 @@
 #include raid56.h
 #include sysfs.h
 #include qgroup.h
+#include ref-verify.h
 
 #ifdef CONFIG_X86
 #include asm/cpufeature.h
@@ -2295,6 +2296,11 @@ int open_ctree(struct super_block *sb,
 #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
fs_info-check_integrity_print_mask = 0;
 #endif
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spin_lock_init(fs_info-ref_verify_lock);
+   fs_info-block_tree = RB_ROOT;
+   fs_info-ref_verify_enabled = true;
+#endif
 
spin_lock_init(fs_info-balance_lock);
mutex_init(fs_info-balance_mutex);
@@ -2843,6 +2849,13 @@ retry_root_backup:
if (ret)
goto fail_trans_kthread;
 
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   if (btrfs_build_ref_tree(fs_info)) {
+   fs_info-ref_verify_enabled = false;
+   printk(KERN_ERR BTRFS: couldn't build ref tree\n);
+   }
+#endif
+
/* do not make disk changes in broken FS */
if (btrfs_super_log_root(disk_super) != 0) {
u64 bytenr = btrfs_super_log_root(disk_super);
@@ -3669,6 +3682,7 @@ int 

Re: [PATCH] Btrfs: add a extent ref verify tool V2

[Zach Brown]

 +#ifdef CONFIG_BTRFS_FS_REF_VERIFY
 +int btrfs_build_ref_tree(struct btrfs_fs_info *fs_info);
 +void btrfs_free_ref_cache(struct btrfs_fs_info *fs_info);
 +int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
 +u64 parent, u64 ref_root, u64 owner, u64 offset,
 +int action);
 +void btrfs_free_ref_tree_range(struct btrfs_fs_info *fs_info, u64 start,
 +u64 len);
 +#else
 +
 +#define btrfs_free_ref_cache(fs_info) do { } while (0)
 +#define btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, ref_root,
 \
 +owner, offset, action) do { } while (0)
 +#define btrfs_free_ref_tree_range(fs_info, start, len) do { } while (0)
 +
 +#endif /* CONFIG_BTRFS_FS_REF_VERIFY */
 +#endif /* _REF_VERIFY__ */

Don't just omit the arguments when the config isn't enabled.  That can
let these calls bit rot over time as everyone build tests their changes
with the config disabled, as they will.

int derp;

...

btrfs_free_ref_cache(derp);

...

-   derp++;


Will have gcc warn that derp is unused.  They'll delete it and send out
the patch.  You'll later turn on the config and get undefined derp
warnings.

inline stubs are the best way out.  They'll still warn if the arg is
used undefined.  Putting (void)arg; in the macros doesn't warn in that
case.

- z
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Btrfs: add a extent ref verify tool V2

[Josef Bacik]

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  This should only be used with a clean file system to start with and
then have the tests run as it doesn't lookup the actual shared refs on mount, so
it will get snapshots and things wrong.  This could be fixed in the future
easily, I just didn't need it for my particular test.  Thanks,

Signed-off-by: Josef Bacik jba...@fb.com
---
V1-V2:
-added something to drop block entries for a deleted block group, otherwise
 we'll OOM after lots of rebalances.

 fs/btrfs/Kconfig   |  10 +
 fs/btrfs/Makefile  |   1 +
 fs/btrfs/ctree.c   |   2 +-
 fs/btrfs/ctree.h   |   7 +
 fs/btrfs/disk-io.c |  14 +-
 fs/btrfs/extent-tree.c |  18 +
 fs/btrfs/ref-verify.c  | 951 +
 fs/btrfs/ref-verify.h  |  37 ++
 fs/btrfs/relocation.c  |   1 +
 9 files changed, 1039 insertions(+), 2 deletions(-)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index a66768e..1dfd411 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -88,3 +88,13 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool Btrfs with the ref verify tool compiled in
+   depends on BTRFS_FS
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index f341a98..ae837d2 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 9d89c16..71bbafe 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -202,7 +202,7 @@ struct extent_buffer *btrfs_lock_root_node(struct 
btrfs_root *root)
  * tree until you end up with a lock on the root.  A locked buffer
  * is returned, with a reference held.
  */
-static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 {
struct extent_buffer *eb;
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4253ab2..2277006 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1680,6 +1680,12 @@ struct btrfs_fs_info {
 
struct semaphore uuid_tree_rescan_sem;
unsigned int update_uuid_tree_gen:1;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+   bool ref_verify_enabled;
+#endif
 };
 
 struct btrfs_subvolume_writers {
@@ -3379,6 +3385,7 @@ void btrfs_set_item_key_safe(struct btrfs_root *root, 
struct btrfs_path *path,
 struct btrfs_key *new_key);
 struct extent_buffer *btrfs_root_node(struct btrfs_root *root);
 struct extent_buffer *btrfs_lock_root_node(struct btrfs_root *root);
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root);
 int btrfs_find_next_key(struct btrfs_root *root, struct btrfs_path *path,
struct btrfs_key *key, int lowest_level,
u64 min_trans);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a152a96..02ae4d1 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -49,6 +49,7 @@
 #include dev-replace.h
 #include raid56.h
 #include sysfs.h
+#include ref-verify.h
 
 #ifdef CONFIG_X86
 #include asm/cpufeature.h
@@ -2268,6 +2269,11 @@ int open_ctree(struct super_block *sb,
 #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
fs_info-check_integrity_print_mask = 0;
 #endif
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spin_lock_init(fs_info-ref_verify_lock);
+   fs_info-block_tree = RB_ROOT;
+   fs_info-ref_verify_enabled = true;
+#endif
 
spin_lock_init(fs_info-balance_lock);
mutex_init(fs_info-balance_mutex);
@@ -2895,7 +2901,12 @@ retry_root_backup:
 
if (sb-s_flags  MS_RDONLY)
return 0;
-
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   if (btrfs_build_ref_tree(fs_info)) {
+   fs_info-ref_verify_enabled = false;
+   printk(KERN_ERR BTRFS: couldn't build ref tree\n);
+   }
+#endif

[PATCH] Btrfs: add a extent ref verify tool

[Josef Bacik]

We were having corruption issues that were tied back to problems with the extent
tree.  In order to track them down I built this tool to try and find the
culprit, which was pretty successful.  If you compile with this tool on it will
live verify every ref update that the fs makes and make sure it is consistent
and valid.  This should only be used with a clean file system to start with and
then have the tests run as it doesn't lookup the actual shared refs on mount, so
it will get snapshots and things wrong.  This could be fixed in the future
easily, I just didn't need it for my particular test.  Thanks,

Signed-off-by: Josef Bacik jba...@fb.com
---
 fs/btrfs/Kconfig   |  10 +
 fs/btrfs/Makefile  |   1 +
 fs/btrfs/ctree.c   |   2 +-
 fs/btrfs/ctree.h   |   7 +
 fs/btrfs/disk-io.c |  14 +-
 fs/btrfs/extent-tree.c |  16 +
 fs/btrfs/ref-verify.c  | 892 +
 fs/btrfs/ref-verify.h  |  34 ++
 fs/btrfs/relocation.c  |   1 +
 9 files changed, 975 insertions(+), 2 deletions(-)
 create mode 100644 fs/btrfs/ref-verify.c
 create mode 100644 fs/btrfs/ref-verify.h

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index a66768e..1dfd411 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -88,3 +88,13 @@ config BTRFS_ASSERT
  any of the assertions trip.  This is meant for btrfs developers only.
 
  If unsure, say N.
+
+config BTRFS_FS_REF_VERIFY
+   bool Btrfs with the ref verify tool compiled in
+   depends on BTRFS_FS
+   help
+ Enable run-time extent reference verification instrumentation.  This
+ is meant to be used by btrfs developers for tracking down extent
+ reference problems or verifying they didn't break something.
+
+ If unsure, say N.
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index f341a98..ae837d2 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -13,6 +13,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
+btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
 
 btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
tests/extent-buffer-tests.o tests/btrfs-tests.o \
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 9d89c16..71bbafe 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -202,7 +202,7 @@ struct extent_buffer *btrfs_lock_root_node(struct 
btrfs_root *root)
  * tree until you end up with a lock on the root.  A locked buffer
  * is returned, with a reference held.
  */
-static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 {
struct extent_buffer *eb;
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4253ab2..2277006 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1680,6 +1680,12 @@ struct btrfs_fs_info {
 
struct semaphore uuid_tree_rescan_sem;
unsigned int update_uuid_tree_gen:1;
+
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spinlock_t ref_verify_lock;
+   struct rb_root block_tree;
+   bool ref_verify_enabled;
+#endif
 };
 
 struct btrfs_subvolume_writers {
@@ -3379,6 +3385,7 @@ void btrfs_set_item_key_safe(struct btrfs_root *root, 
struct btrfs_path *path,
 struct btrfs_key *new_key);
 struct extent_buffer *btrfs_root_node(struct btrfs_root *root);
 struct extent_buffer *btrfs_lock_root_node(struct btrfs_root *root);
+struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root);
 int btrfs_find_next_key(struct btrfs_root *root, struct btrfs_path *path,
struct btrfs_key *key, int lowest_level,
u64 min_trans);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a152a96..02ae4d1 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -49,6 +49,7 @@
 #include dev-replace.h
 #include raid56.h
 #include sysfs.h
+#include ref-verify.h
 
 #ifdef CONFIG_X86
 #include asm/cpufeature.h
@@ -2268,6 +2269,11 @@ int open_ctree(struct super_block *sb,
 #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
fs_info-check_integrity_print_mask = 0;
 #endif
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   spin_lock_init(fs_info-ref_verify_lock);
+   fs_info-block_tree = RB_ROOT;
+   fs_info-ref_verify_enabled = true;
+#endif
 
spin_lock_init(fs_info-balance_lock);
mutex_init(fs_info-balance_mutex);
@@ -2895,7 +2901,12 @@ retry_root_backup:
 
if (sb-s_flags  MS_RDONLY)
return 0;
-
+#ifdef CONFIG_BTRFS_FS_REF_VERIFY
+   if (btrfs_build_ref_tree(fs_info)) {
+   fs_info-ref_verify_enabled = false;
+   printk(KERN_ERR BTRFS: couldn't build ref tree\n);
+   }
+#endif
down_read(fs_info-cleanup_work_sem);
if ((ret = btrfs_orphan_cleanup(fs_info-fs_root)) ||
(ret =