On Mon, Mar 02, 2020 at 01:47:51PM -0500, Josef Bacik wrote:
> If we have an error while building the backref tree in relocation we'll
> process all the pending edges and then free the node. This isn't quite
> right however as the node could be integrated into the existing cache
> partially, linking children within itself into the cache, but not
> properly linked into the cache itself.
I'm missing description of what's the problem. Something is linked and
then freed, followed by 'fixed by'.
> The fix for this is simple, use
> remove_backref_node() instead of free_backref_node(), which will clean
> up the cache related to this node completely.
So this means that some entries are left in the cache? Leaked memory or
something else?
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> fs/btrfs/relocation.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 4fb7e3cc2aca..507361e99316 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1244,7 +1244,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
> free_backref_node(cache, lower);
> }
>
> - free_backref_node(cache, node);
> + remove_backref_node(cache, node);
> return ERR_PTR(err);
> }
> ASSERT(!node || !node->detached);
There's a similar pattern in clone_backref_node
1317 fail:
1318 while (!list_empty(&new_node->lower)) {
1319 new_edge = list_entry(new_node->lower.next,
1320 struct backref_edge, list[UPPER]);
1321 list_del(&new_edge->list[UPPER]);
1322 free_backref_edge(cache, new_edge);
1323 }
1324 free_backref_node(cache, new_node);
Does this also need to be fixed?
