On Thu, Apr 2, 2020 at 8:53 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> Previously we would set the reloc root's last snapshot to transid - 1.
> However there was a problem with doing this, and we changed it to
> setting the last snapshot to the generation of the commit node of the fs
> root.
>
> This however broke should_ignore_root(). The assumption is that if we
> are in a generation newer than when the reloc root was created, then we
> would find the reloc root through normal backref lookups, and thus can
> ignore any fs roots we find with an old enough reloc root.
>
> Now that the last snapshot could be considerably further in the past
> than before, we'd end up incorrectly ignoring an fs root. Thus we'd
> find no nodes for the bytenr we were searching for, and we'd fail to
> relocate anything. We'd loop through the relocate code again and see
> that there were still used space in that block group, attempt to
> relocate those bytenr's again, fail in the same way, and just loop like
> this forever. This is tricky in that we have to not modify the fs root
> at all during this time, so we need to have a block group that has data
> in this fs root that is not shared by any other root, which is why this
> has been difficult to reproduce.
>
> Fixes: 054570a1dc94 ("Btrfs: fix relocation incorrectly dropping data references")
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
Looks good, thanks.
> ---
> fs/btrfs/relocation.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 89a218cb81ef..7cb8d4123169 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -616,8 +616,8 @@ static int should_ignore_root(struct btrfs_root *root)
> if (!reloc_root)
> return 0;
>
> - if (btrfs_root_last_snapshot(&reloc_root->root_item) ==
> - root->fs_info->running_transaction->transid - 1)
> + if (btrfs_header_generation(reloc_root->commit_root) ==
> + root->fs_info->running_transaction->transid)
> return 0;
> /*
> * if there is reloc tree and it was created in previous
> --
> 2.24.1
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”