lkml.org 
[lkml]   [2020]   [Feb]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4.9 083/116] btrfs: free block groups after freeing fs trees
On Thu, Feb 13, 2020 at 09:55:33PM +0100, David Sterba wrote:
>On Thu, Feb 13, 2020 at 07:20:27AM -0800, Greg Kroah-Hartman wrote:
>> From: Josef Bacik <josef@toxicpanda.com>
>>
>> [ Upstream commit 4e19443da1941050b346f8fc4c368aa68413bc88 ]
>>
>> Sometimes when running generic/475 we would trip the
>> WARN_ON(cache->reserved) check when free'ing the block groups on umount.
>> This is because sometimes we don't commit the transaction because of IO
>> errors and thus do not cleanup the tree logs until at umount time.
>>
>> These blocks are still reserved until they are cleaned up, but they
>> aren't cleaned up until _after_ we do the free block groups work. Fix
>> this by moving the free after free'ing the fs roots, that way all of the
>> tree logs are cleaned up and we have a properly cleaned fs. A bunch of
>> loops of generic/475 confirmed this fixes the problem.
>>
>> CC: stable@vger.kernel.org # 4.9+
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> Reviewed-by: David Sterba <dsterba@suse.com>
>> Signed-off-by: David Sterba <dsterba@suse.com>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>> fs/btrfs/disk-io.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index eab5a9065f093..439b5f5dc3274 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3864,6 +3864,15 @@ void close_ctree(struct btrfs_root *root)
>> clear_bit(BTRFS_FS_OPEN, &fs_info->flags);
>> free_root_pointers(fs_info, true);
>>
>> + /*
>> + * We must free the block groups after dropping the fs_roots as we could
>> + * have had an IO error and have left over tree log blocks that aren't
>> + * cleaned up until the fs roots are freed. This makes the block group
>> + * accounting appear to be wrong because there's pending reserved bytes,
>> + * so make sure we do the block group cleanup afterwards.
>> + */
>> + btrfs_free_block_groups(fs_info);
>
>Something's wrong here. The patch 4e19443da1 moves the
>btrfs_free_block_groups() call and the stable backport lacks the "-"
>line. However the patch applies cleanly on 4.9.213.
>
>3855 btrfs_free_block_groups(fs_info);
>^^^^
>
>3856
>3857 /*
>3858 * we must make sure there is not any read request to
>3859 * submit after we stopping all workers.
>3860 */
>3861 invalidate_inode_pages2(fs_info->btree_inode->i_mapping);
>3862 btrfs_stop_all_workers(fs_info);
>3863
>3864 clear_bit(BTRFS_FS_OPEN, &fs_info->flags);
>3865 free_root_pointers(fs_info, 1);
>3866
>3867 /*
>3868 * We must free the block groups after dropping the fs_roots as we could
>3869 * have had an IO error and have left over tree log blocks that aren't
>3870 * cleaned up until the fs roots are freed. This makes the block group
>3871 * accounting appear to be wrong because there's pending reserved bytes,
>3872 * so make sure we do the block group cleanup afterwards.
>3873 */
>3874 btrfs_free_block_groups(fs_info);
>
>The first one should not be there.

Sigh, sorry about that. The story behind this is that for this patch to
apply, we first must have 5cdd7db6c5c9 ("Btrfs: fix assertion failure
when freeing block groups at close_ctree()") which moves the
btrfs_free_block_groups() call to line 3863 in your code above.

I somehow goofed up picking it up and (probably) messed up the rebase
when it went missing, sorry again.

I'll fix it up for the next release.

--
Thanks,
Sasha

\
 
 \ /
  Last update: 2020-02-14 02:15    [W:0.073 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site