Btrfs devel - Nov 2013 - Re: [PATCH] Btrfs: optimize btrfs_lookup_extent_info() - extent refcount mismatches?

Hi,

On Fri, Jul 05, 2013 at 09:32:05PM +0100, Filipe David Borba Manana
wrote:
> If we''re looking for a metadata item in the tree and the
> search fails with return value of 1, and the slot doesn''t
> point to the first item in the leaf, check if the previous
> item in the leaf corresponds to an extent item for the same
> object id - if it does, then don''t do another tree search
> to get it.
I''m suspecting this patch to cause some trouble, see
https://bugzilla.kernel.org/show_bug.cgi?id=64961

but there were more reports similar to this one:
http://www.spinics.net/lists/linux-btrfs/msg29243.html
http://www.spinics.net/lists/linux-btrfs/msg29478.html

I did a quick analysis in comment 3, the warning pops when there are 0
references, there''s no other evidence that the extent should have none.
This happens during balance, so far the working hypothesis is that the
refcount changes during relocation (eg. subvolume deletion) but is not
fixed up in the late reloc phase.

fsck finds lots of wrong refcounts, off by one. The kernel warning fires
only if the refcount is 0 though there are > 0 expected. So this could
mean that the refcount is silently wrong all the time.


david
--
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

On Fri, Nov 29, 2013 at 3:45 PM, David Sterba <dsterba@suse.cz>
wrote:
> Hi,
>
> On Fri, Jul 05, 2013 at 09:32:05PM +0100, Filipe David Borba Manana wrote:
>> If we''re looking for a metadata item in the tree and the
>> search fails with return value of 1, and the slot doesn''t
>> point to the first item in the leaf, check if the previous
>> item in the leaf corresponds to an extent item for the same
>> object id - if it does, then don''t do another tree search
>> to get it.
>
> I''m suspecting this patch to cause some trouble, see
> https://bugzilla.kernel.org/show_bug.cgi?id=64961
Hi David,

What makes you believe the problem is exactly in this function?

I read it again, and I can''t see how it can miss an extent item that
it couldn''t before, specially without skinny metadata enabled.  Did
the fs had skinny metadata enabled?

thanks

>
> but there were more reports similar to this one:
> http://www.spinics.net/lists/linux-btrfs/msg29243.html
> http://www.spinics.net/lists/linux-btrfs/msg29478.html
>
> I did a quick analysis in comment 3, the warning pops when there are 0
> references, there''s no other evidence that the extent should have
none.
> This happens during balance, so far the working hypothesis is that the
> refcount changes during relocation (eg. subvolume deletion) but is not
> fixed up in the late reloc phase.
>
> fsck finds lots of wrong refcounts, off by one. The kernel warning fires
> only if the refcount is 0 though there are > 0 expected. So this could
> mean that the refcount is silently wrong all the time.
>
>
> david


-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That''s why all progress depends on unreasonable men."
--
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

On Fri, Nov 29, 2013 at 07:55:37PM +0000, Filipe David Manana
wrote:
> On Fri, Nov 29, 2013 at 3:45 PM, David Sterba <dsterba@suse.cz>
wrote:
> > Hi,
> >
> > On Fri, Jul 05, 2013 at 09:32:05PM +0100, Filipe David Borba Manana
wrote:
> >> If we''re looking for a metadata item in the tree and the
> >> search fails with return value of 1, and the slot doesn''t
> >> point to the first item in the leaf, check if the previous
> >> item in the leaf corresponds to an extent item for the same
> >> object id - if it does, then don''t do another tree search
> >> to get it.
> >
> > I''m suspecting this patch to cause some trouble, see
> > https://bugzilla.kernel.org/show_bug.cgi?id=64961
> 
> What makes you believe the problem is exactly in this function?
It was a first guess based on the stacktraces and last patch that
touched the code. I''ve replied to mailinlist because it has higher
visibility, number of bugreports in balance/relocation has been
increasing lately (not blaming you).
> I read it again, and I can''t see how it can miss an extent item
that
> it couldn''t before, specially without skinny metadata enabled. 
Did
> the fs had skinny metadata enabled?
According to the updated bugzilla yes, and that''s another possible
cause although we haven''t seen bugs with skinny metadata yet.
There''s
another bugreport that has them enabled,
https://bugzilla.kernel.org/show_bug.cgi?id=66411


david
--
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

On Tue, Dec 3, 2013 at 8:43 AM, David Sterba <dsterba@suse.cz>
wrote:
> On Fri, Nov 29, 2013 at 07:55:37PM +0000, Filipe David Manana wrote:
>> On Fri, Nov 29, 2013 at 3:45 PM, David Sterba <dsterba@suse.cz>
wrote:
>> > Hi,
>> >
>> > On Fri, Jul 05, 2013 at 09:32:05PM +0100, Filipe David Borba
Manana wrote:
>> >> If we''re looking for a metadata item in the tree and
the
>> >> search fails with return value of 1, and the slot
doesn''t
>> >> point to the first item in the leaf, check if the previous
>> >> item in the leaf corresponds to an extent item for the same
>> >> object id - if it does, then don''t do another tree
search
>> >> to get it.
>> >
>> > I''m suspecting this patch to cause some trouble, see
>> > https://bugzilla.kernel.org/show_bug.cgi?id=64961
>>
>> What makes you believe the problem is exactly in this function?
>
> It was a first guess based on the stacktraces and last patch that
> touched the code. I''ve replied to mailinlist because it has higher
> visibility, number of bugreports in balance/relocation has been
> increasing lately (not blaming you).
>
>> I read it again, and I can''t see how it can miss an extent
item that
>> it couldn''t before, specially without skinny metadata enabled.
Did
>> the fs had skinny metadata enabled?
>
> According to the updated bugzilla yes, and that''s another possible
> cause although we haven''t seen bugs with skinny metadata yet.
There''s
> another bugreport that has them enabled,
> https://bugzilla.kernel.org/show_bug.cgi?id=66411
>
I''ve been seeing similar issues while testing my qgroups patches that
I don''t think is related to my qgroup work.  I''m going to try
and
track it down this week but I may not get to it until next week.
Thanks,

Josef
--
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

On Tue, Dec 3, 2013 at 6:28 PM, Josef Bacik <josef@toxicpanda.com>
wrote:
> On Tue, Dec 3, 2013 at 8:43 AM, David Sterba <dsterba@suse.cz> wrote:
>> On Fri, Nov 29, 2013 at 07:55:37PM +0000, Filipe David Manana wrote:
>>> On Fri, Nov 29, 2013 at 3:45 PM, David Sterba
<dsterba@suse.cz> wrote:
>>> > Hi,
>>> >
>>> > On Fri, Jul 05, 2013 at 09:32:05PM +0100, Filipe David Borba
Manana wrote:
>>> >> If we''re looking for a metadata item in the tree
and the
>>> >> search fails with return value of 1, and the slot
doesn''t
>>> >> point to the first item in the leaf, check if the previous
>>> >> item in the leaf corresponds to an extent item for the
same
>>> >> object id - if it does, then don''t do another
tree search
>>> >> to get it.
>>> >
>>> > I''m suspecting this patch to cause some trouble, see
>>> > https://bugzilla.kernel.org/show_bug.cgi?id=64961
>>>
>>> What makes you believe the problem is exactly in this function?
>>
>> It was a first guess based on the stacktraces and last patch that
>> touched the code. I''ve replied to mailinlist because it has
higher
>> visibility, number of bugreports in balance/relocation has been
>> increasing lately (not blaming you).
>>
>>> I read it again, and I can''t see how it can miss an extent
item that
>>> it couldn''t before, specially without skinny metadata
enabled.  Did
>>> the fs had skinny metadata enabled?
>>
>> According to the updated bugzilla yes, and that''s another
possible
>> cause although we haven''t seen bugs with skinny metadata yet.
There''s
>> another bugreport that has them enabled,
>> https://bugzilla.kernel.org/show_bug.cgi?id=66411
I found an issue in btrfs_lookup_extent_info() when skinny metadata is
enabled. It can actually miss existing skinny extent items.
The following patch fixes it and explains in detail the issue:

https://patchwork.kernel.org/patch/3305521/

thanks
>>
>
> I''ve been seeing similar issues while testing my qgroups patches
that
> I don''t think is related to my qgroup work.  I''m going to
try and
> track it down this week but I may not get to it until next week.
> Thanks,
>
> Josef


-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That''s why all progress depends on unreasonable men."
--
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