From: Zygo Blaxell Date: Thu, 4 Jul 2024 01:00:18 +0000 (-0400) Subject: btrfs: don't abort raid56 data scrub on first uncorrectable sector X-Git-Url: http://git.hungrycats.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b7e4d398c6d07af66740468fc9618b15841adf0;p=linux btrfs: don't abort raid56 data scrub on first uncorrectable sector In commit 1009254bf22a ("btrfs: scrub: use scrub_stripe to implement RAID56 P/Q scrub") a new function scrub_raid56_parity_stripe() was introduced to recalculate the parity stripe after correcting any correctable errors. When it detected unrepaired sectors in a data stripe, it returned -EIO, which propagated up the stack through scrub_stripe, scrub_chunk, and scrub_enumerate_chunks, where an error makes the scrub break out of its loop early and return the error to userspace. That was a regression: uncorrectable data blocks are one of the expected possible events that occur during a scrub, so scrub should continue until it has counted and reported all of the uncorrectable stripes in the filesystem. Perhaps more importantly, scrub should fix up any correctable errors that might exist in other stripes beyond the first uncorrectable stripe. Errors from this function are only appropriate when scrub cannot do its job at all: unable to read csums, unable to map an extent's data blocks, or out of memory. On current kernels the bug has changed shape: at the point where unrepaired sectors are detected, ret no longer holds -EIO. It holds the leftover return value of the last scrub_find_fill_first_stripe() call from the loop that populates the data stripes, which is 0, or 1 when the last data stripe of the full stripe contains no extents. When the stale 1 leaks out, the caller treats any nonzero value as an error ("if (ret) goto out;" in scrub_stripe()) and aborts the whole scrub with a meaningless positive return value; when it is 0, the scrub continues only by accident. scrub_raid56_parity_stripe() has successfully completed its task as soon as it has performed its data correction and verification steps. Return 0 explicitly at that point, regardless of the outcome of the verification. The P/Q update step is still skipped for the affected full stripe, so no garbage is written back to the devices. Detected errors are reported to the user via device stats and dmesg messages, not via the return code of the scrub ioctl. Fixes: 1009254bf22a ("btrfs: scrub: use scrub_stripe to implement RAID56 P/Q scrub") Signed-off-by: Zygo Blaxell --- diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index c09d4213ad891..e6377c7a36020 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -2248,7 +2248,7 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx, "scrub: unrepaired sectors detected, full stripe %llu data stripe %u errors %*pbl", full_stripe_start, i, stripe->nr_sectors, &error); - return ret; + return 0; } bitmap_or(&extent_bitmap, &extent_bitmap, &has_extent, stripe->nr_sectors);