Re: [PATCH v3 2/2] btrfs: automatically remove the subvolume qgroup

[Qu Wenruo <wqu@suse.com>]

在 2024/5/21 08:33, Qu Wenruo 写道:
> 
> 
> 在 2024/5/21 08:20, Boris Burkov 写道:
> [...]
>>> +    /*
>>> +     * It's squota and the subvolume still has numbers needed
>>> +     * for future accounting, in this case we can not delete.
>>> +     * Just skip it.
>>> +     */
>>
>> Maybe throw in an ASSERT or WARN or whatever you think is best checking
>> for squota mode, if we are sure this shouldn't happen for normal qgroup?
> 
> Sounds good.
> 
> Would add an ASSERT() for making sure it's squota mode.

After more thought, I believe ASSERT() can lead to false alerts.

The problem here is, we do not have any extra race prevention here, 
really rely on one time call on btrfs_remove_qgroup() to do the proper 
locking.

But after btrfs_remove_qgroup() returned -EBUSY, we can race with qgroup 
disabling, thus doing an ASSERT() without the proper lock context can 
lead to false alert, e.g. the qgroup is disabled after 
btrfs_remove_qgroup() call.

So I'm afraid we can not do extra checks here.

Thanks,
Qu
> 
> Thanks,
> Qu
> 
>>
>>> +    if (ret == -EBUSY)
>>> +        ret = 0;
>>> +    return ret;
>>> +}
>>> +
>>>   int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
>>>                  struct btrfs_qgroup_limit *limit)
>>>   {
>>> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
>>> index 706640be0ec2..3f93856a02e1 100644
>>> --- a/fs/btrfs/qgroup.h
>>> +++ b/fs/btrfs/qgroup.h
>>> @@ -327,6 +327,8 @@ int btrfs_del_qgroup_relation(struct 
>>> btrfs_trans_handle *trans, u64 src,
>>>                     u64 dst);
>>>   int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 
>>> qgroupid);
>>>   int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 
>>> qgroupid);
>>> +int btrfs_qgroup_cleanup_dropped_subvolume(struct btrfs_fs_info 
>>> *fs_info,
>>> +                       u64 subvolid);
>>>   int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
>>>                  struct btrfs_qgroup_limit *limit);
>>>   int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info);
>>> -- 
>>> 2.45.0
>>>
>