Patch "btrfs: make found_logical_ret parameter mandatory for function queue_scrub_stripe()" has been added to the 6.6-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    btrfs: make found_logical_ret parameter mandatory for function queue_scrub_stripe()

to the 6.6-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     btrfs-make-found_logical_ret-parameter-mandatory-for.patch
and it can be found in the queue-6.6 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 6ad244d326948613e17b00fe43ccf27cff206cce
Author: Qu Wenruo <wqu@xxxxxxxx>
Date:   Sat Oct 28 13:28:45 2023 +1030

    btrfs: make found_logical_ret parameter mandatory for function queue_scrub_stripe()
    
    [ Upstream commit 47e2b06b7b5cb356a987ba3429550c3a89ea89d6 ]
    
    [BUG]
    There is a compilation warning reported on commit ae76d8e3e135 ("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@xxxxxxxxxx/
    Fixes: ae76d8e3e135 ("btrfs: scrub: fix grouping of read IO")
    Reviewed-by: Anand Jain <anand.jain@xxxxxxxxxx>
    Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
    Reviewed-by: David Sterba <dsterba@xxxxxxxx>
    Signed-off-by: David Sterba <dsterba@xxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index b877203f1dc5a..4445a52a07076 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -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,8 +1809,7 @@ 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;
+	*found_logical_ret = stripe->logical;
 	sctx->cur_stripe++;
 
 	/* We filled one group, submit it. */
@@ -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 */



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux