[RFC PATCH 2/8] btrfs: move kfree out of btrfs_create_qgroup's cleanup path

Gladyshev Ilya posted 8 patches 2 months, 3 weeks ago
[RFC PATCH 2/8] btrfs: move kfree out of btrfs_create_qgroup's cleanup path
Posted by Gladyshev Ilya 2 months, 3 weeks ago
Relocate kfree() from the generic cleanup path to the specific error
exit where the allocation could leak. This prepares for future
simplification by allowing removal of the 'out' label and use of
mutex_guard for cleaner resource management.

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

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 9904bcfd3a60..a8474d0a9c58 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1659,7 +1659,7 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *quota_root;
 	struct btrfs_qgroup *qgroup;
-	struct btrfs_qgroup *prealloc = NULL;
+	struct btrfs_qgroup *prealloc;
 	int ret = 0;
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
@@ -1681,18 +1681,18 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 	}
 
 	ret = add_qgroup_item(trans, quota_root, qgroupid);
-	if (ret)
+	if (ret) {
+		kfree(prealloc);
 		goto out;
+	}
 
 	spin_lock(&fs_info->qgroup_lock);
 	qgroup = add_qgroup_rb(fs_info, prealloc, qgroupid);
 	spin_unlock(&fs_info->qgroup_lock);
-	prealloc = NULL;
 
 	ret = btrfs_sysfs_add_one_qgroup(fs_info, qgroup);
 out:
 	mutex_unlock(&fs_info->qgroup_ioctl_lock);
-	kfree(prealloc);
 	return ret;
 }
 
-- 
2.51.1.dirty
Re: [RFC PATCH 2/8] btrfs: move kfree out of btrfs_create_qgroup's cleanup path
Posted by Qu Wenruo 2 months, 3 weeks ago

在 2025/11/13 05:19, Gladyshev Ilya 写道:
> Relocate kfree() from the generic cleanup path to the specific error
> exit where the allocation could leak. This prepares for future
> simplification by allowing removal of the 'out' label and use of
> mutex_guard for cleaner resource management.
> 
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> ---
>   fs/btrfs/qgroup.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 9904bcfd3a60..a8474d0a9c58 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1659,7 +1659,7 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>   	struct btrfs_fs_info *fs_info = trans->fs_info;
>   	struct btrfs_root *quota_root;
>   	struct btrfs_qgroup *qgroup;
> -	struct btrfs_qgroup *prealloc = NULL;
> +	struct btrfs_qgroup *prealloc;
>   	int ret = 0;
>   
>   	mutex_lock(&fs_info->qgroup_ioctl_lock);
> @@ -1681,18 +1681,18 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>   	}
>   
>   	ret = add_qgroup_item(trans, quota_root, qgroupid);
> -	if (ret)
> +	if (ret) {
> +		kfree(prealloc);
>   		goto out;
> +	}
>   
>   	spin_lock(&fs_info->qgroup_lock);
>   	qgroup = add_qgroup_rb(fs_info, prealloc, qgroupid);
>   	spin_unlock(&fs_info->qgroup_lock);
> -	prealloc = NULL;
>   
>   	ret = btrfs_sysfs_add_one_qgroup(fs_info, qgroup);
>   out:
>   	mutex_unlock(&fs_info->qgroup_ioctl_lock);

You're not on the latest for-next branch, which has the following patch 
applied doing the extra sanity checks:

https://lore.kernel.org/linux-btrfs/20251024102143.236665-5-mssola@mssola.com/

With the extra ASSERT()s, the old code makes more sense.

Thanks,
Qu
> -	kfree(prealloc);
>   	return ret;
>   }
>