Re: [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2017年10月11日 23:41, Nikolay Borisov wrote:


On  9.10.2017 04:51, Qu Wenruo wrote:
Enhance the output to print:
1) Reason
2) Bad value
    If reason can't explain enough
3) Good value (range)

Signed-off-by: Qu Wenruo <quwenruo.btrfs@xxxxxxx>
---
  fs/btrfs/tree-checker.c | 27 +++++++++++++++++++++------
  1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index b4ced8d3ce2a..7bba195ecc8b 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -233,8 +233,9 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
  			eb = btrfs_root_node(check_root);
  			/* if leaf is the root, then it's fine */
  			if (leaf != eb) {
-				CORRUPT("non-root leaf's nritems is 0",
-					leaf, check_root, 0);
+				generic_err(check_root, leaf, 0,
+					"invalid nritems, have %u shouldn't be 0 for non-root leaf",
+					nritems);

I'm a bit confused by what this error messages wants to convey. Even
reading the previous version with CORRUPT() it still didn't make sense.
So what we want to say here is we shouldn't have empty leaf nodes. So
Something along the line of "Unexpected empty leaf".

Yes, the error message is too fixed to follow the output format.

Why would the (leaf != eb) check not trigger, given that we call
btrfs_check_leaf when we now that the item is a leaf (level is 0 )?

What's the problem here? I didn't really get your point.

Did you mean leaf can't be tree root? Or empty tree root is not possible?


It's completely possible for a leaf to be a tree root.

All tree roots of a newly created (without --rootdir) is leaf.
Because the content of each tree is so few that one leaf can contain them all.


And it's also very possible to have empty tree, whose root (leaf) is also empty.
Still for a newly created btrfs, its csum tree is empty.
Its uuid tree is also empty.


But the only valid case for empty leaf is when it's a tree root.
So the code just checks it, and I didn't find anything wrong here.

Thanks,
Qu



  				free_extent_buffer(eb);
  				return -EUCLEAN;
  			}
@@ -265,7 +266,11 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
/* Make sure the keys are in the right order */
  		if (btrfs_comp_cpu_keys(&prev_key, &key) >= 0) {
-			CORRUPT("bad key order", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"bad key order, prev key (%llu %u %llu) current key (%llu %u %llu)",
+				prev_key.objectid, prev_key.type,
+				prev_key.offset, key.objectid, key.type,
+				key.offset);
  			return -EUCLEAN;
  		}
@@ -280,7 +285,10 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
  			item_end_expected = btrfs_item_offset_nr(leaf,
  								 slot - 1);
  		if (btrfs_item_end_nr(leaf, slot) != item_end_expected) {
-			CORRUPT("slot offset bad", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"discontinious item end, have %u expect %u",

s/discontinious/unexpected ?

+				btrfs_item_end_nr(leaf, slot),
+				item_end_expected);
  			return -EUCLEAN;
  		}
@@ -291,14 +299,21 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
  		 */
  		if (btrfs_item_end_nr(leaf, slot) >
  		    BTRFS_LEAF_DATA_SIZE(fs_info)) {
-			CORRUPT("slot end outside of leaf", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"slot end outside of leaf, have %u expect range [0, %u]",> +				btrfs_item_end_nr(leaf, slot),
+				BTRFS_LEAF_DATA_SIZE(fs_info));
  			return -EUCLEAN;
  		}
/* Also check if the item pointer overlaps with btrfs item. */
  		if (btrfs_item_nr_offset(slot) + sizeof(struct btrfs_item) >
  		    btrfs_item_ptr_offset(leaf, slot)) {
-			CORRUPT("slot overlap with its data", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"slot overlap with its data, item end %lu data start %lu",
+				btrfs_item_nr_offset(slot) +
+				sizeof(struct btrfs_item),
+				btrfs_item_ptr_offset(leaf, slot));
  			return -EUCLEAN;
  		}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux