* [PATCH 1/2] btrfs: free fs roots on failed mount @ 2020-07-22 16:07 Josef Bacik 2020-07-22 16:07 ` [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu Josef Bacik 2020-07-27 14:19 ` [PATCH 1/2] btrfs: free fs roots on failed mount David Sterba 0 siblings, 2 replies; 12+ messages in thread From: Josef Bacik @ 2020-07-22 16:07 UTC (permalink / raw) To: linux-btrfs, kernel-team While testing a weird problem with -o degraded, I noticed I was getting leaked root errors BTRFS warning (device loop0): writable mount is not allowed due to too many missing devices BTRFS error (device loop0): open_ctree failed BTRFS error (device loop0): leaked root -9-0 refcount 1 This is the DATA_RELOC root, which gets read before the other fs roots, but is included in the fs roots radix tree, and thus gets freed by btrfs_free_fs_roots. Fix this by moving the call into fail_tree_roots: in open_ctree. With this fix we no longer leak that root on mount failure. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/disk-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index c850d7f44fbe..f1fdbdd44c02 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3421,7 +3421,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device fail_trans_kthread: kthread_stop(fs_info->transaction_kthread); btrfs_cleanup_transaction(fs_info); - btrfs_free_fs_roots(fs_info); fail_cleaner: kthread_stop(fs_info->cleaner_kthread); @@ -3441,6 +3440,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device btrfs_put_block_group_cache(fs_info); fail_tree_roots: + btrfs_free_fs_roots(fs_info); free_root_pointers(fs_info, true); invalidate_inode_pages2(fs_info->btree_inode->i_mapping); -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu 2020-07-22 16:07 [PATCH 1/2] btrfs: free fs roots on failed mount Josef Bacik @ 2020-07-22 16:07 ` Josef Bacik 2020-07-23 14:20 ` David Sterba 2020-07-27 14:19 ` [PATCH 1/2] btrfs: free fs roots on failed mount David Sterba 1 sibling, 1 reply; 12+ messages in thread From: Josef Bacik @ 2020-07-22 16:07 UTC (permalink / raw) To: linux-btrfs, kernel-team I'm a actual human being so am incapable of converting u64 to s64 in my head, use %lld so we can see the negative number in order to identify which of our special roots we leaked. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/disk-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index f1fdbdd44c02..cc4081a1c7f9 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1508,7 +1508,7 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info) while (!list_empty(&fs_info->allocated_roots)) { root = list_first_entry(&fs_info->allocated_roots, struct btrfs_root, leak_list); - btrfs_err(fs_info, "leaked root %llu-%llu refcount %d", + btrfs_err(fs_info, "leaked root %lld-%llu refcount %d", root->root_key.objectid, root->root_key.offset, refcount_read(&root->refs)); while (refcount_read(&root->refs) > 1) -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu 2020-07-22 16:07 ` [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu Josef Bacik @ 2020-07-23 14:20 ` David Sterba 2020-07-24 0:40 ` Qu Wenruo 0 siblings, 1 reply; 12+ messages in thread From: David Sterba @ 2020-07-23 14:20 UTC (permalink / raw) To: Josef Bacik; +Cc: linux-btrfs, kernel-team On Wed, Jul 22, 2020 at 12:07:22PM -0400, Josef Bacik wrote: > I'm a actual human being so am incapable of converting u64 to s64 in my > head, use %lld so we can see the negative number in order to identify > which of our special roots we leaked. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/disk-io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index f1fdbdd44c02..cc4081a1c7f9 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -1508,7 +1508,7 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info) > while (!list_empty(&fs_info->allocated_roots)) { > root = list_first_entry(&fs_info->allocated_roots, > struct btrfs_root, leak_list); > - btrfs_err(fs_info, "leaked root %llu-%llu refcount %d", > + btrfs_err(fs_info, "leaked root %lld-%llu refcount %d", But this is wrong in another way, roots with high numbers will appear as negative numbers. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu 2020-07-23 14:20 ` David Sterba @ 2020-07-24 0:40 ` Qu Wenruo 2020-07-27 14:03 ` David Sterba 0 siblings, 1 reply; 12+ messages in thread From: Qu Wenruo @ 2020-07-24 0:40 UTC (permalink / raw) To: dsterba, Josef Bacik, linux-btrfs, kernel-team [-- Attachment #1.1: Type: text/plain, Size: 1334 bytes --] On 2020/7/23 下午10:20, David Sterba wrote: > On Wed, Jul 22, 2020 at 12:07:22PM -0400, Josef Bacik wrote: >> I'm a actual human being so am incapable of converting u64 to s64 in my >> head, use %lld so we can see the negative number in order to identify >> which of our special roots we leaked. >> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >> --- >> fs/btrfs/disk-io.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >> index f1fdbdd44c02..cc4081a1c7f9 100644 >> --- a/fs/btrfs/disk-io.c >> +++ b/fs/btrfs/disk-io.c >> @@ -1508,7 +1508,7 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info) >> while (!list_empty(&fs_info->allocated_roots)) { >> root = list_first_entry(&fs_info->allocated_roots, >> struct btrfs_root, leak_list); >> - btrfs_err(fs_info, "leaked root %llu-%llu refcount %d", >> + btrfs_err(fs_info, "leaked root %lld-%llu refcount %d", > > But this is wrong in another way, roots with high numbers will appear as > negative numbers. > Nope. We won't have that many roots. In fact, for subvolumes, the highest id is only 2 ^ 48, an special limit introduced for qgroup. So we won't have high enough subvolume ids to be negative, but only special trees. Thanks, Qu [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu 2020-07-24 0:40 ` Qu Wenruo @ 2020-07-27 14:03 ` David Sterba 2020-07-27 14:07 ` David Sterba 2020-07-27 23:47 ` Qu Wenruo 0 siblings, 2 replies; 12+ messages in thread From: David Sterba @ 2020-07-27 14:03 UTC (permalink / raw) To: Qu Wenruo; +Cc: dsterba, Josef Bacik, linux-btrfs, kernel-team On Fri, Jul 24, 2020 at 08:40:17AM +0800, Qu Wenruo wrote: > > > On 2020/7/23 下午10:20, David Sterba wrote: > > On Wed, Jul 22, 2020 at 12:07:22PM -0400, Josef Bacik wrote: > >> I'm a actual human being so am incapable of converting u64 to s64 in my > >> head, use %lld so we can see the negative number in order to identify > >> which of our special roots we leaked. > >> > >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> > >> --- > >> fs/btrfs/disk-io.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > >> index f1fdbdd44c02..cc4081a1c7f9 100644 > >> --- a/fs/btrfs/disk-io.c > >> +++ b/fs/btrfs/disk-io.c > >> @@ -1508,7 +1508,7 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info) > >> while (!list_empty(&fs_info->allocated_roots)) { > >> root = list_first_entry(&fs_info->allocated_roots, > >> struct btrfs_root, leak_list); > >> - btrfs_err(fs_info, "leaked root %llu-%llu refcount %d", > >> + btrfs_err(fs_info, "leaked root %lld-%llu refcount %d", > > > > But this is wrong in another way, roots with high numbers will appear as > > negative numbers. > > > > Nope. We won't have that many roots. > > In fact, for subvolumes, the highest id is only 2 ^ 48, an special limit > introduced for qgroup. It's not a hard limit and certainly can have subvolumes with numbers that high. That qgoups interpret the qgroup in some way is not a limitation on subvolumes. We'll have to start reusing the subvolume ids eventually, with qgroups we can on. Also the negativer numbers start to appear with 2^32 so that's still below the percieved limit of 2^48. > So we won't have high enough subvolume ids to be negative, but only > special trees. For the internal trees we eg. have pretty-printer in progs so kernel can reuse that. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu 2020-07-27 14:03 ` David Sterba @ 2020-07-27 14:07 ` David Sterba 2020-07-27 23:47 ` Qu Wenruo 1 sibling, 0 replies; 12+ messages in thread From: David Sterba @ 2020-07-27 14:07 UTC (permalink / raw) To: dsterba, Qu Wenruo, Josef Bacik, linux-btrfs, kernel-team On Mon, Jul 27, 2020 at 04:03:14PM +0200, David Sterba wrote: > On Fri, Jul 24, 2020 at 08:40:17AM +0800, Qu Wenruo wrote: > > Nope. We won't have that many roots. > > > > In fact, for subvolumes, the highest id is only 2 ^ 48, an special limit > > introduced for qgroup. > > It's not a hard limit and certainly can have subvolumes with numbers > that high. That qgoups interpret the qgroup in some way is not a > limitation on subvolumes. We'll have to start reusing the subvolume ids > eventually, with qgroups we can on. Let me rephrase without the typos: It's not a hard limit and we certainly can have subvolumes with numbers that high. That qgroups interpret the qgroup id in some way is not a limitation on subvolumes. We'll have to start reusing the subvolume ids eventually, when qgroups are turned on. (The comment about 2^32/2^48 was incorrect.) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu 2020-07-27 14:03 ` David Sterba 2020-07-27 14:07 ` David Sterba @ 2020-07-27 23:47 ` Qu Wenruo 2020-07-28 15:09 ` David Sterba 1 sibling, 1 reply; 12+ messages in thread From: Qu Wenruo @ 2020-07-27 23:47 UTC (permalink / raw) To: dsterba, Josef Bacik, linux-btrfs, kernel-team [-- Attachment #1.1: Type: text/plain, Size: 2273 bytes --] On 2020/7/27 下午10:03, David Sterba wrote: > On Fri, Jul 24, 2020 at 08:40:17AM +0800, Qu Wenruo wrote: >> >> >> On 2020/7/23 下午10:20, David Sterba wrote: >>> On Wed, Jul 22, 2020 at 12:07:22PM -0400, Josef Bacik wrote: >>>> I'm a actual human being so am incapable of converting u64 to s64 in my >>>> head, use %lld so we can see the negative number in order to identify >>>> which of our special roots we leaked. >>>> >>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >>>> --- >>>> fs/btrfs/disk-io.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>>> index f1fdbdd44c02..cc4081a1c7f9 100644 >>>> --- a/fs/btrfs/disk-io.c >>>> +++ b/fs/btrfs/disk-io.c >>>> @@ -1508,7 +1508,7 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info) >>>> while (!list_empty(&fs_info->allocated_roots)) { >>>> root = list_first_entry(&fs_info->allocated_roots, >>>> struct btrfs_root, leak_list); >>>> - btrfs_err(fs_info, "leaked root %llu-%llu refcount %d", >>>> + btrfs_err(fs_info, "leaked root %lld-%llu refcount %d", >>> >>> But this is wrong in another way, roots with high numbers will appear as >>> negative numbers. >>> >> >> Nope. We won't have that many roots. >> >> In fact, for subvolumes, the highest id is only 2 ^ 48, an special limit >> introduced for qgroup. > > It's not a hard limit and certainly can have subvolumes with numbers > that high. That qgoups interpret the qgroup in some way is not a > limitation on subvolumes. We'll have to start reusing the subvolume ids > eventually, with qgroups we can on. Strange... I still remember that we put that limit as a hard limit for subvolume creation. Did we change that behavior in recent releases? > > Also the negativer numbers start to appear with 2^32 so that's still > below the percieved limit of 2^48. Nope, For signed 64bits, it's -2^63 ~ 2^63 - 1, not 2 ^ 32. > >> So we won't have high enough subvolume ids to be negative, but only >> special trees. > > For the internal trees we eg. have pretty-printer in progs so kernel can > reuse that. > That's true. Pretty tree name is much better for human to read. Thanks, Qu [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu 2020-07-27 23:47 ` Qu Wenruo @ 2020-07-28 15:09 ` David Sterba 0 siblings, 0 replies; 12+ messages in thread From: David Sterba @ 2020-07-28 15:09 UTC (permalink / raw) To: Qu Wenruo; +Cc: dsterba, Josef Bacik, linux-btrfs, kernel-team On Tue, Jul 28, 2020 at 07:47:00AM +0800, Qu Wenruo wrote: > I still remember that we put that limit as a hard limit for subvolume > creation. Ok, I found it, added by your patch e09fe2d2119800e6 in 2015. It's not documented anywhere, eg. manual page 5 states the limit is 2^64. 2^48 should be enough for everyone and we can't probably changed it right now as it's part of API and ABI. Oh well. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] btrfs: free fs roots on failed mount 2020-07-22 16:07 [PATCH 1/2] btrfs: free fs roots on failed mount Josef Bacik 2020-07-22 16:07 ` [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu Josef Bacik @ 2020-07-27 14:19 ` David Sterba 2020-07-27 14:33 ` Josef Bacik 1 sibling, 1 reply; 12+ messages in thread From: David Sterba @ 2020-07-27 14:19 UTC (permalink / raw) To: Josef Bacik; +Cc: linux-btrfs, kernel-team On Wed, Jul 22, 2020 at 12:07:21PM -0400, Josef Bacik wrote: > While testing a weird problem with -o degraded, I noticed I was getting > leaked root errors > > BTRFS warning (device loop0): writable mount is not allowed due to too many missing devices > BTRFS error (device loop0): open_ctree failed > BTRFS error (device loop0): leaked root -9-0 refcount 1 > > This is the DATA_RELOC root, which gets read before the other fs roots, > but is included in the fs roots radix tree, and thus gets freed by > btrfs_free_fs_roots. Fix this by moving the call into fail_tree_roots: > in open_ctree. With this fix we no longer leak that root on mount > failure. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/disk-io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index c850d7f44fbe..f1fdbdd44c02 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -3421,7 +3421,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device > fail_trans_kthread: > kthread_stop(fs_info->transaction_kthread); > btrfs_cleanup_transaction(fs_info); > - btrfs_free_fs_roots(fs_info); > fail_cleaner: > kthread_stop(fs_info->cleaner_kthread); > > @@ -3441,6 +3440,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device > btrfs_put_block_group_cache(fs_info); > > fail_tree_roots: > + btrfs_free_fs_roots(fs_info); > free_root_pointers(fs_info, true); The data reloc tree is freed inside free_root_pointers, that it's also in the radix tree is for convenience so I'd rather fix it inside free_root_pointers and not reorder btrfs_free_fs_roots. > invalidate_inode_pages2(fs_info->btree_inode->i_mapping); > > -- > 2.24.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] btrfs: free fs roots on failed mount 2020-07-27 14:19 ` [PATCH 1/2] btrfs: free fs roots on failed mount David Sterba @ 2020-07-27 14:33 ` Josef Bacik 2020-07-27 15:17 ` David Sterba 0 siblings, 1 reply; 12+ messages in thread From: Josef Bacik @ 2020-07-27 14:33 UTC (permalink / raw) To: dsterba, linux-btrfs, kernel-team On 7/27/20 10:19 AM, David Sterba wrote: > On Wed, Jul 22, 2020 at 12:07:21PM -0400, Josef Bacik wrote: >> While testing a weird problem with -o degraded, I noticed I was getting >> leaked root errors >> >> BTRFS warning (device loop0): writable mount is not allowed due to too many missing devices >> BTRFS error (device loop0): open_ctree failed >> BTRFS error (device loop0): leaked root -9-0 refcount 1 >> >> This is the DATA_RELOC root, which gets read before the other fs roots, >> but is included in the fs roots radix tree, and thus gets freed by >> btrfs_free_fs_roots. Fix this by moving the call into fail_tree_roots: >> in open_ctree. With this fix we no longer leak that root on mount >> failure. >> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >> --- >> fs/btrfs/disk-io.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >> index c850d7f44fbe..f1fdbdd44c02 100644 >> --- a/fs/btrfs/disk-io.c >> +++ b/fs/btrfs/disk-io.c >> @@ -3421,7 +3421,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device >> fail_trans_kthread: >> kthread_stop(fs_info->transaction_kthread); >> btrfs_cleanup_transaction(fs_info); >> - btrfs_free_fs_roots(fs_info); >> fail_cleaner: >> kthread_stop(fs_info->cleaner_kthread); >> >> @@ -3441,6 +3440,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device >> btrfs_put_block_group_cache(fs_info); >> >> fail_tree_roots: >> + btrfs_free_fs_roots(fs_info); >> free_root_pointers(fs_info, true); > > The data reloc tree is freed inside free_root_pointers, that it's also > in the radix tree is for convenience so I'd rather fix it inside > free_root_pointers and not reorder btrfs_free_fs_roots. > We can't do that, because free_root_pointers() is called to drop the extent buffers when we're looping through the backup roots. btrfs_free_fs_roots() is the correct thing to do here, it goes through anything that ended up in the radix tree. Thanks, Josef ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] btrfs: free fs roots on failed mount 2020-07-27 14:33 ` Josef Bacik @ 2020-07-27 15:17 ` David Sterba 2020-07-27 15:37 ` Josef Bacik 0 siblings, 1 reply; 12+ messages in thread From: David Sterba @ 2020-07-27 15:17 UTC (permalink / raw) To: Josef Bacik; +Cc: dsterba, linux-btrfs, kernel-team On Mon, Jul 27, 2020 at 10:33:32AM -0400, Josef Bacik wrote: > >> @@ -3441,6 +3440,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device > >> btrfs_put_block_group_cache(fs_info); > >> > >> fail_tree_roots: > >> + btrfs_free_fs_roots(fs_info); > >> free_root_pointers(fs_info, true); > > > > The data reloc tree is freed inside free_root_pointers, that it's also > > in the radix tree is for convenience so I'd rather fix it inside > > free_root_pointers and not reorder btrfs_free_fs_roots. > > We can't do that, because free_root_pointers() is called to drop the extent > buffers when we're looping through the backup roots. btrfs_free_fs_roots() is > the correct thing to do here, it goes through anything that ended up in the > radix tree. Thanks, I see, free_root_pointers is used elsewhere. I'm concerned about the reordeing because there are several functions that are now called after the roots are freed. (before) btrfs_free_fs_roots(fs_info); kthread_stop(fs_info->cleaner_kthread); filemap_write_and_wait(fs_info->btree_inode->i_mapping); btrfs_sysfs_remove_mounted(fs_info); btrfs_sysfs_remove_fsid(fs_info->fs_devices); btrfs_put_block_group_cache(fs_info); (after) btrfs_free_fs_roots(fs_info); If something called by btrfs_free_fs_roots depends on structures removed/cleaned by the other functions, eg. btrfs_put_block_group_cache ti could be a problem. I haven't spotted anything so far, the functions are called after failure still during mount, this is easier than shutdown sequence after a full mount. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] btrfs: free fs roots on failed mount 2020-07-27 15:17 ` David Sterba @ 2020-07-27 15:37 ` Josef Bacik 0 siblings, 0 replies; 12+ messages in thread From: Josef Bacik @ 2020-07-27 15:37 UTC (permalink / raw) To: dsterba, linux-btrfs, kernel-team On 7/27/20 11:17 AM, David Sterba wrote: > On Mon, Jul 27, 2020 at 10:33:32AM -0400, Josef Bacik wrote: >>>> @@ -3441,6 +3440,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device >>>> btrfs_put_block_group_cache(fs_info); >>>> >>>> fail_tree_roots: >>>> + btrfs_free_fs_roots(fs_info); >>>> free_root_pointers(fs_info, true); >>> >>> The data reloc tree is freed inside free_root_pointers, that it's also >>> in the radix tree is for convenience so I'd rather fix it inside >>> free_root_pointers and not reorder btrfs_free_fs_roots. >> >> We can't do that, because free_root_pointers() is called to drop the extent >> buffers when we're looping through the backup roots. btrfs_free_fs_roots() is >> the correct thing to do here, it goes through anything that ended up in the >> radix tree. Thanks, > > I see, free_root_pointers is used elsewhere. I'm concerned about the > reordeing because there are several functions that are now called after > the roots are freed. > > (before) btrfs_free_fs_roots(fs_info); > > kthread_stop(fs_info->cleaner_kthread); > filemap_write_and_wait(fs_info->btree_inode->i_mapping); > btrfs_sysfs_remove_mounted(fs_info); > btrfs_sysfs_remove_fsid(fs_info->fs_devices); > btrfs_put_block_group_cache(fs_info); > > (after) btrfs_free_fs_roots(fs_info); > > If something called by btrfs_free_fs_roots depends on structures > removed/cleaned by the other functions, eg. btrfs_put_block_group_cache > ti could be a problem. > > I haven't spotted anything so far, the functions are called after > failure still during mount, this is easier than shutdown sequence after > a full mount. > Yeah I'm always loathe to move this stuff around. I actually think we can end up with the case described in close_ctree if we get an error during log replay on mount. I'll rework this some more. Thanks, Josef ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-07-28 15:09 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-22 16:07 [PATCH 1/2] btrfs: free fs roots on failed mount Josef Bacik 2020-07-22 16:07 ` [PATCH 2/2] btrfs: fix root leak printk to use %lld instead of %llu Josef Bacik 2020-07-23 14:20 ` David Sterba 2020-07-24 0:40 ` Qu Wenruo 2020-07-27 14:03 ` David Sterba 2020-07-27 14:07 ` David Sterba 2020-07-27 23:47 ` Qu Wenruo 2020-07-28 15:09 ` David Sterba 2020-07-27 14:19 ` [PATCH 1/2] btrfs: free fs roots on failed mount David Sterba 2020-07-27 14:33 ` Josef Bacik 2020-07-27 15:17 ` David Sterba 2020-07-27 15:37 ` Josef Bacik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.