Re: [PATCH] btrfs: check commit root generation in should_ignore_root

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.”




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux