On Fri, Dec 06, 2019 at 05:03:36PM +0200, Nikolay Borisov wrote:
> On 6.12.19 г. 16:37 ч., Josef Bacik wrote:
> > If we fsync on a subvolume and create a log root for that volume, and
> > then later delete that subvolume we'll never clean up its log root. Fix
> > this by making switch_commit_roots free the log for any dropped roots we
> > encounter.
> >
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > ---
> > fs/btrfs/transaction.c | 22 ++++++++++++----------
> > 1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> > index cfc08ef9b876..55d8fd68775a 100644
> > --- a/fs/btrfs/transaction.c
> > +++ b/fs/btrfs/transaction.c
> > @@ -147,13 +147,14 @@ void btrfs_put_transaction(struct btrfs_transaction *transaction)
> > }
> > }
> >
> > -static noinline void switch_commit_roots(struct btrfs_transaction *trans)
> > +static noinline void switch_commit_roots(struct btrfs_trans_handle *trans)
> > {
> > + struct btrfs_transaction *cur_trans = trans->transaction;
> > struct btrfs_fs_info *fs_info = trans->fs_info;
> > struct btrfs_root *root, *tmp;
> >
> > down_write(&fs_info->commit_root_sem);
> > - list_for_each_entry_safe(root, tmp, &trans->switch_commits,
> > + list_for_each_entry_safe(root, tmp, &cur_trans->switch_commits,
> > dirty_list) {
> > list_del_init(&root->dirty_list);
> > free_extent_buffer(root->commit_root);
> > @@ -165,16 +166,17 @@ static noinline void switch_commit_roots(struct btrfs_transaction *trans)
> > }
> >
> > /* We can free old roots now. */
> > - spin_lock(&trans->dropped_roots_lock);
> > - while (!list_empty(&trans->dropped_roots)) {
> > - root = list_first_entry(&trans->dropped_roots,
> > + spin_lock(&cur_trans->dropped_roots_lock);
> > + while (!list_empty(&cur_trans->dropped_roots)) {
> > + root = list_first_entry(&cur_trans->dropped_roots,
> > struct btrfs_root, root_list);
> > list_del_init(&root->root_list);
> > - spin_unlock(&trans->dropped_roots_lock);
> > + spin_unlock(&cur_trans->dropped_roots_lock);
> > + btrfs_free_log(trans, root);
>
> THis patch should really have been this line and converting
> switch_commit_roots to taking a trans handle another patch. Otherwise
> this is lost in the mechanical refactoring.
Agreed, as this is a bugfix, we want it backported to older threes so
minimizing the potential conflicts is desired. For that the order 1)
fix 2) cleanup, seems appropriate.
