| Message ID | 20200304161830.2360-7-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | relocation error handling fixes | expand |
On 2020/3/5 上午12:18, Josef Bacik wrote: > We were doing the clear dance for the reloc root after doing the drop of > the reloc root, which means we have a giant window where we could miss > having BTRFS_ROOT_DEAD_RELOC_TREE unset and the reloc_root == NULL. Still, I can't see the problem where we have BTRFS_ROOT_DEAD_RELOC_TREE and reloc_root == NULL. IMHO, that would cause anything wrong. Or is there anything I missed? Thanks, Qu > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/relocation.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index 53509c367eff..ceb90d152cdd 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -2291,18 +2291,19 @@ static int clean_dirty_subvols(struct reloc_control *rc) > > list_del_init(&root->reloc_dirty_list); > root->reloc_root = NULL; > - if (reloc_root) { > - > - ret2 = btrfs_drop_snapshot(reloc_root, NULL, 0, 1); > - if (ret2 < 0 && !ret) > - ret = ret2; > - } > /* > * Need barrier to ensure clear_bit() only happens after > * root->reloc_root = NULL. Pairs with have_reloc_root. > */ > smp_wmb(); > clear_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state); > + > + if (reloc_root) { > + > + ret2 = btrfs_drop_snapshot(reloc_root, NULL, 0, 1); > + if (ret2 < 0 && !ret) > + ret = ret2; > + } > btrfs_put_root(root); > } else { > /* Orphan reloc tree, just clean it up */ >
On 3/5/20 6:41 AM, Qu Wenruo wrote: > > > On 2020/3/5 上午12:18, Josef Bacik wrote: >> We were doing the clear dance for the reloc root after doing the drop of >> the reloc root, which means we have a giant window where we could miss >> having BTRFS_ROOT_DEAD_RELOC_TREE unset and the reloc_root == NULL. > > Still, I can't see the problem where we have BTRFS_ROOT_DEAD_RELOC_TREE > and reloc_root == NULL. > > IMHO, that would cause anything wrong. Or is there anything I missed? > I was still hitting leaks and I was convinced it was because we were re-init'ing the reloc root, but I think that line of reasoning is just wrong. I'll reword the changelog, it's just a cosmetic thing at this point, not a real problem. Thanks, Josef
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 53509c367eff..ceb90d152cdd 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2291,18 +2291,19 @@ static int clean_dirty_subvols(struct reloc_control *rc) list_del_init(&root->reloc_dirty_list); root->reloc_root = NULL; - if (reloc_root) { - - ret2 = btrfs_drop_snapshot(reloc_root, NULL, 0, 1); - if (ret2 < 0 && !ret) - ret = ret2; - } /* * Need barrier to ensure clear_bit() only happens after * root->reloc_root = NULL. Pairs with have_reloc_root. */ smp_wmb(); clear_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state); + + if (reloc_root) { + + ret2 = btrfs_drop_snapshot(reloc_root, NULL, 0, 1); + if (ret2 < 0 && !ret) + ret = ret2; + } btrfs_put_root(root); } else { /* Orphan reloc tree, just clean it up */
We were doing the clear dance for the reloc root after doing the drop of the reloc root, which means we have a giant window where we could miss having BTRFS_ROOT_DEAD_RELOC_TREE unset and the reloc_root == NULL. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)