Re: [PATCH 1/5] btrfs: drop log root for dropped roots

[Josef Bacik <josef@xxxxxxxxxxxxxx>]

On 12/6/19 10:03 AM, 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.

We need the trans handle to even call btrfs_free_log, we're just fixing it so 
the trans handle can be passed in, making its separate is just superfluous.  Thanks,

Josef