David Sterba wrote on 2016/04/26 12:06 +0200:
On Tue, Apr 26, 2016 at 10:49:49AM +0800, Qu Wenruo wrote:
In the new add_extent_rec_nolookup() function, we add bytes_used to
update found bytes accounting.
However there is a typo that we used tmpl->nr, which should be rec->nr.
This will make us to add 1 for data backref, instead the correct size.
I'm not able to trace that back to the original code, the value template
is just filled from the parameters and I don't see where bytes_used
would get set from the rec->nr value.
Then it would be a very old bug.
When iteration extent tree, when btrfsck found a data backref, it will
fill tmpl->nr with 1 and max_size with real size.
And if following that code, we increase bytes_used by 1, not max_size.
This makes the final "Found XXX bytes" unaligned to sectorsize.
While not much people will check such output, until we introduce the new
low-memory mode, and compare the output, finding the difference.
If checked manually (by adding all metadata and extent size from
debug-tree output), one would find the result is always smaller than
expected.
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4550,7 +4550,7 @@ static int add_extent_rec_nolookup(struct cache_tree
*extent_cache,
rec->cache.size = tmpl->nr;
ret = insert_cache_extent(extent_cache, &rec->cache);
BUG_ON(ret);
- bytes_used += tmpl->nr;
+ bytes_used += rec->nr;
ie. this would be just 'nr' before the refactoring.
Yes, I also checked v4.2 code, it is indeed "nr", so it's a long
standing bug.
Thanks,
Qu
--
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
