On Tue, Oct 30, 2018 at 7:00 AM robbieko <robbieko@xxxxxxxxxxxx> wrote:
>
> From: Robbie Ko <robbieko@xxxxxxxxxxxx>
>
> In apply_children_dir_moves, we first create an empty list (stack),
> then we get an entry from pending_dir_moves and add it to the stack,
> but we didn't delete the entry from rb_tree.
>
> So, in add_pending_dir_move, we create a new entry and then use the
> parent_ino in the current rb_tree to find the corresponding entry,
> and if so, add the new entry to the corresponding list.
>
> However, the entry may have been added to the stack, causing new
> entries to be added to the stack as well.
>
> Finally, each time we take the first entry from the stack and start
> processing, it ends up with an infinite loop.
>
> Fix this problem by remove node from pending_dir_moves,
> avoid add new pending_dir_move to error list.
I can't parse that explanation.
Can you give a concrete example (reproducer) or did this came out of thin air?
Thanks.
>
> Signed-off-by: Robbie Ko <robbieko@xxxxxxxxxxxx>
> ---
> fs/btrfs/send.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 094cc144..5be83b5 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -3340,7 +3340,8 @@ static void free_pending_move(struct send_ctx *sctx, struct pending_dir_move *m)
> kfree(m);
> }
>
> -static void tail_append_pending_moves(struct pending_dir_move *moves,
> +static void tail_append_pending_moves(struct send_ctx *sctx,
> + struct pending_dir_move *moves,
> struct list_head *stack)
> {
> if (list_empty(&moves->list)) {
> @@ -3351,6 +3352,10 @@ static void tail_append_pending_moves(struct pending_dir_move *moves,
> list_add_tail(&moves->list, stack);
> list_splice_tail(&list, stack);
> }
> + if (!RB_EMPTY_NODE(&moves->node)) {
> + rb_erase(&moves->node, &sctx->pending_dir_moves);
> + RB_CLEAR_NODE(&moves->node);
> + }
> }
>
> static int apply_children_dir_moves(struct send_ctx *sctx)
> @@ -3365,7 +3370,7 @@ static int apply_children_dir_moves(struct send_ctx *sctx)
> return 0;
>
> INIT_LIST_HEAD(&stack);
> - tail_append_pending_moves(pm, &stack);
> + tail_append_pending_moves(sctx, pm, &stack);
>
> while (!list_empty(&stack)) {
> pm = list_first_entry(&stack, struct pending_dir_move, list);
> @@ -3376,7 +3381,7 @@ static int apply_children_dir_moves(struct send_ctx *sctx)
> goto out;
> pm = get_pending_dir_moves(sctx, parent_ino);
> if (pm)
> - tail_append_pending_moves(pm, &stack);
> + tail_append_pending_moves(sctx, pm, &stack);
> }
> return 0;
>
> --
> 1.9.1
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
