On 29.09.2017 04:36, Qu Wenruo wrote:
> Use inline function to replace macro since we don't need
> stringification.
> (Macro still exist until all caller get updated)
>
> And add more info about the error.
>
> For nr_items error, report if it's too large or too small, and output
> valid value range.
>
> For blk pointer, added a new alignment checker.
>
> For key order, also output the next key to make the problem more
> obvious.
>
> Signed-off-by: Qu Wenruo <quwenruo.btrfs@xxxxxxx>
> ---
> fs/btrfs/tree-checker.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 61 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 301243a69dea..a51f2503acc4 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -37,6 +37,48 @@
> btrfs_header_level(eb) == 0 ? "leaf" : "node", \
> reason, btrfs_header_bytenr(eb), root->objectid, slot)
>
> +/*
> + * Error message should follow the format below:
> + * corrupt <type>: <identifier>, <reason>[, <bad_value>]
> + *
> + * @type: Either leaf or node
> + * @identifier: The necessary info to locate the leaf/node.
> + * It's recommened to decode key.objecitd/offset if it's
> + * meaningful.
> + * @reason: What's wrong
> + * @bad_value: Optional, it's recommened to output bad value and its
> + * expected value (range).
> + *
> + * Since comma is used to separate the components, only SPACE is allowed
> + * inside each component.
> + */
> +
> +/*
> + * Append the generic "corrupt leaf/node root=%llu block=%llu slot=%d: " to
> + * @fmt.
> + * Allowing user to customize their output.
> + */
> +__printf(4, 5)
> +static void generic_err(const struct btrfs_root *root,
> + const struct extent_buffer *eb,
> + int slot, const char *fmt, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, fmt);
> +
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + btrfs_crit(root->fs_info,
> + "corrupt %s: root=%llu block=%llu slot=%d, %pV",
> + btrfs_header_level(eb) == 0 ? "leaf" : "node",
> + root->objectid, btrfs_header_bytenr(eb), slot,
> + &vaf);
> + va_end(args);
> +}
> +
> static int check_extent_data_item(struct btrfs_root *root,
> struct extent_buffer *leaf,
> struct btrfs_key *key, int slot)
> @@ -282,8 +324,10 @@ int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
>
> if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(root->fs_info)) {
> btrfs_crit(root->fs_info,
> - "corrupt node: block %llu root %llu nritems %lu",
> - node->start, root->objectid, nr);
> + "corrupt node: root=%llu block=%llu, nritems too %s, have %lu expect range [1,%u]",
> + root->objectid, node->start,
> + nr == 0 ? "small" : "large", nr,
> + BTRFS_NODEPTRS_PER_BLOCK(root->fs_info));
> return -EIO;
This is separate from this patch but :
Why not EUCLEAN, could we get this error because of corrupted data and
not necessarily EIO ? Your other patches consistently use EUCLEAN ?
> }
>
> @@ -293,13 +337,26 @@ int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
> btrfs_node_key_to_cpu(node, &next_key, slot + 1);
>
> if (!bytenr) {
> - CORRUPT("invalid item slot", node, root, slot);
> + generic_err(root, node, slot,
> + "invalid node pointer, have %llu shouldn't be 0",
> + bytenr);
nit: Perhaps just say "Invalid null node pointer", if we trigger this
assert it means bytenr is 0 so I see no reason why we should be doing
any special formatting. It's not a big deal so might not be worth it a
resend unless there are other comments.
> ret = -EIO;
Ditto w.r.t EIO ?
> goto out;
> }
> + if (!IS_ALIGNED(bytenr, root->fs_info->sectorsize)) {
> + generic_err(root, node, slot,
> + "unaligned pointer, have %llu should be aligned to %u",
> + bytenr, root->fs_info->sectorsize);
> + ret = -EUCLEAN;
> + goto out;
> + }
>
> if (btrfs_comp_cpu_keys(&key, &next_key) >= 0) {
> - CORRUPT("bad key order", node, root, slot);
> + generic_err(root, node, slot,
> + "bad key order, current key (%llu %u %llu) next key (%llu %u %llu)",
> + key.objectid, key.type, key.offset,
> + next_key.objectid, next_key.type,
> + next_key.offset);
> ret = -EIO;
Ditto w.r.t return code?
> goto out;
> }
>
--
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
