thr3ads.net - Btrfs devel - [PATCH V2 2/6] Btrfs: remove some unnecessary spin

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

Wang Shilong

2013-Mar-28 10:54 UTC

[PATCH V2 2/6] Btrfs: remove some unnecessary spin_lock usages

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

We use mutex_lock to protect all the user change operaions.
So when we are calling find_qgroup_rb() to check whether
qgroup exists, we don''t have to hold spin_lock.

Besides, when enabling/disabling quota,it must be single
thread when operations come to here.Spin_lock must be fistly
used to clear quota_root when disabling quota,while enabling
quota spin_lock must be used to complete the last assign work.

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

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index e3598fa..7df372a 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -42,7 +42,6 @@
  *  - limit
  *  - caches fuer ulists
  *  - performance benchmarks
- *  - check all ioctl parameters
  */
 
 /*
@@ -98,7 +97,11 @@ struct btrfs_qgroup_list {
 	struct btrfs_qgroup *member;
 };
 
-/* must be called with qgroup_lock held */
+/*
+ * don''t need to be held by spin_lock since
+ * all the quota configurations on memory has been protected
+ * by mutex quota_lock.
+ */
 static struct btrfs_qgroup *find_qgroup_rb(struct btrfs_fs_info *fs_info,
 					   u64 qgroupid)
 {
@@ -793,13 +796,10 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
 	int ret = 0;
 	int slot;
 
-	spin_lock(&fs_info->qgroup_lock);
 	if (fs_info->quota_root) {
 		fs_info->pending_quota_state = 1;
-		spin_unlock(&fs_info->qgroup_lock);
-		goto out;
+		return ret;
 	}
-	spin_unlock(&fs_info->qgroup_lock);
 
 	/*
 	 * initially create the quota tree
@@ -808,7 +808,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
 				       BTRFS_QUOTA_TREE_OBJECTID);
 	if (IS_ERR(quota_root)) {
 		ret =  PTR_ERR(quota_root);
-		goto out;
+		return ret;
 	}
 
 	path = btrfs_alloc_path();
@@ -861,14 +861,11 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
 			if (ret)
 				goto out_free_path;
 
-			spin_lock(&fs_info->qgroup_lock);
 			qgroup = add_qgroup_rb(fs_info, found_key.offset);
 			if (IS_ERR(qgroup)) {
-				spin_unlock(&fs_info->qgroup_lock);
 				ret = PTR_ERR(qgroup);
 				goto out_free_path;
 			}
-			spin_unlock(&fs_info->qgroup_lock);
 		}
 		ret = btrfs_next_item(tree_root, path);
 		if (ret < 0)
@@ -883,13 +880,12 @@ out_add_root:
 	if (ret)
 		goto out_free_path;
 
-	spin_lock(&fs_info->qgroup_lock);
 	qgroup = add_qgroup_rb(fs_info, BTRFS_FS_TREE_OBJECTID);
 	if (IS_ERR(qgroup)) {
-		spin_unlock(&fs_info->qgroup_lock);
 		ret = PTR_ERR(qgroup);
 		goto out_free_path;
 	}
+	spin_lock(&fs_info->qgroup_lock);
 	fs_info->quota_root = quota_root;
 	fs_info->pending_quota_state = 1;
 	spin_unlock(&fs_info->qgroup_lock);
@@ -901,7 +897,6 @@ out_free_root:
 		free_extent_buffer(quota_root->commit_root);
 		kfree(quota_root);
 	}
-out:
 	return ret;
 }
 
@@ -912,11 +907,10 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
 	struct btrfs_root *quota_root;
 	int ret = 0;
 
-	spin_lock(&fs_info->qgroup_lock);
-	if (!fs_info->quota_root) {
-		spin_unlock(&fs_info->qgroup_lock);
+	if (!fs_info->quota_root)
 		return 0;
-	}
+
+	spin_lock(&fs_info->qgroup_lock);
 	fs_info->quota_enabled = 0;
 	fs_info->pending_quota_state = 0;
 	quota_root = fs_info->quota_root;
@@ -1041,15 +1035,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);
+		if (!list_empty(&qgroup->groups) ||
+		    !list_empty(&qgroup->members))
 			return -EBUSY;
-		}
 	}
-	spin_unlock(&fs_info->qgroup_lock);
 
 	ret = del_qgroup_item(trans, quota_root, qgroupid);
 
@@ -1081,20 +1072,17 @@ 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-28 10:54 UTC

head link

[PATCH V2 3/6] Btrfs: fix missing check before updating qgroup limit

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

We should check whether a qgroup exists before updating qgroup limit,
if the relative qgroup dosen''t exsit, return -EEXIST and do not join
a transaction, fix it.

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/ioctl.c  |   18 ++++++++++--------
 fs/btrfs/qgroup.c |   15 ++++++++++++---
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a11a8ed..3fc393d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3094,6 +3094,7 @@ int btrfs_make_block_group(struct btrfs_trans_handle
*trans,
 			   struct btrfs_root *root, u64 bytes_used,
 			   u64 type, u64 chunk_objectid, u64 chunk_offset,
 			   u64 size);
+int btrfs_may_limit_qgroup(struct btrfs_root *root, u64 qgroupid);
 int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root, u64 group_start);
 void btrfs_create_pending_block_groups(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e2950f1..7c72b12 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3859,20 +3859,22 @@ 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);
-		goto out;
-	}
-
 	qgroupid = sa->qgroupid;
 	if (!qgroupid) {
 		/* take the current subvol as qgroup */
 		qgroupid = root->root_key.objectid;
 	}
 
-	/* FIXME: check if the IDs really exist */
+	mutex_lock(&root->fs_info->quota_lock);
+	ret = btrfs_may_limit_qgroup(root, qgroupid);
+	if (ret)
+		goto out;
+
+	trans = btrfs_join_transaction(root);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out;
+	}
 	ret = btrfs_limit_qgroup(trans, root->fs_info, qgroupid, &sa->lim);
 
 	err = btrfs_end_transaction(trans, root);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 7df372a..bed0e22 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1059,9 +1059,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,
@@ -1665,3 +1662,15 @@ void assert_qgroups_uptodate(struct btrfs_trans_handle
*trans)
 		trans->delayed_ref_elem.seq);
 	BUG();
 }
+
+int btrfs_may_limit_qgroup(struct btrfs_root *root, u64 qgroupid)
+{
+	struct btrfs_qgroup *qgroup = NULL;
+
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+	qgroup = find_qgroup_rb(root->fs_info, qgroupid);
+	if (!qgroup)
+		return -ENOENT;
+	return 0;
+}
-- 
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-28 10:54 UTC

head link

[PATCH V2 4/6] Btrfs: fix missing check before creating/destroying a qgroup

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

For creating a qgroup, if the qgroup exsits, return -EEXIST.
For destroying a qgroup, if there are qgroup relations, return -EBUSY,
if the qgroup dosen''t exist, return -ENOENT.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h  |    2 ++
 fs/btrfs/ioctl.c  |    5 ++++-
 fs/btrfs/qgroup.c |   45 ++++++++++++++++++++++++++-------------------
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3fc393d..9e4ccb3 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3831,6 +3831,8 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle
*trans,
 			      struct btrfs_fs_info *fs_info, u64 src, u64 dst);
 int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans,
 			      struct btrfs_fs_info *fs_info, u64 src, u64 dst);
+int btrfs_may_create_qgroup(struct btrfs_root *root,
+			    struct btrfs_ioctl_qgroup_create_args *sa);
 int btrfs_create_qgroup(struct btrfs_trans_handle *trans,
 			struct btrfs_fs_info *fs_info, u64 qgroupid,
 			char *name);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7c72b12..f66d622 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3811,13 +3811,16 @@ static long btrfs_ioctl_qgroup_create(struct file *file,
void __user *arg)
 		goto out;
 	}
 	mutex_lock(&root->fs_info->quota_lock);
+	ret = btrfs_may_create_qgroup(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);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index bed0e22..6f57a98 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1003,15 +1003,13 @@ int btrfs_del_qgroup_relation(struct btrfs_trans_handle
*trans,
 int btrfs_create_qgroup(struct btrfs_trans_handle *trans,
 			struct btrfs_fs_info *fs_info, u64 qgroupid, char *name)
 {
-	struct btrfs_root *quota_root;
+	struct btrfs_root *quota_root = fs_info->quota_root;
 	struct btrfs_qgroup *qgroup;
 	int ret = 0;
 
-	quota_root = fs_info->quota_root;
-	if (!quota_root)
-		return -EINVAL;
-
 	ret = add_qgroup_item(trans, quota_root, qgroupid);
+	if (ret)
+		return ret;
 
 	spin_lock(&fs_info->qgroup_lock);
 	qgroup = add_qgroup_rb(fs_info, qgroupid);
@@ -1026,22 +1024,9 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans,
 int btrfs_remove_qgroup(struct btrfs_trans_handle *trans,
 			struct btrfs_fs_info *fs_info, u64 qgroupid)
 {
-	struct btrfs_root *quota_root;
-	struct btrfs_qgroup *qgroup;
+	struct btrfs_root *quota_root = fs_info->quota_root;
 	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);
-	if (qgroup) {
-		if (!list_empty(&qgroup->groups) ||
-		    !list_empty(&qgroup->members))
-			return -EBUSY;
-	}
-
 	ret = del_qgroup_item(trans, quota_root, qgroupid);
 
 	spin_lock(&fs_info->qgroup_lock);
@@ -1674,3 +1659,25 @@ int btrfs_may_limit_qgroup(struct btrfs_root *root, u64
qgroupid)
 		return -ENOENT;
 	return 0;
 }
+
+int btrfs_may_create_qgroup(struct btrfs_root *root,
+			    struct btrfs_ioctl_qgroup_create_args *sa)
+{
+	struct btrfs_qgroup *qgroup = NULL;
+
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+
+	qgroup = find_qgroup_rb(root->fs_info, sa->qgroupid);
+	if (sa->create) {
+		if (qgroup)
+			return -EEXIST;
+		return 0;
+	}
+	if (!qgroup)
+		return -ENOENT;
+	/* check if there are no relations to this qgroup */
+	if (!list_empty(&qgroup->groups) ||
!list_empty(&qgroup->members))
+		return -EBUSY;
+	return 0;
+}
-- 
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-28 10:54 UTC

head link

[PATCH V2 5/6] Btrfs: fix missing check before assigning/removing qgroup relation

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

If the specified qgroup relation dosen''t exist, removing qgroup
operations
will return -ENOENT;

If the specified qgroup relation exists, assigning qgroup relations will
return -EEXIST.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h  |    2 ++
 fs/btrfs/ioctl.c  |    4 +++-
 fs/btrfs/qgroup.c |   40 ++++++++++++++++++++++++++++++----------
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 9e4ccb3..1ca1c63 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3827,6 +3827,8 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
 int btrfs_quota_disable(struct btrfs_trans_handle *trans,
 			struct btrfs_fs_info *fs_info);
 int btrfs_quota_rescan(struct btrfs_fs_info *fs_info);
+int btrfs_may_assign_relation(struct btrfs_root *root,
+			      struct btrfs_ioctl_qgroup_assign_args *sa);
 int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
 			      struct btrfs_fs_info *fs_info, u64 src, u64 dst);
 int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index f66d622..701d812 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3758,13 +3758,15 @@ static long btrfs_ioctl_qgroup_assign(struct file *file,
void __user *arg)
 	}
 
 	mutex_lock(&root->fs_info->quota_lock);
+	ret = btrfs_may_assign_qgroup(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);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 6f57a98..0e09ac6 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -952,13 +952,9 @@ int btrfs_quota_rescan(struct btrfs_fs_info *fs_info)
 int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
 			      struct btrfs_fs_info *fs_info, u64 src, u64 dst)
 {
-	struct btrfs_root *quota_root;
+	struct btrfs_root *quota_root = fs_info->quota_root;
 	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;
@@ -979,14 +975,10 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle
*trans,
 int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans,
 			      struct btrfs_fs_info *fs_info, u64 src, u64 dst)
 {
-	struct btrfs_root *quota_root;
+	struct btrfs_root *quota_root = fs_info->quota_root;
 	int ret = 0;
 	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)
@@ -1681,3 +1673,31 @@ int btrfs_may_create_qgroup(struct btrfs_root *root,
 		return -EBUSY;
 	return 0;
 }
+
+int btrfs_may_assign_qgroup(struct btrfs_root *root,
+			    struct btrfs_ioctl_qgroup_assign_args *sa)
+{
+
+	struct btrfs_qgroup *parent = NULL;
+	struct btrfs_qgroup *member = NULL;
+	struct btrfs_qgroup_list *list;
+
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+
+	member = find_qgroup_rb(root->fs_info, sa->src);
+	parent = find_qgroup_rb(root->fs_info, sa->dst);
+	if (!member || !parent)
+		return -ENOENT;
+
+	list_for_each_entry(list, &member->groups, next_group) {
+		if (list->group == parent) {
+			if (sa->assign)
+				return -EEXIST;
+			return 0;
+		}
+	}
+	if (sa->assign)
+		return 0;
+	return -ENOENT;
+}
-- 
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-28 10:54 UTC

head link

[PATCH V2 6/6] Btrfs: fix missing check before btrfs_qgroup_inherit()

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

The original code forgot to check ''inherit'',we should gurantee
that all the qgroups in the struct ''inherit'' exist before
btrfs_qgroup_inherit().

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h  |    2 ++
 fs/btrfs/ioctl.c  |    6 ++++++
 fs/btrfs/qgroup.c |   29 ++++++++++++++++++++++++++---
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1ca1c63..0e73385 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3855,6 +3855,8 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle
*trans,
 			     struct btrfs_delayed_extent_op *extent_op);
 int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
 		      struct btrfs_fs_info *fs_info);
+int btrfs_may_inherit_qgroup(struct btrfs_root *root,
+			     struct btrfs_qgroup_inherit *inherit);
 int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
 			 struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid,
 			 struct btrfs_qgroup_inherit *inherit);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 701d812..96fda7e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -753,6 +753,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 = btrfs_may_inherit_qgroup(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 +767,7 @@ 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);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 0e09ac6..14fca77 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1359,9 +1359,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
 	 */
@@ -1701,3 +1698,29 @@ int btrfs_may_assign_qgroup(struct btrfs_root *root,
 		return 0;
 	return -ENOENT;
 }
+
+int btrfs_may_inherit_qgroup(struct btrfs_root *root,
+			     struct btrfs_qgroup_inherit *inherit)
+{
+	u64 i = 0;
+	u64 *i_qgroups = NULL;
+	u64 nums = 0;
+	struct btrfs_qgroup *qgroup = NULL;
+
+	if (!inherit)
+		return 0;
+	if (!root->fs_info->quota_root)
+		return -EINVAL;
+
+	i_qgroups = (u64 *)(inherit + 1);
+	nums = inherit->num_qgroups + 2 * inherit->num_ref_copies +
+	       2 * inherit->num_excl_copies;
+	for (i = 0; i < nums; ++i) {
+		qgroup = find_qgroup_rb(root->fs_info, *i_qgroups);
+		if (!qgroup)
+			return -EINVAL;
+
+		++i_qgroups;
+	}
+	return 0;
+}
-- 
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

Josef Bacik

2013-Mar-28 13:21 UTC

head link

Re: [PATCH V2 5/6] Btrfs: fix missing check before assigning/removing qgroup relation

On Thu, Mar 28, 2013 at 04:54:46AM -0600, Wang Shilong
wrote:> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> If the specified qgroup relation dosen''t exist, removing qgroup
operations
> will return -ENOENT;
> 
> If the specified qgroup relation exists, assigning qgroup relations will
> return -EEXIST.
> 
> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
Fails to compile with

fs/btrfs/ioctl.c: In function ‘btrfs_ioctl_qgroup_assign’:
fs/btrfs/ioctl.c:3771:2: error: implicit declaration of function
‘btrfs_may_assign_qgroup’ [-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors

Please fix and resubmit.  Thanks,

Josef
--
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-28 14:06 UTC

head link

Re: [PATCH V2 5/6] Btrfs: fix missing check before assigning/removing qgroup relation

Hello,> On Thu, Mar 28, 2013 at 04:54:46AM -0600, Wang Shilong wrote:
>> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
>> 
>> If the specified qgroup relation dosen''t exist, removing
qgroup operations
>> will return -ENOENT;
>> 
>> If the specified qgroup relation exists, assigning qgroup relations
will
>> return -EEXIST.
>> 
>> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
>> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
> 
> Fails to compile with
> 
> fs/btrfs/ioctl.c: In function ‘btrfs_ioctl_qgroup_assign’:
> fs/btrfs/ioctl.c:3771:2: error: implicit declaration of function
> ‘btrfs_may_assign_qgroup’ [-Werror=implicit-function-declaration]
> cc1: some warnings being treated as errors
> 
> Please fix and resubmit.  Thanks,
Sorry for such stupid errors, i promise that i have compile and test patch-set.

Things go like this, because when i compile kernel with patch-set , this errors
happen,
i just fix it but don''t update commit.

However, when i pass the test, i forget the compile errors…..

Next time, i will take care of that…

Thanks very much for your testing!


Thanks,
Wang
> 
> Josef
--
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

Arne Jansen

2013-Mar-30 23:41 UTC

head link

Re: [PATCH V2 2/6] Btrfs: remove some unnecessary spin_lock usages

On 03/28/13 11:54, Wang Shilong wrote:> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> We use mutex_lock to protect all the user change operaions.
> So when we are calling find_qgroup_rb() to check whether
> qgroup exists, we don''t have to hold spin_lock.
> 
> Besides, when enabling/disabling quota,it must be single
> thread when operations come to here.Spin_lock must be fistly
> used to clear quota_root when disabling quota,while enabling
> quota spin_lock must be used to complete the last assign work.
> 
> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
>  fs/btrfs/qgroup.c |   42 +++++++++++++++---------------------------
>  1 files changed, 15 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index e3598fa..7df372a 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -42,7 +42,6 @@
>   *  - limit
>   *  - caches fuer ulists
>   *  - performance benchmarks
> - *  - check all ioctl parameters
>   */
>  
>  /*
> @@ -98,7 +97,11 @@ struct btrfs_qgroup_list {
>  	struct btrfs_qgroup *member;
>  };
>  
> -/* must be called with qgroup_lock held */
instead it must be called with quota_lock held. The rest looks
correct to me.

-Arne
> +/*
> + * don''t need to be held by spin_lock since
> + * all the quota configurations on memory has been protected
> + * by mutex quota_lock.
> + */
>  static struct btrfs_qgroup *find_qgroup_rb(struct btrfs_fs_info *fs_info,
>  					   u64 qgroupid)
>  {
> @@ -793,13 +796,10 @@ int btrfs_quota_enable(struct btrfs_trans_handle
*trans,
>  	int ret = 0;
>  	int slot;
>  
> -	spin_lock(&fs_info->qgroup_lock);
>  	if (fs_info->quota_root) {
>  		fs_info->pending_quota_state = 1;
> -		spin_unlock(&fs_info->qgroup_lock);
> -		goto out;
> +		return ret;
>  	}
> -	spin_unlock(&fs_info->qgroup_lock);
>  
>  	/*
>  	 * initially create the quota tree
> @@ -808,7 +808,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle
*trans,
>  				       BTRFS_QUOTA_TREE_OBJECTID);
>  	if (IS_ERR(quota_root)) {
>  		ret =  PTR_ERR(quota_root);
> -		goto out;
> +		return ret;
>  	}
>  
>  	path = btrfs_alloc_path();
> @@ -861,14 +861,11 @@ int btrfs_quota_enable(struct btrfs_trans_handle
*trans,
>  			if (ret)
>  				goto out_free_path;
>  
> -			spin_lock(&fs_info->qgroup_lock);
>  			qgroup = add_qgroup_rb(fs_info, found_key.offset);
>  			if (IS_ERR(qgroup)) {
> -				spin_unlock(&fs_info->qgroup_lock);
>  				ret = PTR_ERR(qgroup);
>  				goto out_free_path;
>  			}
> -			spin_unlock(&fs_info->qgroup_lock);
>  		}
>  		ret = btrfs_next_item(tree_root, path);
>  		if (ret < 0)
> @@ -883,13 +880,12 @@ out_add_root:
>  	if (ret)
>  		goto out_free_path;
>  
> -	spin_lock(&fs_info->qgroup_lock);
>  	qgroup = add_qgroup_rb(fs_info, BTRFS_FS_TREE_OBJECTID);
>  	if (IS_ERR(qgroup)) {
> -		spin_unlock(&fs_info->qgroup_lock);
>  		ret = PTR_ERR(qgroup);
>  		goto out_free_path;
>  	}
> +	spin_lock(&fs_info->qgroup_lock);
>  	fs_info->quota_root = quota_root;
>  	fs_info->pending_quota_state = 1;
>  	spin_unlock(&fs_info->qgroup_lock);
> @@ -901,7 +897,6 @@ out_free_root:
>  		free_extent_buffer(quota_root->commit_root);
>  		kfree(quota_root);
>  	}
> -out:
>  	return ret;
>  }
>  
> @@ -912,11 +907,10 @@ int btrfs_quota_disable(struct btrfs_trans_handle
*trans,
>  	struct btrfs_root *quota_root;
>  	int ret = 0;
>  
> -	spin_lock(&fs_info->qgroup_lock);
> -	if (!fs_info->quota_root) {
> -		spin_unlock(&fs_info->qgroup_lock);
> +	if (!fs_info->quota_root)
>  		return 0;
> -	}
> +
> +	spin_lock(&fs_info->qgroup_lock);
>  	fs_info->quota_enabled = 0;
>  	fs_info->pending_quota_state = 0;
>  	quota_root = fs_info->quota_root;
> @@ -1041,15 +1035,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);
> +		if (!list_empty(&qgroup->groups) ||
> +		    !list_empty(&qgroup->members))
>  			return -EBUSY;
> -		}
>  	}
> -	spin_unlock(&fs_info->qgroup_lock);
>  
>  	ret = del_qgroup_item(trans, quota_root, qgroupid);
>  
> @@ -1081,20 +1072,17 @@ 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;
> 
--
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 V2 2/6] Btrfs: remove some unnecessary spin_lock usages

[PATCH V2 2/6] Btrfs: remove some unnecessary spin_lock usages

[PATCH V2 3/6] Btrfs: fix missing check before updating qgroup limit

[PATCH V2 4/6] Btrfs: fix missing check before creating/destroying a qgroup

[PATCH V2 5/6] Btrfs: fix missing check before assigning/removing qgroup relation

[PATCH V2 6/6] Btrfs: fix missing check before btrfs_qgroup_inherit()

Re: [PATCH V2 5/6] Btrfs: fix missing check before assigning/removing qgroup relation

Re: [PATCH V2 5/6] Btrfs: fix missing check before assigning/removing qgroup relation

Re: [PATCH V2 2/6] Btrfs: remove some unnecessary spin_lock usages