| Message ID | 20200117135649.41983-1-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] btrfs: free block groups after free'ing fs trees | expand |
On Fri, Jan 17, 2020 at 08:56:49AM -0500, Josef Bacik wrote: > 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. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > - Nothing has changed in this since last submission, it was simply rebased. > > fs/btrfs/disk-io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index d453bdc74e91..55a03a21d752 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -4056,12 +4056,12 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info) > invalidate_inode_pages2(fs_info->btree_inode->i_mapping); > btrfs_stop_all_workers(fs_info); > > - btrfs_free_block_groups(fs_info); > - > clear_bit(BTRFS_FS_OPEN, &fs_info->flags); > free_root_pointers(fs_info, true); > btrfs_free_fs_roots(fs_info); > > + btrfs_free_block_groups(fs_info); Each step in the shutdown sequence should be documented so we don't shuffle the lines like that, the patch that moved btrfs_free_block_groups to the original place fixed another bug (5cdd7db6c5c9d33dc66). > + > iput(fs_info->btree_inode); > > #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY > -- > 2.24.1
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index d453bdc74e91..55a03a21d752 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -4056,12 +4056,12 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info) invalidate_inode_pages2(fs_info->btree_inode->i_mapping); btrfs_stop_all_workers(fs_info); - btrfs_free_block_groups(fs_info); - clear_bit(BTRFS_FS_OPEN, &fs_info->flags); free_root_pointers(fs_info, true); btrfs_free_fs_roots(fs_info); + btrfs_free_block_groups(fs_info); + iput(fs_info->btree_inode); #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
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. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- - Nothing has changed in this since last submission, it was simply rebased. fs/btrfs/disk-io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)