| Message ID | 20200402195118.4406-1-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: check commit root generation in should_ignore_root | expand |
On Thu, Apr 2, 2020 at 8:53 PM Josef Bacik <josef@toxicpanda.com> 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@toxicpanda.com> Reviewed-by: Filipe Manana <fdmanana@suse.com> 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 >
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
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@toxicpanda.com> --- fs/btrfs/relocation.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)