[PATCH 0/7] Qgroup cleanups

[PATCH 0/7] Qgroup cleanups

[David Sterba]

I've spotted some cleanup-ready code while going through the file, so here it
is. There are no changes to the core qgoups code, just the support and
framework code. For 4.11.

David Sterba (7):
      btrfs: qgroups: make __del_qgroup_relation static
      btrfs: ulist: make the finalization function public
      btrfs: embed extent_changeset::range_changed to the structure
      btrfs: check quota status earlier and don't do unnecessary frees
      btrfs: remove unnecessary mutex lock in qgroup_account_snapshot
      btrfs: qgroups: opencode qgroup_free helper
      btrfs: remove pointless rcu protection from btrfs_qgroup_inherit

 fs/btrfs/extent_io.c   |  2 +-
 fs/btrfs/extent_io.h   |  2 +-
 fs/btrfs/qgroup.c      | 51 ++++++++++++++++++++++----------------------------
 fs/btrfs/transaction.c |  6 +-----
 fs/btrfs/ulist.c       |  2 +-
 fs/btrfs/ulist.h       |  1 +
 6 files changed, 27 insertions(+), 37 deletions(-)

[PATCH 1/7] btrfs: qgroups: make __del_qgroup_relation static

[David Sterba]

Internal helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/qgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 8496dbf3f38b..971701328229 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1223,7 +1223,7 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-int __del_qgroup_relation(struct btrfs_trans_handle *trans,
+static int __del_qgroup_relation(struct btrfs_trans_handle *trans,
 			      struct btrfs_fs_info *fs_info, u64 src, u64 dst)
 {
 	struct btrfs_root *quota_root;
-- 
2.10.1

[PATCH 2/7] btrfs: ulist: make the finalization function public

[David Sterba]

Make ulist_fini externally visible so the ulist API is complete.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ulist.c | 2 +-
 fs/btrfs/ulist.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c
index b1434bb57e36..5deee56434fc 100644
--- a/fs/btrfs/ulist.c
+++ b/fs/btrfs/ulist.c
@@ -58,7 +58,7 @@ void ulist_init(struct ulist *ulist)
  * This is useful in cases where the base 'struct ulist' has been statically
  * allocated.
  */
-static void ulist_fini(struct ulist *ulist)
+void ulist_fini(struct ulist *ulist)
 {
 	struct ulist_node *node;
 	struct ulist_node *next;
diff --git a/fs/btrfs/ulist.h b/fs/btrfs/ulist.h
index 007b22fff3f9..1a4130443d7e 100644
--- a/fs/btrfs/ulist.h
+++ b/fs/btrfs/ulist.h
@@ -44,6 +44,7 @@ struct ulist {
 };
 
 void ulist_init(struct ulist *ulist);
+void ulist_fini(struct ulist *ulist);
 void ulist_reinit(struct ulist *ulist);
 struct ulist *ulist_alloc(gfp_t gfp_mask);
 void ulist_free(struct ulist *ulist);
-- 
2.10.1

[PATCH 3/7] btrfs: embed extent_changeset::range_changed to the structure

[David Sterba]

We can embed range_changed to the extent changeset to address following
problems:

- no need to allocate ulist dynamically, we also get rid of the GFP_NOFS
  for free
- fix lack of allocation failure checking in btrfs_qgroup_reserve_data

The stack consuption where extent_changeset is used slightly increases:

before: 16
after: 16 - 8 (for pointer) + 32 (sizeof ulist) = 40

Which is bearable.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c |  2 +-
 fs/btrfs/extent_io.h |  2 +-
 fs/btrfs/qgroup.c    | 24 +++++++++---------------
 3 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 9df6ed30de00..9140847bfb0c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -144,7 +144,7 @@ static void add_extent_changeset(struct extent_state *state, unsigned bits,
 	if (!set && (state->state & bits) == 0)
 		return;
 	changeset->bytes_changed += state->end - state->start + 1;
-	ret = ulist_add(changeset->range_changed, state->start, state->end,
+	ret = ulist_add(&changeset->range_changed, state->start, state->end,
 			GFP_ATOMIC);
 	/* ENOMEM */
 	BUG_ON(ret < 0);
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 4551a5b4b8f5..270d03be290e 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -193,7 +193,7 @@ struct extent_changeset {
 	u64 bytes_changed;
 
 	/* Changed ranges */
-	struct ulist *range_changed;
+	struct ulist range_changed;
 };
 
 static inline void extent_set_compress_type(unsigned long *bio_flags,
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 971701328229..7cc2e2221f55 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2828,7 +2828,7 @@ int btrfs_qgroup_reserve_data(struct inode *inode, u64 start, u64 len)
 		return 0;
 
 	changeset.bytes_changed = 0;
-	changeset.range_changed = ulist_alloc(GFP_NOFS);
+	ulist_init(&changeset.range_changed);
 	ret = set_record_extent_bits(&BTRFS_I(inode)->io_tree, start,
 			start + len -1, EXTENT_QGROUP_RESERVED, &changeset);
 	trace_btrfs_qgroup_reserve_data(inode, start, len,
@@ -2840,17 +2840,17 @@ int btrfs_qgroup_reserve_data(struct inode *inode, u64 start, u64 len)
 	if (ret < 0)
 		goto cleanup;
 
-	ulist_free(changeset.range_changed);
+	ulist_fini(&changeset.range_changed);
 	return ret;
 
 cleanup:
 	/* cleanup already reserved ranges */
 	ULIST_ITER_INIT(&uiter);
-	while ((unode = ulist_next(changeset.range_changed, &uiter)))
+	while ((unode = ulist_next(&changeset.range_changed, &uiter)))
 		clear_extent_bit(&BTRFS_I(inode)->io_tree, unode->val,
 				 unode->aux, EXTENT_QGROUP_RESERVED, 0, 0, NULL,
 				 GFP_NOFS);
-	ulist_free(changeset.range_changed);
+	ulist_fini(&changeset.range_changed);
 	return ret;
 }
 
@@ -2862,10 +2862,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, u64 start, u64 len,
 	int ret;
 
 	changeset.bytes_changed = 0;
-	changeset.range_changed = ulist_alloc(GFP_NOFS);
-	if (!changeset.range_changed)
-		return -ENOMEM;
-
+	ulist_init(&changeset.range_changed);
 	ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, start, 
 			start + len -1, EXTENT_QGROUP_RESERVED, &changeset);
 	if (ret < 0)
@@ -2878,7 +2875,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, u64 start, u64 len,
 	trace_btrfs_qgroup_release_data(inode, start, len,
 					changeset.bytes_changed, trace_op);
 out:
-	ulist_free(changeset.range_changed);
+	ulist_fini(&changeset.range_changed);
 	return ret;
 }
 
@@ -2976,22 +2973,19 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode)
 	int ret;
 
 	changeset.bytes_changed = 0;
-	changeset.range_changed = ulist_alloc(GFP_NOFS);
-	if (WARN_ON(!changeset.range_changed))
-		return;
-
+	ulist_init(&changeset.range_changed);
 	ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, 0, (u64)-1,
 			EXTENT_QGROUP_RESERVED, &changeset);
 
 	WARN_ON(ret < 0);
 	if (WARN_ON(changeset.bytes_changed)) {
 		ULIST_ITER_INIT(&iter);
-		while ((unode = ulist_next(changeset.range_changed, &iter))) {
+		while ((unode = ulist_next(&changeset.range_changed, &iter))) {
 			btrfs_warn(BTRFS_I(inode)->root->fs_info,
 				"leaking qgroup reserved space, ino: %lu, start: %llu, end: %llu",
 				inode->i_ino, unode->val, unode->aux);
 		}
 		qgroup_free(BTRFS_I(inode)->root, changeset.bytes_changed);
 	}
-	ulist_free(changeset.range_changed);
+	ulist_fini(&changeset.range_changed);
 }
-- 
2.10.1

[PATCH 4/7] btrfs: check quota status earlier and don't do unnecessary frees

[David Sterba]

Status of quotas should be the first check in
btrfs_qgroup_account_extent and we can return immediatelly, no need to
do no-op ulist frees.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/qgroup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 7cc2e2221f55..2b22223c2ad9 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1945,13 +1945,14 @@ btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans,
 	u64 nr_old_roots = 0;
 	int ret = 0;
 
+	if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
+		return 0;
+
 	if (new_roots)
 		nr_new_roots = new_roots->nnodes;
 	if (old_roots)
 		nr_old_roots = old_roots->nnodes;
 
-	if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
-		goto out_free;
 	BUG_ON(!fs_info->quota_root);
 
 	trace_btrfs_qgroup_account_extent(fs_info, bytenr, num_bytes,
-- 
2.10.1

[PATCH 5/7] btrfs: remove unnecessary mutex lock in qgroup_account_snapshot

[David Sterba]

The quota status used to be tracked as a variable, so the mutex was
needed (until "Btrfs: add a flags field to btrfs_fs_info" afcdd129e05a9).
Since the status is a bit modified atomically and we don't hold the
mutex beyond the check, we can drop it.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/transaction.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 48aabb367f73..5eefd77bafc7 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1364,12 +1364,8 @@ static int qgroup_account_snapshot(struct btrfs_trans_handle *trans,
 	 * enabled. If this check races with the ioctl, rescan will
 	 * kick in anyway.
 	 */
-	mutex_lock(&fs_info->qgroup_ioctl_lock);
-	if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)) {
-		mutex_unlock(&fs_info->qgroup_ioctl_lock);
+	if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
 		return 0;
-	}
-	mutex_unlock(&fs_info->qgroup_ioctl_lock);
 
 	/*
 	 * We are going to commit transaction, see btrfs_commit_transaction()
-- 
2.10.1

[PATCH 6/7] btrfs: qgroups: opencode qgroup_free helper

[David Sterba]

The helper name is not too helpful and is just wrapping a simple call.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/qgroup.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 2b22223c2ad9..114d6770dc23 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2465,11 +2465,6 @@ void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
 	spin_unlock(&fs_info->qgroup_lock);
 }
 
-static inline void qgroup_free(struct btrfs_root *root, u64 num_bytes)
-{
-	return btrfs_qgroup_free_refroot(root->fs_info, root->objectid,
-					 num_bytes);
-}
 void assert_qgroups_uptodate(struct btrfs_trans_handle *trans)
 {
 	if (list_empty(&trans->qgroup_ref_list) && !trans->delayed_ref_elem.seq)
@@ -2870,7 +2865,9 @@ static int __btrfs_qgroup_release_data(struct inode *inode, u64 start, u64 len,
 		goto out;
 
 	if (free) {
-		qgroup_free(BTRFS_I(inode)->root, changeset.bytes_changed);
+		btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info,
+				BTRFS_I(inode)->root->objectid,
+				changeset.bytes_changed);
 		trace_op = QGROUP_FREE;
 	}
 	trace_btrfs_qgroup_release_data(inode, start, len,
@@ -2945,7 +2942,7 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root)
 	reserved = atomic_xchg(&root->qgroup_meta_rsv, 0);
 	if (reserved == 0)
 		return;
-	qgroup_free(root, reserved);
+	btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved);
 }
 
 void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes)
@@ -2959,7 +2956,7 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes)
 	BUG_ON(num_bytes != round_down(num_bytes, fs_info->nodesize));
 	WARN_ON(atomic_read(&root->qgroup_meta_rsv) < num_bytes);
 	atomic_sub(num_bytes, &root->qgroup_meta_rsv);
-	qgroup_free(root, num_bytes);
+	btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes);
 }
 
 /*
@@ -2986,7 +2983,10 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode)
 				"leaking qgroup reserved space, ino: %lu, start: %llu, end: %llu",
 				inode->i_ino, unode->val, unode->aux);
 		}
-		qgroup_free(BTRFS_I(inode)->root, changeset.bytes_changed);
+		btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info,
+				BTRFS_I(inode)->root->objectid,
+				changeset.bytes_changed);
+
 	}
 	ulist_fini(&changeset.range_changed);
 }
-- 
2.10.1

[PATCH 7/7] btrfs: remove pointless rcu protection from btrfs_qgroup_inherit

[David Sterba]

There was never need for RCU protection around reading nodesize or other
fairly constant filesystem data.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/qgroup.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 114d6770dc23..052f1410dc14 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2189,9 +2189,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
 			goto out;
 		}
 
-		rcu_read_lock();
 		level_size = fs_info->nodesize;
-		rcu_read_unlock();
 	}
 
 	/*
-- 
2.10.1

Re: [PATCH 3/7] btrfs: embed extent_changeset::range_changed to the structure

[Qu Wenruo]

At 02/13/2017 10:30 PM, David Sterba wrote:
> We can embed range_changed to the extent changeset to address following
> problems:
>
> - no need to allocate ulist dynamically, we also get rid of the GFP_NOFS
>   for free
> - fix lack of allocation failure checking in btrfs_qgroup_reserve_data

Nice catch.

>
> The stack consuption where extent_changeset is used slightly increases:
>
> before: 16
> after: 16 - 8 (for pointer) + 32 (sizeof ulist) = 40
>
> Which is bearable.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Thanks,
Qu

> ---
>  fs/btrfs/extent_io.c |  2 +-
>  fs/btrfs/extent_io.h |  2 +-
>  fs/btrfs/qgroup.c    | 24 +++++++++---------------
>  3 files changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 9df6ed30de00..9140847bfb0c 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -144,7 +144,7 @@ static void add_extent_changeset(struct extent_state *state, unsigned bits,
>  	if (!set && (state->state & bits) == 0)
>  		return;
>  	changeset->bytes_changed += state->end - state->start + 1;
> -	ret = ulist_add(changeset->range_changed, state->start, state->end,
> +	ret = ulist_add(&changeset->range_changed, state->start, state->end,
>  			GFP_ATOMIC);
>  	/* ENOMEM */
>  	BUG_ON(ret < 0);
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 4551a5b4b8f5..270d03be290e 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -193,7 +193,7 @@ struct extent_changeset {
>  	u64 bytes_changed;
>
>  	/* Changed ranges */
> -	struct ulist *range_changed;
> +	struct ulist range_changed;
>  };
>
>  static inline void extent_set_compress_type(unsigned long *bio_flags,
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 971701328229..7cc2e2221f55 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2828,7 +2828,7 @@ int btrfs_qgroup_reserve_data(struct inode *inode, u64 start, u64 len)
>  		return 0;
>
>  	changeset.bytes_changed = 0;
> -	changeset.range_changed = ulist_alloc(GFP_NOFS);
> +	ulist_init(&changeset.range_changed);
>  	ret = set_record_extent_bits(&BTRFS_I(inode)->io_tree, start,
>  			start + len -1, EXTENT_QGROUP_RESERVED, &changeset);
>  	trace_btrfs_qgroup_reserve_data(inode, start, len,
> @@ -2840,17 +2840,17 @@ int btrfs_qgroup_reserve_data(struct inode *inode, u64 start, u64 len)
>  	if (ret < 0)
>  		goto cleanup;
>
> -	ulist_free(changeset.range_changed);
> +	ulist_fini(&changeset.range_changed);
>  	return ret;
>
>  cleanup:
>  	/* cleanup already reserved ranges */
>  	ULIST_ITER_INIT(&uiter);
> -	while ((unode = ulist_next(changeset.range_changed, &uiter)))
> +	while ((unode = ulist_next(&changeset.range_changed, &uiter)))
>  		clear_extent_bit(&BTRFS_I(inode)->io_tree, unode->val,
>  				 unode->aux, EXTENT_QGROUP_RESERVED, 0, 0, NULL,
>  				 GFP_NOFS);
> -	ulist_free(changeset.range_changed);
> +	ulist_fini(&changeset.range_changed);
>  	return ret;
>  }
>
> @@ -2862,10 +2862,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, u64 start, u64 len,
>  	int ret;
>
>  	changeset.bytes_changed = 0;
> -	changeset.range_changed = ulist_alloc(GFP_NOFS);
> -	if (!changeset.range_changed)
> -		return -ENOMEM;
> -
> +	ulist_init(&changeset.range_changed);
>  	ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, start,
>  			start + len -1, EXTENT_QGROUP_RESERVED, &changeset);
>  	if (ret < 0)
> @@ -2878,7 +2875,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, u64 start, u64 len,
>  	trace_btrfs_qgroup_release_data(inode, start, len,
>  					changeset.bytes_changed, trace_op);
>  out:
> -	ulist_free(changeset.range_changed);
> +	ulist_fini(&changeset.range_changed);
>  	return ret;
>  }
>
> @@ -2976,22 +2973,19 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode)
>  	int ret;
>
>  	changeset.bytes_changed = 0;
> -	changeset.range_changed = ulist_alloc(GFP_NOFS);
> -	if (WARN_ON(!changeset.range_changed))
> -		return;
> -
> +	ulist_init(&changeset.range_changed);
>  	ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, 0, (u64)-1,
>  			EXTENT_QGROUP_RESERVED, &changeset);
>
>  	WARN_ON(ret < 0);
>  	if (WARN_ON(changeset.bytes_changed)) {
>  		ULIST_ITER_INIT(&iter);
> -		while ((unode = ulist_next(changeset.range_changed, &iter))) {
> +		while ((unode = ulist_next(&changeset.range_changed, &iter))) {
>  			btrfs_warn(BTRFS_I(inode)->root->fs_info,
>  				"leaking qgroup reserved space, ino: %lu, start: %llu, end: %llu",
>  				inode->i_ino, unode->val, unode->aux);
>  		}
>  		qgroup_free(BTRFS_I(inode)->root, changeset.bytes_changed);
>  	}
> -	ulist_free(changeset.range_changed);
> +	ulist_fini(&changeset.range_changed);
>  }
>

Re: [PATCH 2/7] btrfs: ulist: make the finalization function public

[Qu Wenruo]

At 02/13/2017 10:30 PM, David Sterba wrote:
> Make ulist_fini externally visible so the ulist API is complete.

Looks good to me.

While I'm not pretty sure if the name "ulist_fini" is good enough.

Maybe "ulist_finish" or "ulist_release" (just like release_path and 
free_path)?

Thanks,
Qu
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ulist.c | 2 +-
>  fs/btrfs/ulist.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c
> index b1434bb57e36..5deee56434fc 100644
> --- a/fs/btrfs/ulist.c
> +++ b/fs/btrfs/ulist.c
> @@ -58,7 +58,7 @@ void ulist_init(struct ulist *ulist)
>   * This is useful in cases where the base 'struct ulist' has been statically
>   * allocated.
>   */
> -static void ulist_fini(struct ulist *ulist)
> +void ulist_fini(struct ulist *ulist)
>  {
>  	struct ulist_node *node;
>  	struct ulist_node *next;
> diff --git a/fs/btrfs/ulist.h b/fs/btrfs/ulist.h
> index 007b22fff3f9..1a4130443d7e 100644
> --- a/fs/btrfs/ulist.h
> +++ b/fs/btrfs/ulist.h
> @@ -44,6 +44,7 @@ struct ulist {
>  };
>
>  void ulist_init(struct ulist *ulist);
> +void ulist_fini(struct ulist *ulist);
>  void ulist_reinit(struct ulist *ulist);
>  struct ulist *ulist_alloc(gfp_t gfp_mask);
>  void ulist_free(struct ulist *ulist);
>

Re: [PATCH 2/7] btrfs: ulist: make the finalization function public

[David Sterba]

On Tue, Feb 14, 2017 at 05:29:14PM +0800, Qu Wenruo wrote:
> 
> 
> At 02/13/2017 10:30 PM, David Sterba wrote:
> > Make ulist_fini externally visible so the ulist API is complete.
> 
> Looks good to me.
> 
> While I'm not pretty sure if the name "ulist_fini" is good enough.

> 
> Maybe "ulist_finish" or "ulist_release" (just like release_path and 
> free_path)?

Yeah, I don't like the name either, ulist_release sounds ok to me.

[PATCH 8/7] btrfs: ulist: rename ulist_fini to ulist_release

[David Sterba]

Change the name so it matches the naming we already use eg. for
btrfs_path.

Suggested-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/qgroup.c |  8 ++++----
 fs/btrfs/ulist.c  | 10 +++++-----
 fs/btrfs/ulist.h  |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 052f1410dc14..5d53e948a28d 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2834,7 +2834,7 @@ int btrfs_qgroup_reserve_data(struct inode *inode, u64 start, u64 len)
 	if (ret < 0)
 		goto cleanup;
 
-	ulist_fini(&changeset.range_changed);
+	ulist_release(&changeset.range_changed);
 	return ret;
 
 cleanup:
@@ -2844,7 +2844,7 @@ int btrfs_qgroup_reserve_data(struct inode *inode, u64 start, u64 len)
 		clear_extent_bit(&BTRFS_I(inode)->io_tree, unode->val,
 				 unode->aux, EXTENT_QGROUP_RESERVED, 0, 0, NULL,
 				 GFP_NOFS);
-	ulist_fini(&changeset.range_changed);
+	ulist_release(&changeset.range_changed);
 	return ret;
 }
 
@@ -2871,7 +2871,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, u64 start, u64 len,
 	trace_btrfs_qgroup_release_data(inode, start, len,
 					changeset.bytes_changed, trace_op);
 out:
-	ulist_fini(&changeset.range_changed);
+	ulist_release(&changeset.range_changed);
 	return ret;
 }
 
@@ -2986,5 +2986,5 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode)
 				changeset.bytes_changed);
 
 	}
-	ulist_fini(&changeset.range_changed);
+	ulist_release(&changeset.range_changed);
 }
diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c
index 5deee56434fc..d8edf164f81c 100644
--- a/fs/btrfs/ulist.c
+++ b/fs/btrfs/ulist.c
@@ -52,13 +52,13 @@ void ulist_init(struct ulist *ulist)
 }
 
 /**
- * ulist_fini - free up additionally allocated memory for the ulist
+ * ulist_release - free up additionally allocated memory for the ulist
  * @ulist:	the ulist from which to free the additional memory
  *
  * This is useful in cases where the base 'struct ulist' has been statically
  * allocated.
  */
-void ulist_fini(struct ulist *ulist)
+void ulist_release(struct ulist *ulist)
 {
 	struct ulist_node *node;
 	struct ulist_node *next;
@@ -79,7 +79,7 @@ void ulist_fini(struct ulist *ulist)
  */
 void ulist_reinit(struct ulist *ulist)
 {
-	ulist_fini(ulist);
+	ulist_release(ulist);
 	ulist_init(ulist);
 }
 
@@ -105,13 +105,13 @@ struct ulist *ulist_alloc(gfp_t gfp_mask)
  * ulist_free - free dynamically allocated ulist
  * @ulist:	ulist to free
  *
- * It is not necessary to call ulist_fini before.
+ * It is not necessary to call ulist_release before.
  */
 void ulist_free(struct ulist *ulist)
 {
 	if (!ulist)
 		return;
-	ulist_fini(ulist);
+	ulist_release(ulist);
 	kfree(ulist);
 }
 
diff --git a/fs/btrfs/ulist.h b/fs/btrfs/ulist.h
index 1a4130443d7e..53c913632733 100644
--- a/fs/btrfs/ulist.h
+++ b/fs/btrfs/ulist.h
@@ -44,7 +44,7 @@ struct ulist {
 };
 
 void ulist_init(struct ulist *ulist);
-void ulist_fini(struct ulist *ulist);
+void ulist_release(struct ulist *ulist);
 void ulist_reinit(struct ulist *ulist);
 struct ulist *ulist_alloc(gfp_t gfp_mask);
 void ulist_free(struct ulist *ulist);
-- 
2.10.1