Commit b1386563 authored Nov 23, 2023 by Qu Wenruo Committed by Zheng Zengkai Nov 23, 2023

btrfs: make found_logical_ret parameter mandatory for function queue_scrub_stripe()

stable inclusion
from stable-v6.6.2
commit 6927a91ccf724277b902533ed488edc5bcff9c9e
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/I8IW7G

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=6927a91ccf724277b902533ed488edc5bcff9c9e

--------------------------------

[ Upstream commit 47e2b06b7b5cb356a987ba3429550c3a89ea89d6 ]

[BUG]
There is a compilation warning reported on commit ae76d8e3 ("btrfs:
scrub: fix grouping of read IO"), where gcc (14.0.0 20231022 experimental)
is reporting the following uninitialized variable:

  fs/btrfs/scrub.c: In function ‘scrub_simple_mirror.isra’:
  fs/btrfs/scrub.c:2075:29: error: ‘found_logical’ may be used uninitialized [-Werror=maybe-uninitialized[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wmaybe-uninitialized]]
   2075 |                 cur_logical = found_logical + BTRFS_STRIPE_LEN;
  fs/btrfs/scrub.c:2040:21: note: ‘found_logical’ was declared here
   2040 |                 u64 found_logical;
        |                     ^~~~~~~~~~~~~

[CAUSE]
This is a false alert, as @found_logical is passed as parameter
@found_logical_ret of function queue_scrub_stripe().

As long as queue_scrub_stripe() returned 0, we would update
@found_logical_ret.  And if queue_scrub_stripe() returned >0 or <0, the
caller would not utilized @found_logical, thus there should be nothing
wrong.

Although the triggering gcc is still experimental, it looks like the
extra check on "if (found_logical_ret)" can sometimes confuse the
compiler.

Meanwhile the only caller of queue_scrub_stripe() is always passing a
valid pointer, there is no need for such check at all.

[FIX]
Although the report itself is a false alert, we can still make it more
explicit by:

- Replace the check for @found_logical_ret with ASSERT()

- Initialize @found_logical to U64_MAX

- Add one extra ASSERT() to make sure @found_logical got updated

Link: https://lore.kernel.org/linux-btrfs/87fs1x1p93.fsf@gentoo.org/


Fixes: ae76d8e3 ("btrfs: scrub: fix grouping of read IO")
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com>

parent b546d392

fs/btrfs/scrub.c

+7 −3

Original line number	Diff line number	Diff line
		@@ -1798,6 +1798,9 @@ static int queue_scrub_stripe(struct scrub_ctx sctx, struct btrfs_block_group
		*/
		ASSERT(sctx->cur_stripe < SCRUB_TOTAL_STRIPES);

		/* @found_logical_ret must be specified. */
		ASSERT(found_logical_ret);

		stripe = &sctx->stripes[sctx->cur_stripe];
		scrub_reset_stripe(stripe);
		ret = scrub_find_fill_first_stripe(bg, &sctx->extent_path,
		@@ -1806,7 +1809,6 @@ static int queue_scrub_stripe(struct scrub_ctx sctx, struct btrfs_block_group
		/* Either >0 as no more extents or <0 for error. */
		if (ret)
		return ret;
		if (found_logical_ret)
		*found_logical_ret = stripe->logical;
		sctx->cur_stripe++;

		@@ -2010,7 +2012,7 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx,

		/* Go through each extent items inside the logical range */
		while (cur_logical < logical_end) {
		u64 found_logical;
		u64 found_logical = U64_MAX;
		u64 cur_physical = physical + cur_logical - logical_start;

		/* Canceled? */
		@@ -2045,6 +2047,8 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx,
		if (ret < 0)
		break;

		/* queue_scrub_stripe() returned 0, @found_logical must be updated. */
		ASSERT(found_logical != U64_MAX);
		cur_logical = found_logical + BTRFS_STRIPE_LEN;

		/* Don't hold CPU for too long time */