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