First of all, thanks for reviewing! On Tue, Jun 26, 2012 at 06:17:39PM +0200, David Sterba wrote: > On Fri, Jun 22, 2012 at 09:24:12PM +0300, Ilya Dryomov wrote: > > Fix a bug that triggered asserts in btrfs_balance() in both normal and > > resume modes -- restriper state was not properly restored on read-only > > mounts. This factors out resuming code from btrfs_restore_balance(), > > which is now also called earlier in the mount sequence to avoid the > > problem of some early writes getting the old profile. > > > > Signed-off-by: Ilya Dryomov <idryo...@gmail.com> > > --- > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > > index 77872da..dae7cd6 100644 > > --- a/fs/btrfs/disk-io.c > > +++ b/fs/btrfs/disk-io.c > > @@ -2492,9 +2497,6 @@ retry_root_backup: > > err = btrfs_orphan_cleanup(fs_info->tree_root); > > up_read(&fs_info->cleanup_work_sem); > > > > - if (!err) > > - err = btrfs_recover_balance(fs_info->tree_root); > > - > > if (err) { > > close_ctree(tree_root); > > return err; > > @@ -2518,6 +2520,9 @@ fail_cleaner: > > fail_block_groups: > > btrfs_free_block_groups(fs_info); > > > > +fail_balance_ctl: > > + kfree(fs_info->balance_ctl); > > I think you need to set fs_info->balance_ctl to NULL, otherwise this > could lead to double free from free_fs_info. I was looking along the
Yes, I do. I meant to call unset_balance_control(fs_info) there, but changed it to kfree(), because of the BUG_ON() in the former. unset_balance_control(), of course, sets ->balance_ctl to NULL ;) > call paths and didn't see free_fs_info called on the mount failure path: > > vfs->mount > btrfs_mount > btrfs_fill_super > open_ctree > (recover balance fails, frees ctl) > > error is propagated back to vfs, no other fs callback is done (like > kill_super which does call free_fs_info). > > The only exit path that is not going through free_fs_info is after error > from btrfs_fill_super, and this can fail from various reasons. > > Either I'm missing something, or we leak a btrfs_fs_info every time a > mount fails ... No, we don't, you just missed it. It's freed from btrfs_kill_super(), which is called from deactivate_locked_super() after btrfs_fill_super() errors out. > > > Back to your patch, apart from the balance_ctl pointer reset, both are > ok and given the number of bug reports [useless padding text here] > > this should go to 3.5-rc. Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html