[PATCHv3 0/3] change some function to be void function

[PATCHv3 0/3] change some function to be void function

[zhong jiang]

v3->v2:
 - Repost the patches with detailed changelog.
v1->v2:
 - Fix v1 issue that forget to return after remove the variable "ret".

zhong jiang (3):
  fs/btrfs/disk-io: change btrfs_destroy_delayed_refs to be void
    function
  fs/btrfs/extent-tree: change btrfs_free_reserved_bytes to be void
    funtion
  fs/btrfs/tree-log: change btrfs_pin_log_trans to be void function

 fs/btrfs/disk-io.c     | 13 +++++--------
 fs/btrfs/extent-tree.c |  6 ++----
 fs/btrfs/tree-log.c    |  5 +----
 fs/btrfs/tree-log.h    |  2 +-
 4 files changed, 9 insertions(+), 17 deletions(-)

-- 
1.7.12.4

[PATCHv3 1/3] fs/btrfs/disk-io: change btrfs_destroy_delayed_refs to be void function

[zhong jiang]

btrfs_destroy_delayed_refs defines the variable "ret" for return value, but
it is not modified after initialization. Further, I find that any of the
callees do not handle the return value.  So it is safe to drop the
unneeded value "ret".

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 fs/btrfs/disk-io.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 5124c15..f972fdd 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -53,8 +53,8 @@
 static const struct extent_io_ops btree_extent_io_ops;
 static void end_workqueue_fn(struct btrfs_work *work);
 static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
-static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
-				      struct btrfs_fs_info *fs_info);
+static void btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
+				       struct btrfs_fs_info *fs_info);
 static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root);
 static int btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
 					struct extent_io_tree *dirty_pages,
@@ -4186,13 +4186,12 @@ static void btrfs_destroy_all_ordered_extents(struct btrfs_fs_info *fs_info)
 	spin_unlock(&fs_info->ordered_root_lock);
 }
 
-static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
-				      struct btrfs_fs_info *fs_info)
+static void btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
+				       struct btrfs_fs_info *fs_info)
 {
 	struct rb_node *node;
 	struct btrfs_delayed_ref_root *delayed_refs;
 	struct btrfs_delayed_ref_node *ref;
-	int ret = 0;
 
 	delayed_refs = &trans->delayed_refs;
 
@@ -4200,7 +4199,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
 	if (atomic_read(&delayed_refs->num_entries) == 0) {
 		spin_unlock(&delayed_refs->lock);
 		btrfs_info(fs_info, "delayed_refs has NO entry");
-		return ret;
+		return;
 	}
 
 	while ((node = rb_first(&delayed_refs->href_root)) != NULL) {
@@ -4254,8 +4253,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
 	}
 
 	spin_unlock(&delayed_refs->lock);
-
-	return ret;
 }
 
 static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
-- 
1.7.12.4

[PATCHv3 2/3] fs/btrfs/extent-tree: change btrfs_free_reserved_bytes to be void funtion

[zhong jiang]

btrfs_free_reserved_bytes defines the variable "ret" for return value,
but it is not modified after initialzation. Further, I find that any of
the callees do not handle the return value, so it is safe to drop the
unneeded value "ret".

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 fs/btrfs/extent-tree.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f77226d..ff305f5 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6464,11 +6464,10 @@ static int btrfs_add_reserved_bytes(struct btrfs_block_group_cache *cache,
  * reserve set to 0 in order to clear the reservation.
  */
 
-static int btrfs_free_reserved_bytes(struct btrfs_block_group_cache *cache,
-				     u64 num_bytes, int delalloc)
+static void btrfs_free_reserved_bytes(struct btrfs_block_group_cache *cache,
+				      u64 num_bytes, int delalloc)
 {
 	struct btrfs_space_info *space_info = cache->space_info;
-	int ret = 0;
 
 	spin_lock(&space_info->lock);
 	spin_lock(&cache->lock);
@@ -6481,7 +6480,6 @@ static int btrfs_free_reserved_bytes(struct btrfs_block_group_cache *cache,
 		cache->delalloc_bytes -= num_bytes;
 	spin_unlock(&cache->lock);
 	spin_unlock(&space_info->lock);
-	return ret;
 }
 void btrfs_prepare_extent_commit(struct btrfs_fs_info *fs_info)
 {
-- 
1.7.12.4

[PATCHv3 3/3] fs/btrfs/tree-log: change btrfs_pin_log_trans to be void function

[zhong jiang]

btrfs_pin_log_trans defines the variable "ret" for return valule,
but it is not modified after initialization. Further, I find that
any of the callees do not handle the return value, so it is safe
to drop the unneeded value "ret".

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 fs/btrfs/tree-log.c | 5 +----
 fs/btrfs/tree-log.h | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 1650dc4..4ed7379 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -205,14 +205,11 @@ static int join_running_log_trans(struct btrfs_root *root)
  * until you call btrfs_end_log_trans() or it makes any future
  * log transactions wait until you call btrfs_end_log_trans()
  */
-int btrfs_pin_log_trans(struct btrfs_root *root)
+void btrfs_pin_log_trans(struct btrfs_root *root)
 {
-	int ret = -ENOENT;
-
 	mutex_lock(&root->log_mutex);
 	atomic_inc(&root->log_writers);
 	mutex_unlock(&root->log_mutex);
-	return ret;
 }
 
 /*
diff --git a/fs/btrfs/tree-log.h b/fs/btrfs/tree-log.h
index 122e68b..8977678 100644
--- a/fs/btrfs/tree-log.h
+++ b/fs/btrfs/tree-log.h
@@ -65,7 +65,7 @@ int btrfs_del_inode_ref_in_log(struct btrfs_trans_handle *trans,
 			       const char *name, int name_len,
 			       struct btrfs_inode *inode, u64 dirid);
 void btrfs_end_log_trans(struct btrfs_root *root);
-int btrfs_pin_log_trans(struct btrfs_root *root);
+void btrfs_pin_log_trans(struct btrfs_root *root);
 void btrfs_record_unlink_dir(struct btrfs_trans_handle *trans,
 			     struct btrfs_inode *dir, struct btrfs_inode *inode,
 			     int for_rename);
-- 
1.7.12.4

Re: [PATCHv3 1/3] fs/btrfs/disk-io: change btrfs_destroy_delayed_refs to be void function

[David Sterba]

On Wed, Aug 08, 2018 at 09:47:53PM +0800, zhong jiang wrote:
> btrfs_destroy_delayed_refs defines the variable "ret" for return value, but
> it is not modified after initialization. Further, I find that any of the
> callees do not handle the return value.  So it is safe to drop the
> unneeded value "ret".

The callers care about the return values, not the callees. My point in
the previous patch iterations was to make sure that none of the callees,
ie. the functions that are called by btrfs_destroy_delayed_refs, do not
lack error handling.

And I found one that does, btrfs_pin_extent, near the end of the while
loop. So, there's more work and I can't apply this patch until it's
fixed, which may make this patch obsolete as the return value would be
actually needed.

Re: [PATCHv3 0/3] change some function to be void function

[David Sterba]

On Wed, Aug 08, 2018 at 09:47:52PM +0800, zhong jiang wrote:
> v3->v2:
>  - Repost the patches with detailed changelog.
> v1->v2:
>  - Fix v1 issue that forget to return after remove the variable "ret".
> 
> zhong jiang (3):
>   fs/btrfs/disk-io: change btrfs_destroy_delayed_refs to be void
>     function
>   fs/btrfs/extent-tree: change btrfs_free_reserved_bytes to be void
>     funtion
>   fs/btrfs/tree-log: change btrfs_pin_log_trans to be void function

The patches 2 and 3 can be applied, but please update che subject and
put just "btrfs: " there, not the full file path. That we can find in
the diff. The changelogs can be still improved to be clear about callers
vs callees. Please update and resend.

Re: [PATCHv3 0/3] change some function to be void function

[zhong jiang]

On 2018/8/15 20:53, David Sterba wrote:
> On Wed, Aug 08, 2018 at 09:47:52PM +0800, zhong jiang wrote:
>> v3->v2:
>>  - Repost the patches with detailed changelog.
>> v1->v2:
>>  - Fix v1 issue that forget to return after remove the variable "ret".
>>
>> zhong jiang (3):
>>   fs/btrfs/disk-io: change btrfs_destroy_delayed_refs to be void
>>     function
>>   fs/btrfs/extent-tree: change btrfs_free_reserved_bytes to be void
>>     funtion
>>   fs/btrfs/tree-log: change btrfs_pin_log_trans to be void function
> The patches 2 and 3 can be applied, but please update che subject and
> put just "btrfs: " there, not the full file path. That we can find in
> the diff. The changelogs can be still improved to be clear about callers
> vs callees. Please update and resend.
>
> .
>
 Thank you for review and  suggestion.   I will repost v3.

Sincerely,
zhong jiang