* [PATCH] btrfs: qgroup: Cleanup open-coded old/new_refcnt update and read.
@ 2015-03-12 8:33 Qu Wenruo
2015-03-12 13:45 ` Liu Bo
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2015-03-12 8:33 UTC (permalink / raw)
To: linux-btrfs
Use inline functions to do such things, to improve readability.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
fs/btrfs/qgroup.c | 102 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 65 insertions(+), 37 deletions(-)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 97159a8..3f2a1e7 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -84,11 +84,58 @@ struct btrfs_qgroup {
/*
* temp variables for accounting operations
+ * Refer to qgroup_shared_accouting() for details.
*/
u64 old_refcnt;
u64 new_refcnt;
};
+static void update_qg_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod,
+ int update_old)
+{
+ if (update_old) {
+ if (qg->old_refcnt < seq)
+ qg->old_refcnt = seq;
+ qg->old_refcnt += mod;
+ } else {
+ if (qg->new_refcnt < seq)
+ qg->new_refcnt = seq;
+ qg->new_refcnt += mod;
+ }
+}
+
+static void update_qg_old_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod)
+{
+ update_qg_refcnt(qg, seq, mod, 1);
+}
+
+static void update_qg_new_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod)
+{
+ update_qg_refcnt(qg, seq, mod, 0);
+}
+
+static inline u64 get_qg_refcnt(struct btrfs_qgroup *qg, u64 seq, int get_old)
+{
+ if (get_old) {
+ if (qg->old_refcnt < seq)
+ return 0;
+ return qg->old_refcnt - seq;
+ }
+ if (qg->new_refcnt < seq)
+ return 0;
+ return qg->new_refcnt - seq;
+}
+
+static inline u64 get_qg_old_refcnt(struct btrfs_qgroup *qg, u64 seq)
+{
+ return get_qg_refcnt(qg, seq, 1);
+}
+
+static inline u64 get_qg_new_refcnt(struct btrfs_qgroup *qg, u64 seq)
+{
+ return get_qg_refcnt(qg, seq, 0);
+}
+
/*
* glue structure to represent the relations between qgroups.
*/
@@ -1497,6 +1544,7 @@ static int qgroup_calc_old_refcnt(struct btrfs_fs_info *fs_info,
ULIST_ITER_INIT(&tmp_uiter);
while ((tmp_unode = ulist_next(tmp, &tmp_uiter))) {
struct btrfs_qgroup_list *glist;
+ int mod;
qg = u64_to_ptr(tmp_unode->aux);
/*
@@ -1508,20 +1556,15 @@ static int qgroup_calc_old_refcnt(struct btrfs_fs_info *fs_info,
* upper level qgroups in order to determine exclusive
* counts.
*
- * For rescan we want to set old_refcnt to seq so our
- * exclusive calculations end up correct.
+ * For rescan none of the extent is recorded before so
+ * we just don't add old_refcnt.
*/
if (rescan)
- qg->old_refcnt = seq;
- else if (qg->old_refcnt < seq)
- qg->old_refcnt = seq + 1;
+ mod = 0;
else
- qg->old_refcnt++;
-
- if (qg->new_refcnt < seq)
- qg->new_refcnt = seq + 1;
- else
- qg->new_refcnt++;
+ mod = 1;
+ update_qg_old_refcnt(qg, seq, mod);
+ update_qg_new_refcnt(qg, seq, 1);
list_for_each_entry(glist, &qg->groups, next_group) {
ret = ulist_add(qgroups, glist->group->qgroupid,
ptr_to_u64(glist->group),
@@ -1619,10 +1662,8 @@ next:
qg->old_refcnt = seq + 1;
else
qg->old_refcnt++;
- if (qg->new_refcnt < seq)
- qg->new_refcnt = seq + 1;
- else
- qg->new_refcnt++;
+ update_qg_old_refcnt(qg, seq, 1);
+ update_qg_new_refcnt(qg, seq, 1);
list_for_each_entry(glist, &qg->groups, next_group) {
ret = ulist_add(qgroups, glist->group->qgroupid,
ptr_to_u64(glist->group), GFP_ATOMIC);
@@ -1663,17 +1704,10 @@ static int qgroup_calc_new_refcnt(struct btrfs_fs_info *fs_info,
struct btrfs_qgroup_list *glist;
qg = u64_to_ptr(unode->aux);
- if (oper->type == BTRFS_QGROUP_OPER_ADD_SHARED) {
- if (qg->new_refcnt < seq)
- qg->new_refcnt = seq + 1;
- else
- qg->new_refcnt++;
- } else {
- if (qg->old_refcnt < seq)
- qg->old_refcnt = seq + 1;
- else
- qg->old_refcnt++;
- }
+ if (oper->type == BTRFS_QGROUP_OPER_ADD_SHARED)
+ update_qg_new_refcnt(qg, seq, 1);
+ else
+ update_qg_old_refcnt(qg, seq, 1);
list_for_each_entry(glist, &qg->groups, next_group) {
ret = ulist_add(tmp, glist->group->qgroupid,
ptr_to_u64(glist->group), GFP_ATOMIC);
@@ -1706,11 +1740,14 @@ static int qgroup_adjust_counters(struct btrfs_fs_info *fs_info,
bool dirty = false;
qg = u64_to_ptr(unode->aux);
+ cur_old_count = get_qg_old_refcnt(qg, seq);
+ cur_new_count = get_qg_new_refcnt(qg, seq);
+
/*
* Wasn't referenced before but is now, add to the reference
* counters.
*/
- if (qg->old_refcnt <= seq && qg->new_refcnt > seq) {
+ if (cur_old_count == 0 && cur_new_count > 0) {
qg->rfer += num_bytes;
qg->rfer_cmpr += num_bytes;
dirty = true;
@@ -1720,21 +1757,12 @@ static int qgroup_adjust_counters(struct btrfs_fs_info *fs_info,
* Was referenced before but isn't now, subtract from the
* reference counters.
*/
- if (qg->old_refcnt > seq && qg->new_refcnt <= seq) {
+ if (cur_old_count > 0 && cur_new_count == 0) {
qg->rfer -= num_bytes;
qg->rfer_cmpr -= num_bytes;
dirty = true;
}
- if (qg->old_refcnt < seq)
- cur_old_count = 0;
- else
- cur_old_count = qg->old_refcnt - seq;
- if (qg->new_refcnt < seq)
- cur_new_count = 0;
- else
- cur_new_count = qg->new_refcnt - seq;
-
/*
* If our refcount was the same as the roots previously but our
* new count isn't the same as the number of roots now then we
--
2.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Cleanup open-coded old/new_refcnt update and read.
2015-03-12 8:33 [PATCH] btrfs: qgroup: Cleanup open-coded old/new_refcnt update and read Qu Wenruo
@ 2015-03-12 13:45 ` Liu Bo
2015-03-13 0:37 ` Qu Wenruo
0 siblings, 1 reply; 4+ messages in thread
From: Liu Bo @ 2015-03-12 13:45 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, Mar 12, 2015 at 04:33:27PM +0800, Qu Wenruo wrote:
> Use inline functions to do such things, to improve readability.
This has actually hiden informations behind the helper function and
old_refcnt and new_refcnt can explain themselves, I don't see any benefit.
Thanks,
-liubo
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
> fs/btrfs/qgroup.c | 102 ++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 65 insertions(+), 37 deletions(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 97159a8..3f2a1e7 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -84,11 +84,58 @@ struct btrfs_qgroup {
>
> /*
> * temp variables for accounting operations
> + * Refer to qgroup_shared_accouting() for details.
> */
> u64 old_refcnt;
> u64 new_refcnt;
> };
>
> +static void update_qg_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod,
> + int update_old)
> +{
> + if (update_old) {
> + if (qg->old_refcnt < seq)
> + qg->old_refcnt = seq;
> + qg->old_refcnt += mod;
> + } else {
> + if (qg->new_refcnt < seq)
> + qg->new_refcnt = seq;
> + qg->new_refcnt += mod;
> + }
> +}
> +
> +static void update_qg_old_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod)
> +{
> + update_qg_refcnt(qg, seq, mod, 1);
> +}
> +
> +static void update_qg_new_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod)
> +{
> + update_qg_refcnt(qg, seq, mod, 0);
> +}
> +
> +static inline u64 get_qg_refcnt(struct btrfs_qgroup *qg, u64 seq, int get_old)
> +{
> + if (get_old) {
> + if (qg->old_refcnt < seq)
> + return 0;
> + return qg->old_refcnt - seq;
> + }
> + if (qg->new_refcnt < seq)
> + return 0;
> + return qg->new_refcnt - seq;
> +}
> +
> +static inline u64 get_qg_old_refcnt(struct btrfs_qgroup *qg, u64 seq)
> +{
> + return get_qg_refcnt(qg, seq, 1);
> +}
> +
> +static inline u64 get_qg_new_refcnt(struct btrfs_qgroup *qg, u64 seq)
> +{
> + return get_qg_refcnt(qg, seq, 0);
> +}
> +
> /*
> * glue structure to represent the relations between qgroups.
> */
> @@ -1497,6 +1544,7 @@ static int qgroup_calc_old_refcnt(struct btrfs_fs_info *fs_info,
> ULIST_ITER_INIT(&tmp_uiter);
> while ((tmp_unode = ulist_next(tmp, &tmp_uiter))) {
> struct btrfs_qgroup_list *glist;
> + int mod;
>
> qg = u64_to_ptr(tmp_unode->aux);
> /*
> @@ -1508,20 +1556,15 @@ static int qgroup_calc_old_refcnt(struct btrfs_fs_info *fs_info,
> * upper level qgroups in order to determine exclusive
> * counts.
> *
> - * For rescan we want to set old_refcnt to seq so our
> - * exclusive calculations end up correct.
> + * For rescan none of the extent is recorded before so
> + * we just don't add old_refcnt.
> */
> if (rescan)
> - qg->old_refcnt = seq;
> - else if (qg->old_refcnt < seq)
> - qg->old_refcnt = seq + 1;
> + mod = 0;
> else
> - qg->old_refcnt++;
> -
> - if (qg->new_refcnt < seq)
> - qg->new_refcnt = seq + 1;
> - else
> - qg->new_refcnt++;
> + mod = 1;
> + update_qg_old_refcnt(qg, seq, mod);
> + update_qg_new_refcnt(qg, seq, 1);
> list_for_each_entry(glist, &qg->groups, next_group) {
> ret = ulist_add(qgroups, glist->group->qgroupid,
> ptr_to_u64(glist->group),
> @@ -1619,10 +1662,8 @@ next:
> qg->old_refcnt = seq + 1;
> else
> qg->old_refcnt++;
> - if (qg->new_refcnt < seq)
> - qg->new_refcnt = seq + 1;
> - else
> - qg->new_refcnt++;
> + update_qg_old_refcnt(qg, seq, 1);
> + update_qg_new_refcnt(qg, seq, 1);
> list_for_each_entry(glist, &qg->groups, next_group) {
> ret = ulist_add(qgroups, glist->group->qgroupid,
> ptr_to_u64(glist->group), GFP_ATOMIC);
> @@ -1663,17 +1704,10 @@ static int qgroup_calc_new_refcnt(struct btrfs_fs_info *fs_info,
> struct btrfs_qgroup_list *glist;
>
> qg = u64_to_ptr(unode->aux);
> - if (oper->type == BTRFS_QGROUP_OPER_ADD_SHARED) {
> - if (qg->new_refcnt < seq)
> - qg->new_refcnt = seq + 1;
> - else
> - qg->new_refcnt++;
> - } else {
> - if (qg->old_refcnt < seq)
> - qg->old_refcnt = seq + 1;
> - else
> - qg->old_refcnt++;
> - }
> + if (oper->type == BTRFS_QGROUP_OPER_ADD_SHARED)
> + update_qg_new_refcnt(qg, seq, 1);
> + else
> + update_qg_old_refcnt(qg, seq, 1);
> list_for_each_entry(glist, &qg->groups, next_group) {
> ret = ulist_add(tmp, glist->group->qgroupid,
> ptr_to_u64(glist->group), GFP_ATOMIC);
> @@ -1706,11 +1740,14 @@ static int qgroup_adjust_counters(struct btrfs_fs_info *fs_info,
> bool dirty = false;
>
> qg = u64_to_ptr(unode->aux);
> + cur_old_count = get_qg_old_refcnt(qg, seq);
> + cur_new_count = get_qg_new_refcnt(qg, seq);
> +
> /*
> * Wasn't referenced before but is now, add to the reference
> * counters.
> */
> - if (qg->old_refcnt <= seq && qg->new_refcnt > seq) {
> + if (cur_old_count == 0 && cur_new_count > 0) {
> qg->rfer += num_bytes;
> qg->rfer_cmpr += num_bytes;
> dirty = true;
> @@ -1720,21 +1757,12 @@ static int qgroup_adjust_counters(struct btrfs_fs_info *fs_info,
> * Was referenced before but isn't now, subtract from the
> * reference counters.
> */
> - if (qg->old_refcnt > seq && qg->new_refcnt <= seq) {
> + if (cur_old_count > 0 && cur_new_count == 0) {
> qg->rfer -= num_bytes;
> qg->rfer_cmpr -= num_bytes;
> dirty = true;
> }
>
> - if (qg->old_refcnt < seq)
> - cur_old_count = 0;
> - else
> - cur_old_count = qg->old_refcnt - seq;
> - if (qg->new_refcnt < seq)
> - cur_new_count = 0;
> - else
> - cur_new_count = qg->new_refcnt - seq;
> -
> /*
> * If our refcount was the same as the roots previously but our
> * new count isn't the same as the number of roots now then we
> --
> 2.3.1
>
> --
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Cleanup open-coded old/new_refcnt update and read.
2015-03-12 13:45 ` Liu Bo
@ 2015-03-13 0:37 ` Qu Wenruo
2015-03-13 13:48 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2015-03-13 0:37 UTC (permalink / raw)
To: bo.li.liu; +Cc: linux-btrfs
-------- Original Message --------
Subject: Re: [PATCH] btrfs: qgroup: Cleanup open-coded old/new_refcnt
update and read.
From: Liu Bo <bo.li.liu@oracle.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2015年03月12日 21:45
> On Thu, Mar 12, 2015 at 04:33:27PM +0800, Qu Wenruo wrote:
>> Use inline functions to do such things, to improve readability.
>
> This has actually hiden informations behind the helper function and
> old_refcnt and new_refcnt can explain themselves, I don't see any benefit.
Old/new_refcnt won't explain themselves.
In fact, it's a combination of sequence and reference number, which is
very confusing if you don't read qgroup_shared_accouting().
This patch will hides the confusing detail, caller will only need to
care the number to increase.
Also, I'm considering split old/new_refcnt into seq and change them to
the delta to seq.
Current implement makes qgroup oper->seq completely meaningless.
Based on this patch, we only need some minor change to do this.
Thanks,
Qu
>
> Thanks,
>
> -liubo
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>> fs/btrfs/qgroup.c | 102 ++++++++++++++++++++++++++++++++++--------------------
>> 1 file changed, 65 insertions(+), 37 deletions(-)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index 97159a8..3f2a1e7 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -84,11 +84,58 @@ struct btrfs_qgroup {
>>
>> /*
>> * temp variables for accounting operations
>> + * Refer to qgroup_shared_accouting() for details.
>> */
>> u64 old_refcnt;
>> u64 new_refcnt;
>> };
>>
>> +static void update_qg_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod,
>> + int update_old)
>> +{
>> + if (update_old) {
>> + if (qg->old_refcnt < seq)
>> + qg->old_refcnt = seq;
>> + qg->old_refcnt += mod;
>> + } else {
>> + if (qg->new_refcnt < seq)
>> + qg->new_refcnt = seq;
>> + qg->new_refcnt += mod;
>> + }
>> +}
>> +
>> +static void update_qg_old_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod)
>> +{
>> + update_qg_refcnt(qg, seq, mod, 1);
>> +}
>> +
>> +static void update_qg_new_refcnt(struct btrfs_qgroup *qg, u64 seq, int mod)
>> +{
>> + update_qg_refcnt(qg, seq, mod, 0);
>> +}
>> +
>> +static inline u64 get_qg_refcnt(struct btrfs_qgroup *qg, u64 seq, int get_old)
>> +{
>> + if (get_old) {
>> + if (qg->old_refcnt < seq)
>> + return 0;
>> + return qg->old_refcnt - seq;
>> + }
>> + if (qg->new_refcnt < seq)
>> + return 0;
>> + return qg->new_refcnt - seq;
>> +}
>> +
>> +static inline u64 get_qg_old_refcnt(struct btrfs_qgroup *qg, u64 seq)
>> +{
>> + return get_qg_refcnt(qg, seq, 1);
>> +}
>> +
>> +static inline u64 get_qg_new_refcnt(struct btrfs_qgroup *qg, u64 seq)
>> +{
>> + return get_qg_refcnt(qg, seq, 0);
>> +}
>> +
>> /*
>> * glue structure to represent the relations between qgroups.
>> */
>> @@ -1497,6 +1544,7 @@ static int qgroup_calc_old_refcnt(struct btrfs_fs_info *fs_info,
>> ULIST_ITER_INIT(&tmp_uiter);
>> while ((tmp_unode = ulist_next(tmp, &tmp_uiter))) {
>> struct btrfs_qgroup_list *glist;
>> + int mod;
>>
>> qg = u64_to_ptr(tmp_unode->aux);
>> /*
>> @@ -1508,20 +1556,15 @@ static int qgroup_calc_old_refcnt(struct btrfs_fs_info *fs_info,
>> * upper level qgroups in order to determine exclusive
>> * counts.
>> *
>> - * For rescan we want to set old_refcnt to seq so our
>> - * exclusive calculations end up correct.
>> + * For rescan none of the extent is recorded before so
>> + * we just don't add old_refcnt.
>> */
>> if (rescan)
>> - qg->old_refcnt = seq;
>> - else if (qg->old_refcnt < seq)
>> - qg->old_refcnt = seq + 1;
>> + mod = 0;
>> else
>> - qg->old_refcnt++;
>> -
>> - if (qg->new_refcnt < seq)
>> - qg->new_refcnt = seq + 1;
>> - else
>> - qg->new_refcnt++;
>> + mod = 1;
>> + update_qg_old_refcnt(qg, seq, mod);
>> + update_qg_new_refcnt(qg, seq, 1);
>> list_for_each_entry(glist, &qg->groups, next_group) {
>> ret = ulist_add(qgroups, glist->group->qgroupid,
>> ptr_to_u64(glist->group),
>> @@ -1619,10 +1662,8 @@ next:
>> qg->old_refcnt = seq + 1;
>> else
>> qg->old_refcnt++;
>> - if (qg->new_refcnt < seq)
>> - qg->new_refcnt = seq + 1;
>> - else
>> - qg->new_refcnt++;
>> + update_qg_old_refcnt(qg, seq, 1);
>> + update_qg_new_refcnt(qg, seq, 1);
>> list_for_each_entry(glist, &qg->groups, next_group) {
>> ret = ulist_add(qgroups, glist->group->qgroupid,
>> ptr_to_u64(glist->group), GFP_ATOMIC);
>> @@ -1663,17 +1704,10 @@ static int qgroup_calc_new_refcnt(struct btrfs_fs_info *fs_info,
>> struct btrfs_qgroup_list *glist;
>>
>> qg = u64_to_ptr(unode->aux);
>> - if (oper->type == BTRFS_QGROUP_OPER_ADD_SHARED) {
>> - if (qg->new_refcnt < seq)
>> - qg->new_refcnt = seq + 1;
>> - else
>> - qg->new_refcnt++;
>> - } else {
>> - if (qg->old_refcnt < seq)
>> - qg->old_refcnt = seq + 1;
>> - else
>> - qg->old_refcnt++;
>> - }
>> + if (oper->type == BTRFS_QGROUP_OPER_ADD_SHARED)
>> + update_qg_new_refcnt(qg, seq, 1);
>> + else
>> + update_qg_old_refcnt(qg, seq, 1);
>> list_for_each_entry(glist, &qg->groups, next_group) {
>> ret = ulist_add(tmp, glist->group->qgroupid,
>> ptr_to_u64(glist->group), GFP_ATOMIC);
>> @@ -1706,11 +1740,14 @@ static int qgroup_adjust_counters(struct btrfs_fs_info *fs_info,
>> bool dirty = false;
>>
>> qg = u64_to_ptr(unode->aux);
>> + cur_old_count = get_qg_old_refcnt(qg, seq);
>> + cur_new_count = get_qg_new_refcnt(qg, seq);
>> +
>> /*
>> * Wasn't referenced before but is now, add to the reference
>> * counters.
>> */
>> - if (qg->old_refcnt <= seq && qg->new_refcnt > seq) {
>> + if (cur_old_count == 0 && cur_new_count > 0) {
>> qg->rfer += num_bytes;
>> qg->rfer_cmpr += num_bytes;
>> dirty = true;
>> @@ -1720,21 +1757,12 @@ static int qgroup_adjust_counters(struct btrfs_fs_info *fs_info,
>> * Was referenced before but isn't now, subtract from the
>> * reference counters.
>> */
>> - if (qg->old_refcnt > seq && qg->new_refcnt <= seq) {
>> + if (cur_old_count > 0 && cur_new_count == 0) {
>> qg->rfer -= num_bytes;
>> qg->rfer_cmpr -= num_bytes;
>> dirty = true;
>> }
>>
>> - if (qg->old_refcnt < seq)
>> - cur_old_count = 0;
>> - else
>> - cur_old_count = qg->old_refcnt - seq;
>> - if (qg->new_refcnt < seq)
>> - cur_new_count = 0;
>> - else
>> - cur_new_count = qg->new_refcnt - seq;
>> -
>> /*
>> * If our refcount was the same as the roots previously but our
>> * new count isn't the same as the number of roots now then we
>> --
>> 2.3.1
>>
>> --
>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Cleanup open-coded old/new_refcnt update and read.
2015-03-13 0:37 ` Qu Wenruo
@ 2015-03-13 13:48 ` David Sterba
0 siblings, 0 replies; 4+ messages in thread
From: David Sterba @ 2015-03-13 13:48 UTC (permalink / raw)
To: Qu Wenruo; +Cc: bo.li.liu, linux-btrfs
On Fri, Mar 13, 2015 at 08:37:59AM +0800, Qu Wenruo wrote:
> > On Thu, Mar 12, 2015 at 04:33:27PM +0800, Qu Wenruo wrote:
> >> Use inline functions to do such things, to improve readability.
> >
> > This has actually hiden informations behind the helper function and
> > old_refcnt and new_refcnt can explain themselves, I don't see any benefit.
> Old/new_refcnt won't explain themselves.
I find the helpers useful, just please name them
btrfs_qgroup_get_new_refcnt, btrfs_qgroup_get_old_refcnt etc.
> In fact, it's a combination of sequence and reference number, which is
> very confusing if you don't read qgroup_shared_accouting().
>
> This patch will hides the confusing detail, caller will only need to
> care the number to increase.
>
> Also, I'm considering split old/new_refcnt into seq and change them to
> the delta to seq.
> Current implement makes qgroup oper->seq completely meaningless.
> Based on this patch, we only need some minor change to do this.
Acked-by: David Sterba <dsterba@suse.cz>
I haven't really checked it's correct & working.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-13 13:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-12 8:33 [PATCH] btrfs: qgroup: Cleanup open-coded old/new_refcnt update and read Qu Wenruo
2015-03-12 13:45 ` Liu Bo
2015-03-13 0:37 ` Qu Wenruo
2015-03-13 13:48 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.