[PATCH 0/7] Qgroup status flag fix and semi-automatic qgroup update on relationship assign

[PATCH 0/7] Qgroup status flag fix and semi-automatic qgroup update on relationship assign

[Qu Wenruo]

This patchset will do the following bug fix mostly on incorrect flags.
1) INCONSISTENT flag never cleared bug
2) RESCANNING flag never cleared unless umount.

Also add 2 qgroup level related checks:
1) Kernel check on qgroup assign to ensure dst's level is higher than
src.
2) Restrict qgroup id under 1<<48, to ensure qgroup id won't have level
higher than 1.

At last, add semi-automatic qgroup info update on relationship assign.
The automatic part only happens when all extents of src are exclusive.
If not, it will set INCONSISTENT flag and return >0 to info user land
progs to do rescan.

Dongsheng Yang (1):
  btrfs: qgroup: clear STATUS_FLAG_ON in disabling quota.

Qu Wenruo (6):
  btrfs: Check qgroup level in kernel qgroup assign.
  btrfs: Don't allow subvolid >= (1 << BTRFS_QGROUP_LEVEL_SHIFT) to be  
      created
  btrfs: qgroup: Fix dead judgement on qgroup_rescan_leaf() return
    value.
  btrfs: Update btrfs qgroup status item when rescan is done.
  btrfs: quota: Automatically update related qgroups or mark
    INCONSISTENT     flags when assigning/deleting a qgroup relations.
  btrfs: quota: Update quota tree after qgroup relationship change.

 fs/btrfs/ctree.h  |   9 ++-
 fs/btrfs/ioctl.c  |  12 ++++
 fs/btrfs/qgroup.c | 203 ++++++++++++++++++++++++++++++++++++++++--------------
 3 files changed, 170 insertions(+), 54 deletions(-)

-- 
2.3.0

[PATCH 1/7] btrfs: Check qgroup level in kernel qgroup assign.

[Qu Wenruo]

Although we have qgroup level check in btrfs-progs, it's not enough
since other programe may still call ioctl directly not using
btrfs-progs. For example, systemd.

But it's btrfs-progs to be blame since we don't provide a
full-function(like subvolume create things) btrfs library with enough
check, and only rely on kernel ioctl.

So Add level checks in kernel too.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/ctree.h  | 6 ++++++
 fs/btrfs/qgroup.c | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0b18070..cbaf1a4 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1056,6 +1056,12 @@ struct btrfs_block_group_item {
 	__le64 flags;
 } __attribute__ ((__packed__));
 
+#define BTRFS_QGROUP_LEVEL_SHIFT		48
+static inline u64 btrfs_qgroup_level(u64 qgroupid)
+{
+	return qgroupid >> BTRFS_QGROUP_LEVEL_SHIFT;
+}
+
 /*
  * is subvolume quota turned on?
  */
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 48b60db..523e5b2 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1010,6 +1010,10 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
 	struct btrfs_qgroup_list *list;
 	int ret = 0;
 
+	/* Check the level of src and dst first */
+	if (btrfs_qgroup_level(src) >= btrfs_qgroup_level(dst))
+		return -EINVAL;
+
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
 	quota_root = fs_info->quota_root;
 	if (!quota_root) {
-- 
2.3.0

Re: [PATCH 1/7] btrfs: Check qgroup level in kernel qgroup assign.

[Josef Bacik]

On 02/27/2015 03:24 AM, Qu Wenruo wrote:
> Although we have qgroup level check in btrfs-progs, it's not enough
> since other programe may still call ioctl directly not using
> btrfs-progs. For example, systemd.
>
> But it's btrfs-progs to be blame since we don't provide a
> full-function(like subvolume create things) btrfs library with enough
> check, and only rely on kernel ioctl.
>
> So Add level checks in kernel too.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef

Re: [PATCH 1/7] btrfs: Check qgroup level in kernel qgroup assign.

[David Sterba]

On Fri, Feb 27, 2015 at 04:24:22PM +0800, Qu Wenruo wrote:
> Although we have qgroup level check in btrfs-progs, it's not enough
> since other programe may still call ioctl directly not using
> btrfs-progs. For example, systemd.
> 
> But it's btrfs-progs to be blame since we don't provide a
> full-function(like subvolume create things) btrfs library with enough
> check, and only rely on kernel ioctl.
> 
> So Add level checks in kernel too.

The checks should be duplicated in kernel if they are part of the qgroup
naming/numbering protocol. The library interface would help programmers
to get it right but as there's a direct interface via the ioctls, the
checks have to be there regardless.

[PATCH 2/7] btrfs: Don't allow subvolid >= (1 << BTRFS_QGROUP_LEVEL_SHIFT) to be created

[Qu Wenruo]

Btrfs will create qgroup on subvolume creation if quota is enabled, but
qgroup uses the high bits(currently 16 bits) as level, to build the
inheritance.

However it is fully possible a subvolume can be created with a
subvolumeid larger than 1 << BTRFS_QGROUP_LEVEL_SHIFT, so it will be
considered as level 1 and can't be assigned to other qgroup in level 1.

This patch will prevent such things so qgroup inheritance will not be
screwed up.
The downside is very clear, btrfs subvolume number limit will decrease
from (u64 max - 256(fisrt free objectid) - 256(last free objectid)) to
(u48 max -256(first free objectid)).
But we still have near u48(that's 15 digits in dec), so that should not
be a huge problem.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/ctree.h | 3 ++-
 fs/btrfs/ioctl.c | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index cbaf1a4..fb4b52b 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -4200,7 +4200,8 @@ int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
 static inline int is_fstree(u64 rootid)
 {
 	if (rootid == BTRFS_FS_TREE_OBJECTID ||
-	    (s64)rootid >= (s64)BTRFS_FIRST_FREE_OBJECTID)
+	    ((s64)rootid >= (s64)BTRFS_FIRST_FREE_OBJECTID &&
+	      !btrfs_qgroup_level(rootid)))
 		return 1;
 	return 0;
 }
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index d49fe8a..f93ca4b 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -456,6 +456,13 @@ static noinline int create_subvol(struct inode *dir,
 	if (ret)
 		return ret;
 
+	/*
+	 * Don't create subvolume whose level is not zero. Or qgroup will be
+	 * screwed up since it assume subvolme qgroup's level to be 0.
+	 */
+	if (btrfs_qgroup_level(objectid))
+		return -ENOSPC;
+
 	btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
 	/*
 	 * The same as the snapshot creation, please see the comment
-- 
2.3.0

Re: [PATCH 2/7] btrfs: Don't allow subvolid >= (1 << BTRFS_QGROUP_LEVEL_SHIFT) to be created

[Josef Bacik]

On 02/27/2015 03:24 AM, Qu Wenruo wrote:
> Btrfs will create qgroup on subvolume creation if quota is enabled, but
> qgroup uses the high bits(currently 16 bits) as level, to build the
> inheritance.
>
> However it is fully possible a subvolume can be created with a
> subvolumeid larger than 1 << BTRFS_QGROUP_LEVEL_SHIFT, so it will be
> considered as level 1 and can't be assigned to other qgroup in level 1.
>
> This patch will prevent such things so qgroup inheritance will not be
> screwed up.
> The downside is very clear, btrfs subvolume number limit will decrease
> from (u64 max - 256(fisrt free objectid) - 256(last free objectid)) to
> (u48 max -256(first free objectid)).
> But we still have near u48(that's 15 digits in dec), so that should not
> be a huge problem.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef

[PATCH 3/7] btrfs: qgroup: Fix dead judgement on qgroup_rescan_leaf() return value.

[Qu Wenruo]

Old qgroup_rescan_leaf() comment indicates ret == 2 as complete and
cleared INCONSISTENT flag.

This is not true since it will never return 2, and inside it no codes
will clear INCONSISTENT flag.
The flag clearance is done in btrfs_qgroup_rescan_work().
This caused the bug that INCONSISTENT flag is never cleared.

So change the comment and fix the dead judgment.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/qgroup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 523e5b2..a48a590 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2518,7 +2518,7 @@ void assert_qgroups_uptodate(struct btrfs_trans_handle *trans)
 
 /*
  * returns < 0 on error, 0 when more leafs are to be scanned.
- * returns 1 when done, 2 when done and FLAG_INCONSISTENT was cleared.
+ * returns 1 when done.
  */
 static int
 qgroup_rescan_leaf(struct btrfs_fs_info *fs_info, struct btrfs_path *path,
@@ -2665,7 +2665,7 @@ out:
 	mutex_lock(&fs_info->qgroup_rescan_lock);
 	fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
 
-	if (err == 2 &&
+	if (err > 0 &&
 	    fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT) {
 		fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
 	} else if (err < 0) {
@@ -2675,7 +2675,7 @@ out:
 
 	if (err >= 0) {
 		btrfs_info(fs_info, "qgroup scan completed%s",
-			err == 2 ? " (inconsistency flag cleared)" : "");
+			err > 0 ? " (inconsistency flag cleared)" : "");
 	} else {
 		btrfs_err(fs_info, "qgroup scan failed with %d", err);
 	}
-- 
2.3.0

Re: [PATCH 3/7] btrfs: qgroup: Fix dead judgement on qgroup_rescan_leaf() return value.

[Josef Bacik]

On 02/27/2015 03:24 AM, Qu Wenruo wrote:
> Old qgroup_rescan_leaf() comment indicates ret == 2 as complete and
> cleared INCONSISTENT flag.
>
> This is not true since it will never return 2, and inside it no codes
> will clear INCONSISTENT flag.
> The flag clearance is done in btrfs_qgroup_rescan_work().
> This caused the bug that INCONSISTENT flag is never cleared.
>
> So change the comment and fix the dead judgment.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef

[PATCH 4/7] btrfs: Update btrfs qgroup status item when rescan is done.

[Qu Wenruo]

Update qgroup status when rescan is done.

Before this patch, status item is not updated on rescan finish, which
causing the RESCAN and INCONSISTENT flags never cleared.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/qgroup.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index a48a590..20fe219 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2623,6 +2623,7 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 	struct ulist *tmp = NULL, *qgroups = NULL;
 	struct extent_buffer *scratch_leaf = NULL;
 	int err = -ENOMEM;
+	int ret = 0;
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -2673,6 +2674,28 @@ out:
 	}
 	mutex_unlock(&fs_info->qgroup_rescan_lock);
 
+	/*
+	 * only update status, since the previous part has alreay updated the
+	 * qgroup info.
+	 */
+	trans = btrfs_start_transaction(fs_info->quota_root, 1);
+	if (IS_ERR(trans)) {
+		err = PTR_ERR(trans);
+		btrfs_err(fs_info,
+			  "fail to start transaction for status update: %d\n",
+			  err);
+		goto done;
+	}
+	ret = update_qgroup_status_item(trans, fs_info, fs_info->quota_root);
+	if (ret < 0) {
+		err = ret;
+		btrfs_err(fs_info, "fail to update qgroup status: %d\n",
+			  err);
+		btrfs_abort_transaction(trans, fs_info->quota_root, err);
+		goto done;
+	}
+	btrfs_end_transaction(trans, fs_info->quota_root);
+
 	if (err >= 0) {
 		btrfs_info(fs_info, "qgroup scan completed%s",
 			err > 0 ? " (inconsistency flag cleared)" : "");
@@ -2680,6 +2703,7 @@ out:
 		btrfs_err(fs_info, "qgroup scan failed with %d", err);
 	}
 
+done:
 	complete_all(&fs_info->qgroup_rescan_completion);
 }
 
@@ -2714,7 +2738,6 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
 			mutex_unlock(&fs_info->qgroup_rescan_lock);
 			goto err;
 		}
-
 		fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_RESCAN;
 	}
 
-- 
2.3.0

Re: [PATCH 4/7] btrfs: Update btrfs qgroup status item when rescan is done.

[Josef Bacik]

On 02/27/2015 03:24 AM, Qu Wenruo wrote:
> Update qgroup status when rescan is done.
>
> Before this patch, status item is not updated on rescan finish, which
> causing the RESCAN and INCONSISTENT flags never cleared.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef

[PATCH 5/7] btrfs: qgroup: clear STATUS_FLAG_ON in disabling quota.

[Qu Wenruo]

From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>

we forgot to clear STATUS_FLAG_ON in quota_disable(), it
will cause a problem shown as below:

	# mount /dev/sdc /mnt
	# btrfs quota enable /mnt
	# btrfs quota disable /mnt
	# btrfs quota rescan /mnt
	quota rescan started <--- expecting it fail here.
	# echo $?
	0

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/btrfs/qgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 20fe219..b48f2c8 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -967,6 +967,7 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
 	fs_info->pending_quota_state = 0;
 	quota_root = fs_info->quota_root;
 	fs_info->quota_root = NULL;
+	fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_ON;
 	spin_unlock(&fs_info->qgroup_lock);
 
 	btrfs_free_qgroup_config(fs_info);
-- 
2.3.0

Re: [PATCH 5/7] btrfs: qgroup: clear STATUS_FLAG_ON in disabling quota.

[Josef Bacik]

On 02/27/2015 03:24 AM, Qu Wenruo wrote:
> From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>
> we forgot to clear STATUS_FLAG_ON in quota_disable(), it
> will cause a problem shown as below:
>
> 	# mount /dev/sdc /mnt
> 	# btrfs quota enable /mnt
> 	# btrfs quota disable /mnt
> 	# btrfs quota rescan /mnt
> 	quota rescan started <--- expecting it fail here.
> 	# echo $?
> 	0
>
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef

[PATCH 6/7] btrfs: quota: Automatically update related qgroups or mark INCONSISTENT flags when assigning/deleting a qgroup relations.

[Qu Wenruo]

Operation like qgroups assigning/deleting qgroup relations will mostly
cause qgroup data inconsistent, since it needs to do the full rescan to
determine whether shared extents are exclusive or still shared in
parent qgroups.

But there are some exceptions, like qgroup with only exclusive extents
(qgroup->excl == qgroup->rfer), in that case, we only needs to
modify all its parents' excl and rfer.

So this patch adds a quick path for such qgroup in qgroup
assign/remove routine, and if quick path failed, the qgroup status will
be marked INCONSISTENT, and return 1 to info user-land.

BTW since the quick path is much the same of qgroup_excl_accounting(),
so move the core of it to __qgroup_excl_accounting() and reuse it.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/qgroup.c | 175 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 122 insertions(+), 53 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index b48f2c8..2d6bb92 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1002,6 +1002,106 @@ static void qgroup_dirty(struct btrfs_fs_info *fs_info,
 		list_add(&qgroup->dirty, &fs_info->dirty_qgroups);
 }
 
+/*
+ * The easy accounting, if we are adding/removing the only ref for an extent
+ * then this qgroup and all of the parent qgroups get their refrence and
+ * exclusive counts adjusted.
+ *
+ * Caller should hold fs_info->qgroup_lock.
+ */
+static int __qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
+				    struct ulist *tmp, u64 ref_root,
+				    u64 num_bytes, int sign)
+{
+	struct btrfs_qgroup *qgroup;
+	struct btrfs_qgroup_list *glist;
+	struct ulist_node *unode;
+	struct ulist_iterator uiter;
+	int ret = 0;
+
+	qgroup = find_qgroup_rb(fs_info, ref_root);
+	if (!qgroup)
+		goto out;
+
+	qgroup->rfer += sign * num_bytes;
+	qgroup->rfer_cmpr += sign * num_bytes;
+
+	WARN_ON(sign < 0 && qgroup->excl < num_bytes);
+	qgroup->excl += sign * num_bytes;
+	qgroup->excl_cmpr += sign * num_bytes;
+
+	qgroup_dirty(fs_info, qgroup);
+
+	/* Get all of the parent groups that contain this qgroup */
+	list_for_each_entry(glist, &qgroup->groups, next_group) {
+		ret = ulist_add(tmp, glist->group->qgroupid,
+				ptr_to_u64(glist->group), GFP_ATOMIC);
+		if (ret < 0)
+			goto out;
+	}
+
+	/* Iterate all of the parents and adjust their reference counts */
+	ULIST_ITER_INIT(&uiter);
+	while ((unode = ulist_next(tmp, &uiter))) {
+		qgroup = u64_to_ptr(unode->aux);
+		qgroup->rfer += sign * num_bytes;
+		qgroup->rfer_cmpr += sign * num_bytes;
+		qgroup->excl += sign * num_bytes;
+		if (sign < 0)
+			WARN_ON(qgroup->excl < num_bytes);
+		qgroup->excl_cmpr += sign * num_bytes;
+		qgroup_dirty(fs_info, qgroup);
+
+		/* Add any parents of the parents */
+		list_for_each_entry(glist, &qgroup->groups, next_group) {
+			ret = ulist_add(tmp, glist->group->qgroupid,
+					ptr_to_u64(glist->group), GFP_ATOMIC);
+			if (ret < 0)
+				goto out;
+		}
+	}
+out:
+	return ret;
+}
+
+
+/*
+ * Quick path for updating qgroup with only excl refs.
+ *
+ * In that case, just update all parent will be enough.
+ * Or we needs to do a full rescan.
+ * Caller should also hold fs_info->qgroup_lock.
+ *
+ * Return 0 for quick update, return >0 for need to full rescan
+ * and mark INCONSISTENT flag.
+ * Return < 0 for other error.
+ */
+static int quick_update_accounting(struct btrfs_fs_info *fs_info,
+				   struct ulist *tmp, u64 src, u64 dst,
+				   int sign)
+{
+	struct btrfs_qgroup *qgroup;
+	int ret = 1;
+	int err = 0;
+
+	qgroup = find_qgroup_rb(fs_info, src);
+	if (!qgroup)
+		goto out;
+	if (qgroup->excl == qgroup->rfer) {
+		ret = 0;
+		err = __qgroup_excl_accounting(fs_info, tmp, dst,
+					       qgroup->excl, sign);
+		if (err < 0) {
+			ret = err;
+			goto out;
+		}
+	}
+out:
+	if (ret)
+		fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+	return ret;
+}
+
 int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
 			      struct btrfs_fs_info *fs_info, u64 src, u64 dst)
 {
@@ -1009,8 +1109,13 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
 	struct btrfs_qgroup *parent;
 	struct btrfs_qgroup *member;
 	struct btrfs_qgroup_list *list;
+	struct ulist *tmp;
 	int ret = 0;
 
+	tmp = ulist_alloc(GFP_NOFS);
+	if (!tmp)
+		return -ENOMEM;
+
 	/* Check the level of src and dst first */
 	if (btrfs_qgroup_level(src) >= btrfs_qgroup_level(dst))
 		return -EINVAL;
@@ -1048,9 +1153,15 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
 
 	spin_lock(&fs_info->qgroup_lock);
 	ret = add_relation_rb(quota_root->fs_info, src, dst);
+	if (ret < 0) {
+		spin_unlock(&fs_info->qgroup_lock);
+		goto out;
+	}
+	ret = quick_update_accounting(fs_info, tmp, src, dst, 1);
 	spin_unlock(&fs_info->qgroup_lock);
 out:
 	mutex_unlock(&fs_info->qgroup_ioctl_lock);
+	ulist_free(tmp);
 	return ret;
 }
 
@@ -1061,9 +1172,14 @@ int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans,
 	struct btrfs_qgroup *parent;
 	struct btrfs_qgroup *member;
 	struct btrfs_qgroup_list *list;
+	struct ulist *tmp;
 	int ret = 0;
 	int err;
 
+	tmp = ulist_alloc(GFP_NOFS);
+	if (!tmp)
+		return -ENOMEM;
+
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
 	quota_root = fs_info->quota_root;
 	if (!quota_root) {
@@ -1093,9 +1209,11 @@ exist:
 
 	spin_lock(&fs_info->qgroup_lock);
 	del_relation_rb(fs_info, src, dst);
+	ret = quick_update_accounting(fs_info, tmp, src, dst, -1);
 	spin_unlock(&fs_info->qgroup_lock);
 out:
 	mutex_unlock(&fs_info->qgroup_ioctl_lock);
+	ulist_free(tmp);
 	return ret;
 }
 
@@ -1377,19 +1495,10 @@ int btrfs_qgroup_record_ref(struct btrfs_trans_handle *trans,
 	return 0;
 }
 
-/*
- * The easy accounting, if we are adding/removing the only ref for an extent
- * then this qgroup and all of the parent qgroups get their refrence and
- * exclusive counts adjusted.
- */
 static int qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
 				  struct btrfs_qgroup_operation *oper)
 {
-	struct btrfs_qgroup *qgroup;
 	struct ulist *tmp;
-	struct btrfs_qgroup_list *glist;
-	struct ulist_node *unode;
-	struct ulist_iterator uiter;
 	int sign = 0;
 	int ret = 0;
 
@@ -1400,9 +1509,7 @@ static int qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
 	spin_lock(&fs_info->qgroup_lock);
 	if (!fs_info->quota_root)
 		goto out;
-	qgroup = find_qgroup_rb(fs_info, oper->ref_root);
-	if (!qgroup)
-		goto out;
+
 	switch (oper->type) {
 	case BTRFS_QGROUP_OPER_ADD_EXCL:
 		sign = 1;
@@ -1413,44 +1520,9 @@ static int qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
 	default:
 		ASSERT(0);
 	}
-	qgroup->rfer += sign * oper->num_bytes;
-	qgroup->rfer_cmpr += sign * oper->num_bytes;
-
-	WARN_ON(sign < 0 && qgroup->excl < oper->num_bytes);
-	qgroup->excl += sign * oper->num_bytes;
-	qgroup->excl_cmpr += sign * oper->num_bytes;
-
-	qgroup_dirty(fs_info, qgroup);
-
-	/* Get all of the parent groups that contain this qgroup */
-	list_for_each_entry(glist, &qgroup->groups, next_group) {
-		ret = ulist_add(tmp, glist->group->qgroupid,
-				ptr_to_u64(glist->group), GFP_ATOMIC);
-		if (ret < 0)
-			goto out;
-	}
-
-	/* Iterate all of the parents and adjust their reference counts */
-	ULIST_ITER_INIT(&uiter);
-	while ((unode = ulist_next(tmp, &uiter))) {
-		qgroup = u64_to_ptr(unode->aux);
-		qgroup->rfer += sign * oper->num_bytes;
-		qgroup->rfer_cmpr += sign * oper->num_bytes;
-		qgroup->excl += sign * oper->num_bytes;
-		if (sign < 0)
-			WARN_ON(qgroup->excl < oper->num_bytes);
-		qgroup->excl_cmpr += sign * oper->num_bytes;
-		qgroup_dirty(fs_info, qgroup);
 
-		/* Add any parents of the parents */
-		list_for_each_entry(glist, &qgroup->groups, next_group) {
-			ret = ulist_add(tmp, glist->group->qgroupid,
-					ptr_to_u64(glist->group), GFP_ATOMIC);
-			if (ret < 0)
-				goto out;
-		}
-	}
-	ret = 0;
+	ret = __qgroup_excl_accounting(fs_info, tmp, oper->ref_root,
+				       oper->num_bytes, sign);
 out:
 	spin_unlock(&fs_info->qgroup_lock);
 	ulist_free(tmp);
@@ -2690,10 +2762,7 @@ out:
 	ret = update_qgroup_status_item(trans, fs_info, fs_info->quota_root);
 	if (ret < 0) {
 		err = ret;
-		btrfs_err(fs_info, "fail to update qgroup status: %d\n",
-			  err);
-		btrfs_abort_transaction(trans, fs_info->quota_root, err);
-		goto done;
+		btrfs_err(fs_info, "fail to update qgroup status: %d\n", err);
 	}
 	btrfs_end_transaction(trans, fs_info->quota_root);
 
-- 
2.3.0

Re: [PATCH 6/7] btrfs: quota: Automatically update related qgroups or mark INCONSISTENT flags when assigning/deleting a qgroup relations.

[Josef Bacik]

On 02/27/2015 03:24 AM, Qu Wenruo wrote:
> Operation like qgroups assigning/deleting qgroup relations will mostly
> cause qgroup data inconsistent, since it needs to do the full rescan to
> determine whether shared extents are exclusive or still shared in
> parent qgroups.
>
> But there are some exceptions, like qgroup with only exclusive extents
> (qgroup->excl == qgroup->rfer), in that case, we only needs to
> modify all its parents' excl and rfer.
>
> So this patch adds a quick path for such qgroup in qgroup
> assign/remove routine, and if quick path failed, the qgroup status will
> be marked INCONSISTENT, and return 1 to info user-land.
>
> BTW since the quick path is much the same of qgroup_excl_accounting(),
> so move the core of it to __qgroup_excl_accounting() and reuse it.
>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef

[PATCH 7/7] btrfs: quota: Update quota tree after qgroup relationship change.

[Qu Wenruo]

Previous patch modified the in memory struct but it's not written in
quota tree until next commit.
So user will still get old data using "btrfs qgroup show" after
assign/remove.

This patch will call btrfs_run_qgroups in assign ioctl so it will be
updated to in memory quota trees and user will get up-to-date results.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/ioctl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index f93ca4b..62145ff 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4631,6 +4631,11 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg)
 						sa->src, sa->dst);
 	}
 
+	/* update qgroup status and info */
+	err = btrfs_run_qgroups(trans, root->fs_info);
+	if (err < 0)
+		btrfs_error(root->fs_info, ret,
+			    "failed to update qgroup status and info\n");
 	err = btrfs_end_transaction(trans, root);
 	if (err && !ret)
 		ret = err;
-- 
2.3.0

Re: [PATCH 7/7] btrfs: quota: Update quota tree after qgroup relationship change.

[Josef Bacik]

On 02/27/2015 03:24 AM, Qu Wenruo wrote:
> Previous patch modified the in memory struct but it's not written in
> quota tree until next commit.
> So user will still get old data using "btrfs qgroup show" after
> assign/remove.
>
> This patch will call btrfs_run_qgroups in assign ioctl so it will be
> updated to in memory quota trees and user will get up-to-date results.
>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef