On 2018年04月24日 13:52, Su Yue wrote:
> For an extent item which contains many tree block backrefs, like
> =================================================================
> In 020-extent-ref-cases/keyed_block_ref.img
>
> item 10 key (29470720 METADATA_ITEM 0) itemoff 3450 itemsize 222
> refs 23 gen 10 flags TREE_BLOCK
> tree block skinny level 0
> tree block backref root 278
> tree block backref root 277
> tree block backref root 276
> tree block backref root 275
> tree block backref root 274
> tree block backref root 273
> tree block backref root 272
> tree block backref root 271
> tree block backref root 270
> tree block backref root 269
> tree block backref root 268
> tree block backref root 267
> tree block backref root 266
> tree block backref root 265
> tree block backref root 264
> tree block backref root 263
> tree block backref root 262
> tree block backref root 261
> tree block backref root 260
> tree block backref root 259
> tree block backref root 258
> tree block backref root 257
> =================================================================
> In find_parent_nodes(), these refs's parents are 0, then __merge_refs
> will merge refs to one ref. It causes only one root to be returned.
This is a pretty big problem, and it would cause qgroup verification
code to cause false alerts.
So a new test case would do great help here.
>
> So, if both parents are 0, do not merge refs.
>
> Lowmem check calls find_parent_nodes frequently to decide whether.
> check an extent buffer or not. These bug influences bytes accounting.
Although it looks good so far, and would fix the problem you found, but
I strongly recommend to port kernel backref code to btrfs-progs here, as
kernel backref code is more tested than btrfs-progs backref code.
I'm pretty sure we have more bugs in btrfs-progs backref, and I'm not a
big fan of playing whac-a-mole here.
>
> Signed-off-by: Su Yue <suy.fnst@xxxxxxxxxxxxxx>
> ---
> backref.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/backref.c b/backref.c
> index 51553c702187..daadb6299c79 100644
> --- a/backref.c
> +++ b/backref.c
> @@ -507,6 +507,8 @@ static void __merge_refs(struct pref_state *prefstate, int mode)
> } else {
> if (ref1->parent != ref2->parent)
> continue;
> + if (ref1->parent == 0)
> + continue;
It's better to put it above (ref1->parent != ref2->parent).
As parent == 0 means we haven't resolve the parent bytenr yet, so can't
be merged nor compared.
Thanks,
Qu
> }
>
> eie = ref1->inode_list;
>
Attachment:
signature.asc
Description: OpenPGP digital signature
