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. Now fixed up by just dropping this patch entirely :) Sasha, can you fix it up and add it back for the next round? thanks, greg k-h
