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@xxxxxxxxxxxxxx>
[ 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@xxxxxxxxxxxxxxx # 4.9+
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
Reviewed-by: David Sterba <dsterba@xxxxxxxx>
Signed-off-by: David Sterba <dsterba@xxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
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
