Remove from cleanup path mutex_unlock via guard() and kfree via
__free(kfree) macro. With those two cleanups gone, we can remove `out`
label and replace all gotos with direct returns.

Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
---
 fs/btrfs/qgroup.c | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 9b2f2c8ca505..238c17c7d969 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1528,65 +1528,55 @@ static int quick_update_accounting(struct btrfs_fs_info *fs_info,
  * callers and transferred here (either used or freed on error).
  */
 int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src, u64 dst,
-			      struct btrfs_qgroup_list *prealloc)
+			      struct btrfs_qgroup_list *_prealloc)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_qgroup *parent;
 	struct btrfs_qgroup *member;
 	struct btrfs_qgroup_list *list;
+	struct btrfs_qgroup_list *prealloc __free(kfree) = _prealloc;
 	int ret = 0;
 
 	ASSERT(prealloc);
 
 	/* Check the level of src and dst first */
 	if (btrfs_qgroup_level(src) >= btrfs_qgroup_level(dst)) {
-		kfree(prealloc);
 		return -EINVAL;
 	}
 
-	mutex_lock(&fs_info->qgroup_ioctl_lock);
-	if (!fs_info->quota_root) {
-		ret = -ENOTCONN;
-		goto out;
-	}
+	guard(mutex)(&fs_info->qgroup_ioctl_lock);
+
+	if (!fs_info->quota_root)
+		return -ENOTCONN;
+
 	member = find_qgroup_rb(fs_info, src);
 	parent = find_qgroup_rb(fs_info, dst);
-	if (!member || !parent) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (!member || !parent)
+		return -EINVAL;
 
 	/* check if such qgroup relation exist firstly */
 	list_for_each_entry(list, &member->groups, next_group) {
-		if (list->group == parent) {
-			ret = -EEXIST;
-			goto out;
-		}
+		if (list->group == parent)
+			return -EEXIST;
 	}
 
 	ret = add_qgroup_relation_item(trans, src, dst);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = add_qgroup_relation_item(trans, dst, src);
 	if (ret) {
 		del_qgroup_relation_item(trans, src, dst);
-		goto out;
+		return ret;
 	}
 
-	spin_lock(&fs_info->qgroup_lock);
+	guard(spinlock)(&fs_info->qgroup_lock);
 	ret = __add_relation_rb(prealloc, member, parent);
 	prealloc = NULL;
 	if (ret < 0) {
-		spin_unlock(&fs_info->qgroup_lock);
-		goto out;
+		return ret;
 	}
-	ret = quick_update_accounting(fs_info, src, dst, 1);
-	spin_unlock(&fs_info->qgroup_lock);
-out:
-	kfree(prealloc);
-	mutex_unlock(&fs_info->qgroup_ioctl_lock);
-	return ret;
+	return quick_update_accounting(fs_info, src, dst, 1);
 }
 
 static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
-- 
2.51.1.dirty

在 2025/11/13 05:19, Gladyshev Ilya 写道:
> Remove from cleanup path mutex_unlock via guard() and kfree via
> __free(kfree) macro. With those two cleanups gone, we can remove `out`
> label and replace all gotos with direct returns.
> 
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> ---
>   fs/btrfs/qgroup.c | 42 ++++++++++++++++--------------------------
>   1 file changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 9b2f2c8ca505..238c17c7d969 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1528,65 +1528,55 @@ static int quick_update_accounting(struct btrfs_fs_info *fs_info,
>    * callers and transferred here (either used or freed on error).
>    */
>   int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src, u64 dst,
> -			      struct btrfs_qgroup_list *prealloc)
> +			      struct btrfs_qgroup_list *_prealloc)

We don't use '_' prefix. Even '__' usage is discouraged.

>   {
>   	struct btrfs_fs_info *fs_info = trans->fs_info;
>   	struct btrfs_qgroup *parent;
>   	struct btrfs_qgroup *member;
>   	struct btrfs_qgroup_list *list;
> +	struct btrfs_qgroup_list *prealloc __free(kfree) = _prealloc;

I do not think preallocation is a good use case for scope based cleanup, 
especially when there are two similarly named variables.

This reduces the readability.

Thanks,
Qu

>   	int ret = 0;
>   
>   	ASSERT(prealloc);
>   
>   	/* Check the level of src and dst first */
>   	if (btrfs_qgroup_level(src) >= btrfs_qgroup_level(dst)) {
> -		kfree(prealloc);
>   		return -EINVAL;
>   	}
>   
> -	mutex_lock(&fs_info->qgroup_ioctl_lock);
> -	if (!fs_info->quota_root) {
> -		ret = -ENOTCONN;
> -		goto out;
> -	}
> +	guard(mutex)(&fs_info->qgroup_ioctl_lock);
> +
> +	if (!fs_info->quota_root)
> +		return -ENOTCONN;
> +
>   	member = find_qgroup_rb(fs_info, src);
>   	parent = find_qgroup_rb(fs_info, dst);
> -	if (!member || !parent) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> +	if (!member || !parent)
> +		return -EINVAL;
>   
>   	/* check if such qgroup relation exist firstly */
>   	list_for_each_entry(list, &member->groups, next_group) {
> -		if (list->group == parent) {
> -			ret = -EEXIST;
> -			goto out;
> -		}
> +		if (list->group == parent)
> +			return -EEXIST;
>   	}
>   
>   	ret = add_qgroup_relation_item(trans, src, dst);
>   	if (ret)
> -		goto out;
> +		return ret;
>   
>   	ret = add_qgroup_relation_item(trans, dst, src);
>   	if (ret) {
>   		del_qgroup_relation_item(trans, src, dst);
> -		goto out;
> +		return ret;
>   	}
>   
> -	spin_lock(&fs_info->qgroup_lock);
> +	guard(spinlock)(&fs_info->qgroup_lock);
>   	ret = __add_relation_rb(prealloc, member, parent);
>   	prealloc = NULL;
>   	if (ret < 0) {
> -		spin_unlock(&fs_info->qgroup_lock);
> -		goto out;
> +		return ret;
>   	}
> -	ret = quick_update_accounting(fs_info, src, dst, 1);
> -	spin_unlock(&fs_info->qgroup_lock);
> -out:
> -	kfree(prealloc);
> -	mutex_unlock(&fs_info->qgroup_ioctl_lock);
> -	return ret;
> +	return quick_update_accounting(fs_info, src, dst, 1);
>   }
>   
>   static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,