| Message ID | 1614675476-79534-1-git-send-email-yang.lee@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: turn btrfs_destroy_all_ordered_extents() into void functions | expand |
On Tue, Mar 02, 2021 at 04:57:56PM +0800, Yang Li wrote: > These functions always return '0' and no callers use the return > value. So make it a void function. The reasoning needs to go the other way around: you can make a function return void iff all callees also return void and don't do BUG_ON instead of proper error handling. Which in this case does not hold, because the function itself still does BUG_ON. > It fixes the following warning detected by coccinelle: > ./fs/btrfs/disk-io.c:4522:5-8: Unneeded variable: "ret". Return "0" on > line 4530 Yeah tools can identify the simple cases but fixing that properly needs to extend to the whole callgraph and understand all the contexts where local data are undone and errors propagated up the callchanin.
Hi Yang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on kdave/for-next]
[also build test ERROR on v5.15-rc3 next-20210921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Yang-Li/btrfs-turn-btrfs_destroy_all_ordered_extents-into-void-functions/20210929-133924
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: arc-randconfig-r043-20210929 (attached as .config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c6f80dfd41a91ba3a38e031d0611b91bd1618085
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yang-Li/btrfs-turn-btrfs_destroy_all_ordered_extents-into-void-functions/20210929-133924
git checkout c6f80dfd41a91ba3a38e031d0611b91bd1618085
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> fs/btrfs/disk-io.c:4635:13: error: conflicting types for 'btrfs_destroy_delayed_refs'; have 'void(struct btrfs_transaction *, struct btrfs_fs_info *)'
4635 | static void btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/disk-io.c:56:12: note: previous declaration of 'btrfs_destroy_delayed_refs' with type 'int(struct btrfs_transaction *, struct btrfs_fs_info *)'
56 | static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
vim +4635 fs/btrfs/disk-io.c
4634
> 4635 static void btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
4636 struct btrfs_fs_info *fs_info)
4637 {
4638 struct rb_node *node;
4639 struct btrfs_delayed_ref_root *delayed_refs;
4640 struct btrfs_delayed_ref_node *ref;
4641
4642 delayed_refs = &trans->delayed_refs;
4643
4644 spin_lock(&delayed_refs->lock);
4645 if (atomic_read(&delayed_refs->num_entries) == 0) {
4646 spin_unlock(&delayed_refs->lock);
4647 btrfs_debug(fs_info, "delayed_refs has NO entry");
4648 return;
4649 }
4650
4651 while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
4652 struct btrfs_delayed_ref_head *head;
4653 struct rb_node *n;
4654 bool pin_bytes = false;
4655
4656 head = rb_entry(node, struct btrfs_delayed_ref_head,
4657 href_node);
4658 if (btrfs_delayed_ref_lock(delayed_refs, head))
4659 continue;
4660
4661 spin_lock(&head->lock);
4662 while ((n = rb_first_cached(&head->ref_tree)) != NULL) {
4663 ref = rb_entry(n, struct btrfs_delayed_ref_node,
4664 ref_node);
4665 ref->in_tree = 0;
4666 rb_erase_cached(&ref->ref_node, &head->ref_tree);
4667 RB_CLEAR_NODE(&ref->ref_node);
4668 if (!list_empty(&ref->add_list))
4669 list_del(&ref->add_list);
4670 atomic_dec(&delayed_refs->num_entries);
4671 btrfs_put_delayed_ref(ref);
4672 }
4673 if (head->must_insert_reserved)
4674 pin_bytes = true;
4675 btrfs_free_delayed_extent_op(head->extent_op);
4676 btrfs_delete_ref_head(delayed_refs, head);
4677 spin_unlock(&head->lock);
4678 spin_unlock(&delayed_refs->lock);
4679 mutex_unlock(&head->mutex);
4680
4681 if (pin_bytes) {
4682 struct btrfs_block_group *cache;
4683
4684 cache = btrfs_lookup_block_group(fs_info, head->bytenr);
4685 BUG_ON(!cache);
4686
4687 spin_lock(&cache->space_info->lock);
4688 spin_lock(&cache->lock);
4689 cache->pinned += head->num_bytes;
4690 btrfs_space_info_update_bytes_pinned(fs_info,
4691 cache->space_info, head->num_bytes);
4692 cache->reserved -= head->num_bytes;
4693 cache->space_info->bytes_reserved -= head->num_bytes;
4694 spin_unlock(&cache->lock);
4695 spin_unlock(&cache->space_info->lock);
4696
4697 btrfs_put_block_group(cache);
4698
4699 btrfs_error_unpin_extent_range(fs_info, head->bytenr,
4700 head->bytenr + head->num_bytes - 1);
4701 }
4702 btrfs_cleanup_ref_head_accounting(fs_info, delayed_refs, head);
4703 btrfs_put_delayed_ref_head(head);
4704 cond_resched();
4705 spin_lock(&delayed_refs->lock);
4706 }
4707 btrfs_qgroup_destroy_extent_records(trans);
4708
4709 spin_unlock(&delayed_refs->lock);
4710
4711 return;
4712 }
4713
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 41b718c..7c9e1b4 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -4513,13 +4513,12 @@ static void btrfs_destroy_all_ordered_extents(struct btrfs_fs_info *fs_info) btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1); } -static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans, +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; @@ -4527,7 +4526,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_debug(fs_info, "delayed_refs has NO entry"); - return ret; + return; } while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) { @@ -4593,7 +4592,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans, spin_unlock(&delayed_refs->lock); - return ret; + return; } static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
These functions always return '0' and no callers use the return value. So make it a void function. It fixes the following warning detected by coccinelle: ./fs/btrfs/disk-io.c:4522:5-8: Unneeded variable: "ret". Return "0" on line 4530 Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> --- fs/btrfs/disk-io.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)