thr3ads.net - Btrfs devel - [PATCH 1/7] Btrfs: introduce a mutex lock for btrfs quota operations [Mar 2013]

If this information is useful, please help other people find it:
Share via:

Wang Shilong

2013-Mar-26 11:55 UTC

[PATCH 1/7] Btrfs: introduce a mutex lock for btrfs quota operations

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

The original code only has one spin_lock ''qgroup_lock'' to
protect
quota configurations on memory, if we want to add a BTRFS_QGROUP_INFO_KEY,
it will be added to Btree firstly and then update quota configuration
on memory,this case works ok just because of Btrfs internal btree lock.
However, if we want to add a BTRFS_QGROUP_RELATION_KEY, this will cause
problems, in fact,we need to check ''src'' and
''dst'' before, without an extra lock,
race conditions can not be avoided.

Holding mutex_lock, we can have the check before operations in safety.
Besides, we can remove some spin_lock usages and ease the burdern
of the global spin_lock.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h   |    1 +
 fs/btrfs/disk-io.c |    1 +
 fs/btrfs/ioctl.c   |   13 ++++++++++---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 6e81860..a7fec8c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1582,6 +1582,7 @@ struct btrfs_fs_info {
 
 	/* holds configuration and tracking. Protected by qgroup_lock */
 	struct rb_root qgroup_tree;
+	struct mutex quota_lock;
 	spinlock_t qgroup_lock;
 
 	/* list of dirty qgroups to be written at next commit */
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index fe82d08..4552f14 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2250,6 +2250,7 @@ int open_ctree(struct super_block *sb,
 	mutex_init(&fs_info->dev_replace.lock);
 
 	spin_lock_init(&fs_info->qgroup_lock);
+	mutex_init(&fs_info->quota_lock);
 	fs_info->qgroup_tree = RB_ROOT;
 	INIT_LIST_HEAD(&fs_info->dirty_qgroups);
 	fs_info->qgroup_seq = 1;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 222ce84..0b8ab81 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3693,6 +3693,7 @@ static long btrfs_ioctl_quota_ctl(struct file *file, void
__user *arg)
 		goto drop_write;
 	}
 
+	mutex_lock(&root->fs_info->quota_lock);
 	down_read(&root->fs_info->subvol_sem);
 	if (sa->cmd != BTRFS_QUOTA_CTL_RESCAN) {
 		trans = btrfs_start_transaction(root, 2);
@@ -3728,6 +3729,7 @@ static long btrfs_ioctl_quota_ctl(struct file *file, void
__user *arg)
 out:
 	kfree(sa);
 	up_read(&root->fs_info->subvol_sem);
+	mutex_unlock(&root->fs_info->quota_lock);
 drop_write:
 	mnt_drop_write_file(file);
 	return ret;
@@ -3754,6 +3756,7 @@ static long btrfs_ioctl_qgroup_assign(struct file *file,
void __user *arg)
 		goto drop_write;
 	}
 
+	mutex_lock(&root->fs_info->quota_lock);
 	trans = btrfs_join_transaction(root);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
@@ -3775,6 +3778,7 @@ static long btrfs_ioctl_qgroup_assign(struct file *file,
void __user *arg)
 
 out:
 	kfree(sa);
+	mutex_unlock(&root->fs_info->quota_lock);
 drop_write:
 	mnt_drop_write_file(file);
 	return ret;
@@ -3805,11 +3809,11 @@ static long btrfs_ioctl_qgroup_create(struct file *file,
void __user *arg)
 		ret = -EINVAL;
 		goto out;
 	}
-
+	mutex_lock(&root->fs_info->quota_lock);
 	trans = btrfs_join_transaction(root);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
-		goto out;
+		goto out_unlock;
 	}
 
 	/* FIXME: check if the IDs really exist */
@@ -3824,6 +3828,8 @@ static long btrfs_ioctl_qgroup_create(struct file *file,
void __user *arg)
 	if (err && !ret)
 		ret = err;
 
+out_unlock:
+	mutex_unlock(&root->fs_info->quota_lock);
 out:
 	kfree(sa);
 drop_write:
@@ -3852,7 +3858,7 @@ static long btrfs_ioctl_qgroup_limit(struct file *file,
void __user *arg)
 		ret = PTR_ERR(sa);
 		goto drop_write;
 	}
-
+	mutex_lock(&root->fs_info->quota_lock);
 	trans = btrfs_join_transaction(root);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
@@ -3874,6 +3880,7 @@ static long btrfs_ioctl_qgroup_limit(struct file *file,
void __user *arg)
 
 out:
 	kfree(sa);
+	mutex_unlock(&root->fs_info->quota_lock);
 drop_write:
 	mnt_drop_write_file(file);
 	return ret;
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-26 11:55 UTC

head link

[PATCH 2/7] Btrfs: add a necessary check before updating qgroup limit

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

Add a necessary check before updating qgroup limit,
if the relative qgroup dosen''t exsit, do not join transaction.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ioctl.c |   43 ++++++++++++++++++++++++++++++++++++-------
 1 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 0b8ab81..3c4fde0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3837,6 +3837,33 @@ drop_write:
 	return ret;
 }
 
+static int may_qgroup_limit(struct btrfs_root *root, u64 qgroupid)
+{
+	int ret = 0;
+	struct btrfs_key key;
+	struct btrfs_path *path;
+
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	key.objectid = 0;
+	key.type = BTRFS_QGROUP_INFO_KEY;
+	key.offset = qgroupid;
+
+	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
+				&key, path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	btrfs_free_path(path);
+	return ret;
+}
+
 static long btrfs_ioctl_qgroup_limit(struct file *file, void __user *arg)
 {
 	struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
@@ -3859,19 +3886,21 @@ static long btrfs_ioctl_qgroup_limit(struct file *file,
void __user *arg)
 		goto drop_write;
 	}
 	mutex_lock(&root->fs_info->quota_lock);
-	trans = btrfs_join_transaction(root);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		goto out;
-	}
-
 	qgroupid = sa->qgroupid;
 	if (!qgroupid) {
 		/* take the current subvol as qgroup */
 		qgroupid = root->root_key.objectid;
 	}
+	ret = may_qgroup_limit(root, qgroupid);
+	if (ret)
+		goto out;
+
+	trans = btrfs_join_transaction(root);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out;
+	}
 
-	/* FIXME: check if the IDs really exist */
 	ret = btrfs_limit_qgroup(trans, root->fs_info, qgroupid, &sa->lim);
 
 	err = btrfs_end_transaction(trans, root);
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-26 11:55 UTC

head link

[PATCH 3/7] Btrfs: add a necessary check before creating/destroying a qgroup

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

This patch try to add a check before creating/destroying a qgroup.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ioctl.c |   41 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 3c4fde0..8a01326 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3784,6 +3784,42 @@ drop_write:
 	return ret;
 }
 
+static int may_qgroup_create(struct btrfs_root *root,
+			     struct btrfs_ioctl_qgroup_create_args *sa)
+{
+	int ret = 0;
+	struct btrfs_key key;
+	struct btrfs_path *path;
+
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	key.objectid = 0;
+	key.type = BTRFS_QGROUP_INFO_KEY;
+	key.offset = sa->qgroupid;
+
+	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
+				&key, path, 0, 0);
+	if (sa->create) {
+		if (!ret)
+			ret = -EEXIST;
+		if (ret > 0)
+			ret = 0;
+		goto out;
+	}
+	if (ret > 0)
+		ret = -ENOENT;
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 static long btrfs_ioctl_qgroup_create(struct file *file, void __user *arg)
 {
 	struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
@@ -3810,13 +3846,16 @@ static long btrfs_ioctl_qgroup_create(struct file *file,
void __user *arg)
 		goto out;
 	}
 	mutex_lock(&root->fs_info->quota_lock);
+	ret = may_qgroup_create(root, sa);
+	if (ret)
+		goto out_unlock;
+
 	trans = btrfs_join_transaction(root);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
 		goto out_unlock;
 	}
 
-	/* FIXME: check if the IDs really exist */
 	if (sa->create) {
 		ret = btrfs_create_qgroup(trans, root->fs_info, sa->qgroupid,
 					  NULL);
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-26 11:55 UTC

head link

[PATCH 4/7] Btrfs: add a necessary check before assigning/removing qgroup relations

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

This is one of most important reason why we introduce a mutex here,
the previous code dosen''t check ''src'' and
''dst'' before assigning/removing
a qgroup relations. Without the mutex lock, and checks may leads
a race conditon.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ioctl.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 80 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 8a01326..1cd96f6 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3735,6 +3735,82 @@ drop_write:
 	return ret;
 }
 
+static int may_qgroup_assign(struct btrfs_root *root,
+			     struct btrfs_ioctl_qgroup_assign_args *sa)
+{
+	int ret = 0;
+	struct btrfs_key key;
+	struct btrfs_path *path;
+
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	key.objectid = 0;
+	key.type = BTRFS_QGROUP_INFO_KEY;
+	key.offset = sa->src;
+	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
+				&key, path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+
+	btrfs_release_path(path);
+	key.objectid = 0;
+	key.type = BTRFS_QGROUP_INFO_KEY;
+	key.offset = sa->dst;
+	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
+				&key, path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+
+	btrfs_release_path(path);
+	key.objectid = sa->src;
+	key.type = BTRFS_QGROUP_RELATION_KEY;
+	key.offset = sa->dst;
+	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
+				&key, path, 0, 0);
+	if (sa->assign) {
+		if (!ret)
+			ret = -EEXIST;
+		if (ret < 0)
+			goto out;
+	} else {
+		if (ret > 0)
+			ret = -ENOENT;
+		if (ret < 0)
+			goto out;
+	}
+
+	btrfs_release_path(path);
+	key.objectid = sa->dst;
+	key.type = BTRFS_QGROUP_RELATION_KEY;
+	key.offset = sa->src;
+	if (sa->assign) {
+		if (!ret)
+			ret = -EEXIST;
+		if (ret < 0)
+			goto out;
+	} else {
+		if (ret > 0)
+			ret = -ENOENT;
+		if (ret < 0)
+			goto out;
+	}
+	ret = 0;
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg)
 {
 	struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
@@ -3757,13 +3833,16 @@ static long btrfs_ioctl_qgroup_assign(struct file *file,
void __user *arg)
 	}
 
 	mutex_lock(&root->fs_info->quota_lock);
+	ret = may_qgroup_assign(root, sa);
+	if (ret)
+		goto out;
+
 	trans = btrfs_join_transaction(root);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
 		goto out;
 	}
 
-	/* FIXME: check if the IDs really exist */
 	if (sa->assign) {
 		ret = btrfs_add_qgroup_relation(trans, root->fs_info,
 						sa->src, sa->dst);
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-26 11:55 UTC

head link

[PATCH 5/7] Btrfs: fix missing check before btrfs_qgroup_inherit()

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

The original code dosen''t have necessary checks before
doing qgroup_inherit. Fix it.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ioctl.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 1cd96f6..86e7fb3 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -708,6 +708,49 @@ static inline int btrfs_may_create(struct inode *dir,
struct dentry *child)
 	return inode_permission(dir, MAY_WRITE | MAY_EXEC);
 }
 
+static int check_qgroup_inherit(struct btrfs_root *root,
+				struct btrfs_qgroup_inherit *inherit)
+{
+	int ret = 0;
+	u64 *i_qgroups;
+	u64 nums = 0;
+	u64 i = 0;
+	struct btrfs_path *path;
+	struct btrfs_key key;
+
+	if (!inherit)
+		return 0;
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	i_qgroups = (u64 *)(inherit + 1);
+	nums = inherit->num_qgroups + inherit->num_ref_copies +
+	       inherit->num_excl_copies;
+	for (i = 0; i < nums; ++i) {
+		btrfs_release_path(path);
+		key.objectid = 0;
+		key.type = BTRFS_QGROUP_INFO_KEY;
+		key.offset = *i_qgroups;
+
+		ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
+					&key, path, 0, 0);
+		if (ret > 0)
+			ret = -EINVAL;
+		if (ret)
+			goto out;
+		++i_qgroups;
+	}
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 /*
  * Create a new subvolume below @parent.  This is largely modeled after
  * sys_mkdirat and vfs_mkdir, but we only do a single component lookup
@@ -753,6 +796,11 @@ static noinline int btrfs_mksubvol(struct path *parent,
 	if (btrfs_root_refs(&BTRFS_I(dir)->root->root_item) == 0)
 		goto out_up_read;
 
+	mutex_lock(&BTRFS_I(dir)->root->fs_info->quota_lock);
+	error = check_qgroup_inherit(BTRFS_I(dir)->root, inherit);
+	if (error)
+		goto out_unlock_mutex;
+
 	if (snap_src) {
 		error = create_snapshot(snap_src, dir, dentry, name, namelen,
 					async_transid, readonly, inherit);
@@ -762,6 +810,8 @@ static noinline int btrfs_mksubvol(struct path *parent,
 	}
 	if (!error)
 		fsnotify_mkdir(dir, dentry);
+out_unlock_mutex:
+	mutex_unlock(&BTRFS_I(dir)->root->fs_info->quota_lock);
 out_up_read:
 	up_read(&BTRFS_I(dir)->root->fs_info->subvol_sem);
 out_dput:
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-26 11:55 UTC

head link

[PATCH 6/7] Btrfs: remove unnecessary spin_lock holding for find_qgroup_rb()

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

Since mutex has been used for quota operations, we don''t have to
hold spin_lock when calling find_qgroup_rb.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/qgroup.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index e3598fa..e123d5c 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -98,7 +98,10 @@ struct btrfs_qgroup_list {
 	struct btrfs_qgroup *member;
 };
 
-/* must be called with qgroup_lock held */
+/*
+ * don''t need to be held by spin_lock, because it has been protected
by
+ * quota_lock
+ */
 static struct btrfs_qgroup *find_qgroup_rb(struct btrfs_fs_info *fs_info,
 					   u64 qgroupid)
 {
@@ -1041,15 +1044,12 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle
*trans,
 		return -EINVAL;
 
 	/* check if there are no relations to this qgroup */
-	spin_lock(&fs_info->qgroup_lock);
 	qgroup = find_qgroup_rb(fs_info, qgroupid);
 	if (qgroup) {
 		if (!list_empty(&qgroup->groups) ||
!list_empty(&qgroup->members)) {
-			spin_unlock(&fs_info->qgroup_lock);
 			return -EBUSY;
 		}
 	}
-	spin_unlock(&fs_info->qgroup_lock);
 
 	ret = del_qgroup_item(trans, quota_root, qgroupid);
 
@@ -1081,22 +1081,19 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans,
 		       (unsigned long long)qgroupid);
 	}
 
-	spin_lock(&fs_info->qgroup_lock);
-
 	qgroup = find_qgroup_rb(fs_info, qgroupid);
 	if (!qgroup) {
 		ret = -ENOENT;
-		goto unlock;
+		return ret;
 	}
+	spin_lock(&fs_info->qgroup_lock);
 	qgroup->lim_flags = limit->flags;
 	qgroup->max_rfer = limit->max_rfer;
 	qgroup->max_excl = limit->max_excl;
 	qgroup->rsv_rfer = limit->rsv_rfer;
 	qgroup->rsv_excl = limit->rsv_excl;
 
-unlock:
 	spin_unlock(&fs_info->qgroup_lock);
-
 	return ret;
 }
 
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-26 11:55 UTC

head link

[PATCH 7/7] Btrfs: remove reduplicate check about quota_root in qgroup.c

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

The check has been done just before starting/joining a transaction,
so we don''t need to check it again.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/qgroup.c |   20 --------------------
 1 files changed, 0 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index e123d5c..6fb67df 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -927,9 +927,6 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
 	btrfs_free_qgroup_config(fs_info);
 	spin_unlock(&fs_info->qgroup_lock);
 
-	if (!quota_root)
-		return -EINVAL;
-
 	ret = btrfs_clean_quota_tree(trans, quota_root);
 	if (ret)
 		goto out;
@@ -965,9 +962,6 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle
*trans,
 	int ret = 0;
 
 	quota_root = fs_info->quota_root;
-	if (!quota_root)
-		return -EINVAL;
-
 	ret = add_qgroup_relation_item(trans, quota_root, src, dst);
 	if (ret)
 		return ret;
@@ -993,9 +987,6 @@ int btrfs_del_qgroup_relation(struct btrfs_trans_handle
*trans,
 	int err;
 
 	quota_root = fs_info->quota_root;
-	if (!quota_root)
-		return -EINVAL;
-
 	ret = del_qgroup_relation_item(trans, quota_root, src, dst);
 	err = del_qgroup_relation_item(trans, quota_root, dst, src);
 	if (err && !ret)
@@ -1017,9 +1008,6 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans,
 	int ret = 0;
 
 	quota_root = fs_info->quota_root;
-	if (!quota_root)
-		return -EINVAL;
-
 	ret = add_qgroup_item(trans, quota_root, qgroupid);
 
 	spin_lock(&fs_info->qgroup_lock);
@@ -1040,8 +1028,6 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans,
 	int ret = 0;
 
 	quota_root = fs_info->quota_root;
-	if (!quota_root)
-		return -EINVAL;
 
 	/* check if there are no relations to this qgroup */
 	qgroup = find_qgroup_rb(fs_info, qgroupid);
@@ -1068,9 +1054,6 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans,
 	struct btrfs_qgroup *qgroup;
 	int ret = 0;
 
-	if (!quota_root)
-		return -EINVAL;
-
 	ret = update_qgroup_limit_item(trans, quota_root, qgroupid,
 				       limit->flags, limit->max_rfer,
 				       limit->max_excl, limit->rsv_rfer,
@@ -1394,9 +1377,6 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
 	if (!fs_info->quota_enabled)
 		return 0;
 
-	if (!quota_root)
-		return -EINVAL;
-
 	/*
 	 * create a tracking group for the subvol itself
 	 */
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-26 12:01 UTC

head link

Re: [PATCH 4/7] Btrfs: add a necessary check before assigning/removing qgroup relations

Hello Arne,

I have sent a patch-set about btrfs quota,
would you please review them for me. Any comment is welcome.

Thanks,
Wang
> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> This is one of most important reason why we introduce a mutex here,
> the previous code dosen''t check ''src'' and
''dst'' before assigning/removing
> a qgroup relations. Without the mutex lock, and checks may leads
> a race conditon.
> 
> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
> fs/btrfs/ioctl.c |   81
+++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 80 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 8a01326..1cd96f6 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3735,6 +3735,82 @@ drop_write:
> 	return ret;
> }
> 
> +static int may_qgroup_assign(struct btrfs_root *root,
> +			     struct btrfs_ioctl_qgroup_assign_args *sa)
> +{
> +	int ret = 0;
> +	struct btrfs_key key;
> +	struct btrfs_path *path;
> +
> +	if (!root->fs_info->quota_root)
> +		return -EINVAL;
> +
> +	path = btrfs_alloc_path();
> +	if (!path) {
> +		ret = -ENOMEM;
> +		return ret;
> +	}
> +
> +	key.objectid = 0;
> +	key.type = BTRFS_QGROUP_INFO_KEY;
> +	key.offset = sa->src;
> +	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
> +				&key, path, 0, 0);
> +	if (ret > 0)
> +		ret = -ENOENT;
> +	if (ret < 0)
> +		goto out;
> +
> +	btrfs_release_path(path);
> +	key.objectid = 0;
> +	key.type = BTRFS_QGROUP_INFO_KEY;
> +	key.offset = sa->dst;
> +	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
> +				&key, path, 0, 0);
> +	if (ret > 0)
> +		ret = -ENOENT;
> +	if (ret < 0)
> +		goto out;
> +
> +	btrfs_release_path(path);
> +	key.objectid = sa->src;
> +	key.type = BTRFS_QGROUP_RELATION_KEY;
> +	key.offset = sa->dst;
> +	ret = btrfs_search_slot(NULL, root->fs_info->quota_root,
> +				&key, path, 0, 0);
> +	if (sa->assign) {
> +		if (!ret)
> +			ret = -EEXIST;
> +		if (ret < 0)
> +			goto out;
> +	} else {
> +		if (ret > 0)
> +			ret = -ENOENT;
> +		if (ret < 0)
> +			goto out;
> +	}
> +
> +	btrfs_release_path(path);
> +	key.objectid = sa->dst;
> +	key.type = BTRFS_QGROUP_RELATION_KEY;
> +	key.offset = sa->src;
> +	if (sa->assign) {
> +		if (!ret)
> +			ret = -EEXIST;
> +		if (ret < 0)
> +			goto out;
> +	} else {
> +		if (ret > 0)
> +			ret = -ENOENT;
> +		if (ret < 0)
> +			goto out;
> +	}
> +	ret = 0;
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
> static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg)
> {
> 	struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
> @@ -3757,13 +3833,16 @@ static long btrfs_ioctl_qgroup_assign(struct file
*file, void __user *arg)
> 	}
> 
> 	mutex_lock(&root->fs_info->quota_lock);
> +	ret = may_qgroup_assign(root, sa);
> +	if (ret)
> +		goto out;
> +
> 	trans = btrfs_join_transaction(root);
> 	if (IS_ERR(trans)) {
> 		ret = PTR_ERR(trans);
> 		goto out;
> 	}
> 
> -	/* FIXME: check if the IDs really exist */
> 	if (sa->assign) {
> 		ret = btrfs_add_qgroup_relation(trans, root->fs_info,
> 						sa->src, sa->dst);
> -- 
> 1.7.7.6
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wang Shilong

2013-Mar-27 10:27 UTC

head link

Re: [PATCH 1/7] Btrfs: introduce a mutex lock for btrfs quota operations

Hi,

Please ignore this patchset.
V2 has been made and will be sent out soon!

The main change is to use quota configurations 
on memory to speed up ioctls check.

Thanks,
Wang

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Btrfs devel - Mar 2013 - [PATCH 1/7] Btrfs: introduce a mutex lock for btrfs quota operations

[PATCH 1/7] Btrfs: introduce a mutex lock for btrfs quota operations

[PATCH 2/7] Btrfs: add a necessary check before updating qgroup limit

[PATCH 3/7] Btrfs: add a necessary check before creating/destroying a qgroup

[PATCH 4/7] Btrfs: add a necessary check before assigning/removing qgroup relations

[PATCH 5/7] Btrfs: fix missing check before btrfs_qgroup_inherit()

[PATCH 6/7] Btrfs: remove unnecessary spin_lock holding for find_qgroup_rb()

[PATCH 7/7] Btrfs: remove reduplicate check about quota_root in qgroup.c

Re: [PATCH 4/7] Btrfs: add a necessary check before assigning/removing qgroup relations

Re: [PATCH 1/7] Btrfs: introduce a mutex lock for btrfs quota operations