From 4addc1ffd67ad34394674dc91379dc04cfdd2537 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Wed, 22 May 2024 18:17:05 +0200 Subject: [PATCH] btrfs: qgroup: preallocate memory before adding a relation There's a transaction joined in the qgroup relation add/remove ioctl and any error will lead to abort/error. We could lift the allocation from btrfs_add_qgroup_relation() and move it outside of the transaction context. The relation deletion does not need that. The ownership of the structure is moved to the add relation handler. Reviewed-by: Qu Wenruo Signed-off-by: David Sterba --- fs/btrfs/ioctl.c | 17 ++++++++++++++++- fs/btrfs/qgroup.c | 25 ++++++++----------------- fs/btrfs/qgroup.h | 11 ++++++++++- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index d28ebabe3720..dc7300f2815f 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3829,6 +3829,7 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg) struct btrfs_fs_info *fs_info = inode_to_fs_info(inode); struct btrfs_root *root = BTRFS_I(inode)->root; struct btrfs_ioctl_qgroup_assign_args *sa; + struct btrfs_qgroup_list *prealloc = NULL; struct btrfs_trans_handle *trans; int ret; int err; @@ -3849,14 +3850,27 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg) goto drop_write; } + if (sa->assign) { + prealloc = kzalloc(sizeof(*prealloc), GFP_KERNEL); + if (!prealloc) { + ret = -ENOMEM; + goto drop_write; + } + } + trans = btrfs_join_transaction(root); if (IS_ERR(trans)) { ret = PTR_ERR(trans); goto out; } + /* + * Prealloc ownership is moved to the relation handler, there it's used + * or freed on error. + */ if (sa->assign) { - ret = btrfs_add_qgroup_relation(trans, sa->src, sa->dst); + ret = btrfs_add_qgroup_relation(trans, sa->src, sa->dst, prealloc); + prealloc = NULL; } else { ret = btrfs_del_qgroup_relation(trans, sa->src, sa->dst); } @@ -3873,6 +3887,7 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg) ret = err; out: + kfree(prealloc); kfree(sa); drop_write: mnt_drop_write_file(file); diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index 8b01bfdee820..5d57a285d59b 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -155,16 +155,6 @@ static inline u64 btrfs_qgroup_get_new_refcnt(const struct btrfs_qgroup *qg, u64 return qg->new_refcnt - seq; } -/* - * glue structure to represent the relations between qgroups. - */ -struct btrfs_qgroup_list { - struct list_head next_group; - struct list_head next_member; - struct btrfs_qgroup *group; - struct btrfs_qgroup *member; -}; - static int qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid, int init_flags); @@ -1569,15 +1559,21 @@ out: return ret; } -int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src, u64 dst) +/* + * Add relation between @src and @dst qgroup. The @prealloc is allocated by the + * 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_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 = NULL; int ret = 0; + ASSERT(prealloc); + /* Check the level of src and dst first */ if (btrfs_qgroup_level(src) >= btrfs_qgroup_level(dst)) return -EINVAL; @@ -1602,11 +1598,6 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src, u64 dst } } - prealloc = kzalloc(sizeof(*list), GFP_NOFS); - if (!prealloc) { - ret = -ENOMEM; - goto out; - } ret = add_qgroup_relation_item(trans, src, dst); if (ret) goto out; diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h index 95881dcab684..deb479d176a9 100644 --- a/fs/btrfs/qgroup.h +++ b/fs/btrfs/qgroup.h @@ -278,6 +278,14 @@ struct btrfs_qgroup { struct kobject kobj; }; +/* Glue structure to represent the relations between qgroups. */ +struct btrfs_qgroup_list { + struct list_head next_group; + struct list_head next_member; + struct btrfs_qgroup *group; + struct btrfs_qgroup *member; +}; + struct btrfs_squota_delta { /* The fstree root this delta counts against. */ u64 root; @@ -321,7 +329,8 @@ int btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info); void btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info); int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info *fs_info, bool interruptible); -int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src, u64 dst); +int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src, u64 dst, + struct btrfs_qgroup_list *prealloc); 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);