| Message ID | 1533523978-50321-1-git-send-email-zhongjiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fs/btrfs: remove the unneeded variable "err" and change the function to be void function | expand |
On Mon, Aug 06, 2018 at 10:52:58AM +0800, zhong jiang wrote: > The err is not modified after initalization, So just remove it and make > the function to be void function. > > Signed-off-by: zhong jiang <zhongjiang@huawei.com> > --- > v1->v2: > - Merge v1 series into a patch to void same changelog. Please do one patch per function. If the change follows the same logic, it's not a problem to use the same changelog, but the patch should be revieweable and not doing unrelated things. If the function prototype or return values is changed it's easier from the reviewer's perspective to focus on just one function and the surrounding code. Even if it looks straightforward to you to merge them together. When the return value changes from int -> void, it's necessary to check wheter any of the callees is not hiding a BUG_ON that should be really turned into proper error handling in the caller. In that case the return type should stay and error handling added. After a brief look I think all functions are safe here, but that's something that should be mentioned in the changelog. Please update the patches and resend. Thanks. -- 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
On 2018/8/7 23:25, David Sterba wrote: > On Mon, Aug 06, 2018 at 10:52:58AM +0800, zhong jiang wrote: >> The err is not modified after initalization, So just remove it and make >> the function to be void function. >> >> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >> --- >> v1->v2: >> - Merge v1 series into a patch to void same changelog. > Please do one patch per function. If the change follows the same logic, > it's not a problem to use the same changelog, but the patch should be > revieweable and not doing unrelated things. > > If the function prototype or return values is changed it's easier from > the reviewer's perspective to focus on just one function and the > surrounding code. Even if it looks straightforward to you to merge them > together. > > When the return value changes from int -> void, it's necessary to check > wheter any of the callees is not hiding a BUG_ON that should be really > turned into proper error handling in the caller. In that case the return > type should stay and error handling added. > > After a brief look I think all functions are safe here, but that's > something that should be mentioned in the changelog. Please update the > patches and resend. Thanks. > > . > I will repost the patches with detailed changlog. Thank you for explaination. Thanks zhong jiang -- 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
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 468365d..bad6cc6 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, @@ -4179,13 +4179,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; @@ -4193,7 +4192,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) { @@ -4247,8 +4246,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) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index b1f2f64..ed02305 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) { 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);
The err is not modified after initalization, So just remove it and make the function to be void function. Signed-off-by: zhong jiang <zhongjiang@huawei.com> --- v1->v2: - Merge v1 series into a patch to void same changelog. - Fix v1 issue that forget to return after remove the variable "err". 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(-)