Re: [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs

[Wang Shilong <wangsl.fnst@cn.fujitsu.com>]

Hi Filipe,

On 01/12/2014 11:36 PM, Filipe David Manana wrote:
> On Sun, Jan 12, 2014 at 1:38 PM, Wang Shilong <wangshilong1991@gmail.com> wrote:
>> From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>
>> If @slot=0, we may have an expected item in the previous leaf,
>> So we should handle that case, otherwise, we will miss inline refs
>> ,fix it.
> Hi Shilong.
>
> How can this happen exactly?
>
> So the search key, regardless of having an BTRFS_EXTENT_ITEM_KEY or  a
> BTRFS_METADATA_ITEM_KEY type, always has an offset set to (u64)-1.
> This means the btrfs_search_slot call will always return 1 (not found,
> as expected) or an error.
>
> And btrfs_search_slot, when it doesn't find a key and if that key
> should be the first item in a leaf, it makes the path point to the
> previous leaf with a "path->slots[0] ==
> btrfs_header_nritems(path->nodes[0])" (see [1]), otherwise to some
> value > 0. The only cases I can imagine getting "path->slots[0] == 0"
> is if we hit the first, i.e. left most, leaf in the btree or if the
> btree consists only of 1 leaf (no nodes) - in these cases there's
> isn't a previous leaf.
>
> Did I miss something here?
>
> thanks
>
> [1] - https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/ctree.c?id=refs/tags/v3.13-rc8#n2629
You are right, i was missing something when i made this patch.
Really thanks for you reviewing and correcting me!:-)

Thanks,
Wang
>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>>   fs/btrfs/backref.c | 13 ++++++-------
>>   1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>> index 964679c..4ccd726 100644
>> --- a/fs/btrfs/backref.c
>> +++ b/fs/btrfs/backref.c
>> @@ -893,14 +893,13 @@ again:
>>                  spin_unlock(&delayed_refs->lock);
>>          }
>>
>> -       if (path->slots[0]) {
>> -               struct extent_buffer *leaf;
>> -               int slot;
>> +       ret = btrfs_previous_extent_item(fs_info->extent_root, path,
>> +                                        key.objectid);
>> +       if (ret < 0)
>> +               goto out;
>>
>> -               path->slots[0]--;
>> -               leaf = path->nodes[0];
>> -               slot = path->slots[0];
>> -               btrfs_item_key_to_cpu(leaf, &key, slot);
>> +       if (ret == 0) {
>> +               btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>                  if (key.objectid == bytenr &&
>>                      (key.type == BTRFS_EXTENT_ITEM_KEY ||
>>                       key.type == BTRFS_METADATA_ITEM_KEY)) {
>> --
>> 1.8.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>