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

[Josef Bacik <josef@toxicpanda.com>]

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@toxicpanda.com>
>> ---
>>   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