On Tue, Aug 16, 2016 at 07:07:28PM +0200, David Sterba wrote:
> On Wed, Aug 03, 2016 at 09:57:08PM -0700, Liu Bo wrote:
> > Right now we treat leaf which has zero item as a valid one
> > because we could have an empty tree, that is, a root that is
> > also a leaf without any item, however, in the same case but
> > when the leaf is not a root, we can end up with hitting the
> > BUG_ON(1) in btrfs_extend_item() called by
> > setup_inline_extent_backref().
> >
> > This makes us check the situation as a corruption if leaf is
> > not its own root.
> >
> > Signed-off-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
> > ---
> > fs/btrfs/disk-io.c | 22 +++++++++++++++++++++-
> > 1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index a5a22be..dfaeb96 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -559,8 +559,28 @@ static noinline int check_leaf(struct btrfs_root *root,
> > u32 nritems = btrfs_header_nritems(leaf);
> > int slot;
> >
> > - if (nritems == 0)
> > + if (nritems == 0) {
> > + struct btrfs_root *r;
>
> Please don't use single letter variables.
OK.
>
> > +
> > + key.objectid = btrfs_header_owner(leaf);
> > + key.type = BTRFS_ROOT_ITEM_KEY;
> > + key.offset = -1ULL;
>
> While -1ULL is fine, the common style is the (u64)-1, please use that.
OK.
>
> > +
> > + r = btrfs_get_fs_root(root->fs_info, &key, false);
>
> I wonder how expensive that could be. check_leaf is called at the end of
> the read so it's just the lookup time and the time the lock is held on
> the radix tree. All in all it seems acceptable.
It should be fine, it'd read a root leaf from searching fs radix tree if it's a fs/file root while it'd get root immediately if it's not, like tree_root/extent_root/chunk_root, etc.
Thanks,
-liubo
>
> > + /*
> > + * The only reason we also check NULL here is that during
> > + * open_ctree() some roots has not yet been set up.
> > + */
> > + if (!IS_ERR_OR_NULL(r)) {
> > + /* if leaf is the root, then it's fine */
> > + if (leaf->start != btrfs_root_bytenr(&r->root_item)) {
> > + CORRUPT("non-root leaf's nritems is 0",
> > + leaf, root, 0);
> > + return -EIO;
> > + }
> > + }
> > return 0;
> > + }
> >
> > /* Check the 0 item */
> > if (btrfs_item_offset_nr(leaf, 0) + btrfs_item_size_nr(leaf, 0) !=
> > --
> > 2.5.5
> >
> > --
> > 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
--
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
