On Fri, Sep 29, 2017 at 06:48:46AM +0000, 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, and replace EIO with EUCLEAN.
> 
> 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.
      ^^^
block?

> 
> For key order, also output the next key to make the problem more
> obvious.
> 
> Signed-off-by: Qu Wenruo <quwenruo.bt...@gmx.com>
> ---
>  fs/btrfs/tree-checker.c | 71 
> ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 64 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 301243a69dea..94acf3f5d6fd 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,9 +324,11 @@ 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);
> -             return -EIO;
> +                     "corrupt node: root=%llu block=%llu, nritems too %s, 
> have %lu expect range [1,%u]",

long strings overflowing 80 columns are allowed to be un-indented to the left

> +                        root->objectid, node->start,
> +                        nr == 0 ? "small" : "large", nr,
> +                        BTRFS_NODEPTRS_PER_BLOCK(root->fs_info));
> +             return -EUCLEAN;
>       }
>  
>       for (slot = 0; slot < nr - 1; slot++) {
> @@ -293,14 +337,27 @@ 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);
> -                     ret = -EIO;
> +                     generic_err(root, node, slot,
> +                             "invalid node pointer, have %llu shouldn't be 
> 0",
> +                             bytenr);
> +                     ret = -EUCLEAN;
> +                     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);
> -                     ret = -EIO;
> +                     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 = -EUCLEAN;
>                       goto out;
>               }
>       }

Other than the nits, patch looks good.
--
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

Reply via email to