Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: Mykola Lysenko <nickolay.lysenko@gmail.com>
To: linux-btrfs@vger.kernel.org
Cc: clm@fb.com, josef@toxicpanda.com, dsterba@suse.com, wqu@suse.com,
	linux-kernel@vger.kernel.org,
	Mykola Lysenko <nickolay.lysenko@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH] btrfs: raid56: fix inverted bio-list check in scrub read assembly
Date: Thu, 16 Jul 2026 10:45:11 -0700	[thread overview]
Message-ID: <20260716174511.8738-1-nickolay.lysenko@gmail.com> (raw)

Commit 5387bd958180 ("btrfs: raid56: remove sector_ptr structure")
converted the bio-list membership checks from sector pointers to
physical addresses.  The two conversions in rmw_assemble_write_bios()
kept their polarity (skip the sector when it is NOT in the bio list,
i.e. when there is nothing to write), but scrub_assemble_read_bios()
has the opposite polarity -- skip the sector when it IS in the bio
list, because then there is nothing to read -- and the conversion 
flipped it:

	-	sector = sector_in_rbio(rbio, stripe, sectornr, 1);
	-	if (sector)
	+	paddr = sector_paddr_in_rbio(rbio, stripe, sectornr, 1);
	+	if (paddr == INVALID_PADDR)
			continue;

Since a parity-scrub rbio's bio list only holds the empty completion
bio, the result is that scrub_assemble_read_bios() submits no reads at
all. finish_parity_scrub() then compares the parity it computes from
the (cached, correct) data stripes against whatever happens to be in
the freshly allocated, uninitialized stripe pages:

  - if the garbage differs from the computed parity, the sector is
    "repaired" and written back -- accidentally producing the correct
    on-disk result;
  - if a recycled page happens to still hold the old (correct) parity
    content, the sector is deemed clean, dropped from dbitmap, and the
    actually-corrupt on-disk parity is left in place -- silently, with
    every scrub error counter reading zero.

The second case is intermittent because it depends on page-allocator
recycling.  Observed with fstests btrfs/297 (raid5, 2 devices): the
corrupted P stripe intermittently stays corrupt after a scrub that
reports no errors -- roughly 1/10 runs on x86-64 KVM and up to 7/8 on
a UML build whose timing favors page reuse. Instrumentation of
verify_one_parity_step() showed the "on disk" bytes never matching the
device content (stale 0xaa / zeroed pages instead of the injected
0xff), and after this fix the injected corruption is read, detected
and repaired in every run (8/8 UML, 10/10 KVM).

Fixes: 5387bd958180 ("btrfs: raid56: remove sector_ptr structure")
CC: stable@vger.kernel.org # 7.1+
Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com>
---
Note 1: I found, reproduced and created a fix for this problem using AI
tools

Note 2: I am referencing UML (User-Mode Linux) above which is coming
from the project https://github.com/mykola-lysenko/btrfs-uml-fstests/ to
run xfstests in the UML. For reference only. 

 fs/btrfs/raid56.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 00a01b97cc..93de764d70 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -2910,11 +2910,11 @@ static int scrub_assemble_read_bios(struct btrfs_raid_bio *rbio)
 
 		/*
 		 * We want to find all the sectors missing from the rbio and
-		 * read them from the disk. If sector_paddr_in_rbio() finds a sector
-		 * in the bio list we don't need to read it off the stripe.
+		 * read them from the disk. If sector_paddrs_in_rbio() finds a
+		 * sector in the bio list we don't need to read it off the
+		 * stripe.
 		 */
-		paddrs = sector_paddrs_in_rbio(rbio, stripe, sectornr, 1);
-		if (paddrs == NULL)
+		if (sector_paddrs_in_rbio(rbio, stripe, sectornr, 1))
 			continue;
 
 		paddrs = rbio_stripe_paddrs(rbio, stripe, sectornr);
-- 
2.43.0


             reply	other threads:[~2026-07-16 17:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16 17:45 Mykola Lysenko [this message]
2026-07-16 22:29 ` [PATCH] btrfs: raid56: fix inverted bio-list check in scrub read assembly Qu Wenruo
2026-07-17  1:30   ` Mykola Lysenko
2026-07-22 14:21   ` David Sterba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260716174511.8738-1-nickolay.lysenko@gmail.com \
    --to=nickolay.lysenko@gmail.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox