On 9/6/16 5:51 PM, Liu Bo wrote:
> Hi Filipe,
>
> On Mon, Sep 05, 2016 at 04:28:09PM +0100, Filipe Manana wrote:
>> On Fri, Sep 2, 2016 at 8:35 PM, Liu Bo <bo.li.liu@xxxxxxxxxx> wrote:
>>> This can only happen with CONFIG_BTRFS_FS_CHECK_INTEGRITY=y.
>>>
>>> Commit 1ba98d0 ("Btrfs: detect corruption when non-root leaf has zero item")
>>> assumes that a leaf is its root when leaf->bytenr == btrfs_root_bytenr(root),
>>> however, we should not use btrfs_root_bytenr(root) since it's mainly got
>>> updated during committing transaction. So the check can fail when doing
>>> COW on this leaf while it is a root.
>>>
>>> This changes to use "if (leaf == btrfs_root_node(root))" instead, just like
>>> how we check whether leaf is a root in __btrfs_cow_block().
>>>
>>> Reported-by: Jeff Mahoney <jeffm@xxxxxxxx>
>>> Signed-off-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
>>
>> Hi Bo,
>>
>> Even with this patch applied against latest branch for-linus-4.8, at
>> least on a build with CONFIG_BTRFS_FS_CHECK_INTEGRITY=y,
>> the issue still happens for me when running fsstress with balance in parallel:
>
> Thanks for the report.
>
> This panic shows that we can have non-root btree leaf with 0 nritems during
> split_leaf(), but a btrfs_search_slot which calls split_leaf() like this is
> inserting an item, and while we set @right's nritems to 0, we also assign @disk_key
> associated with @right in the parent node, so I think we're actually having
> nritem 0 temporarily and we can remove this btrfs_mark_buffer_dirty() like the
> following quick patch.
>
> Thanks,
>
> -liubo
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index d1c56c9..5e5ceb5 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -4341,7 +4341,11 @@ again:
> if (path->slots[1] == 0)
> fixup_low_keys(fs_info, path, &disk_key, 1);
> }
> - btrfs_mark_buffer_dirty(right);
> + /*
> + * We create a new leaf 'right' for the required ins_len and
> + * we'll do btrfs_mark_buffer_dirty() on this leaf after copying
> + * the content of ins_len to 'right'.
> + */
> return ret;
> }
>
>
>
I think you're on the right track here. I need to see if I still have
the code lying around, but when I was debugging the btrfs_rename issue
that ended up being a compiler bug, I hooked into check_leaf and ran
into similar issues. We mark the buffer dirty before it's in a
consistent state.
-Jeff
--
Jeff Mahoney
SUSE Labs
Attachment:
signature.asc
Description: OpenPGP digital signature
