Repository navigation
Conversation
This was referenced Oct 5, 2026
817bade ("read: track which ways a read has been attempted, per device") made a read-around the ec read mode, so a failed one marks ec as tried and bch2_bkey_pick_read_device() clears has_ec for the pointer. If the direct read after it fails too, no reconstruct follows and the read fails. ktest ec_read_around_fallback_reconstruct, with the slow member's block and one more data block of a 4+2 stripe bad, gets EIO where it recovered before. The two ec reads aren't the same attempt. A read-around reads k blocks over the extent's range, checked only by the extent's checksum, so it can't tell which block is bad; the full reconstruct reads every block in whole checksum granules, finds the bad one by the stripe's checksums and rebuilds with the parity. So give the read-around its own mode, BCH_READ_MODE_ec_read_around: a failed one marks that tried, the direct read follows, and a failed direct read gets the full reconstruct. Each mode is still tried at most once per device. A failure in either ec mode goes to the ec error, so self healing and damage records don't see a read-around that found a bad block elsewhere in the stripe. bch2_ec_read_optional() was that distinction; the mode says it now.
ec_read_around_blocks(), for the read path, and bch2_ec_read_around_skip(), for stripe repair and reuse, each collected the stripe's device latencies and left out the slowest blocks, by rules that could drift apart: one dropped the slowest down to k with the highest index winning a tie, the other skipped blocks of the required set while they beat the penalty, with the lowest. One function, ec_read_around_slow(), does both. The blocks to read are the k fastest that can be read now, and a block is slow when its device is slower than the slowest of those by the penalty plus the least gain worth having. A read-around of a block is picked when it's slow, and reads those k; repair and reuse rebuild a required block that's slow from the same k, where before they read every other block. The read after all of what was skipped, when the rebuild doesn't check out, is unchanged. The penalty comes from the option inside the function. Reconcile's 16x applies in bch2_ec_read_around_pick()'s own comparison, which is what decides.
bch2_rbio_retry() had its own message path for a read-around that failed and whose direct read then succeeded, with the failure counter in it: a copy of the retry message, under a different condition, and not reached when the direct read failed too and the full reconstruct recovered the data, although a read-around failed there as well. Count data_read_ec_read_around_fail in bch2_rbio_error(), where every failed read-around arrives: from bch2_ec_read_extent() when the stripe read fails, and from the extent's checksum check when a block it read was bad. That's the one place that already knows a failed read-around isn't the device's error. The retry's single message path says what was found, as it does for any recovered retry; since a read-around failure isn't a device error, the accounting after it is untouched.
bch2_rbio_retry() special-cased the read-around in two places: the trace at the top, where it counted data_read_ec_read_around instead of data_read_retry, and the error accounting, where it took whether the failure was a checksum error from the failures instead of from the original error, which is 0 for a read-around. Count the read-around where it's decided, in __bch2_read_extent(), and let the retry trace every retry as such: a forced reconstruct is already a retry by that count. The failures say what any retry recovered from, since the retry marks its original error into them first, so take the checksum verdict from them for every retry. Then the original error and the read-around bool have no users left.
A read-around of a checksummed extent reads only the extent's range of each block, so a61951b ("ec: read around a device much slower than the rest of its stripe") gave bch2_ec_stripe_buf_init() a flags argument, and since eb862ca ("ec: a read-around checks for stripe buffer room when it's picked") the enum has one flag, with a wrapper for the other callers, all to set the buffer's unaligned field. The caller sets up the buffer's key before init; have it set unaligned the same way, and init test the field. The init signature is back to what it was.
bch2_ec_read_extent() left the block out of the read only when a direct read of it had failed, and read it with the rest of the stripe otherwise: for a device that went offline before the read was picked, and for force_reconstruct_read. The former fails the block at once either way; the latter read every block, found nothing bad, and returned the block's own data without going through the parity. Rebuild the block from the others in every mode but a read-around, which chooses its own blocks, and keep the read after all for when they can't. That drops the check of what was tried, and force_reconstruct_read reconstructs.
… once ec_read_around_slow() left out devices with a failure recorded against them, but none of its callers can have one: bch2_ec_read_around_pick() only reads around in the first pass of a read, where there are no failures yet, and in the retry that carries its decision, whose failures are empty until that read-around runs. Repair and reuse pass NULL. Drop the argument, and with it bch2_ec_read_extent()'s need for the failures: it takes the message buffer again, as before the series. The retry carrying a decision ran the latency checks again before honouring it. Handle the retry first: it reads around if it was decided and hasn't been tried, and otherwise doesn't, so the rest of the function is the first pass's decision, unnested. The selection takes the penalty and returns the cost, which the pick compared against after computing it from the latency the selection returned; reconcile's 16x now selects the slow blocks too, as it decides. has_ec is cleared with the other per-failure fields, where the line it replaced was.
The read path, stripe reuse and stripe repair each had a loop that read the blocks it wanted and marked the rest skipped, and each had a copy of the read after all: clear the post-recovery errors, clear the block's error, issue the read. bch2_stripe_buf_read() reads the blocks in a mask and marks the rest skipped; the blocks a first round doesn't need are erasures too, which only matters if a validate runs before they're read, and none does. bch2_ec_block_io() clears the block's error at issue, as reading it again replaces the result, and validate clears the post-recovery errors it's about to recompute. The read after all is then just the reads. stripe_repair_read() reads the skipped blocks on the path that reads the rest when a required block is bad, and the caller validates the whole stripe as it does then, instead of the read checking the required blocks itself.
A read-around is decided in the first pass and done by the retry it's
punted to, and 8ef50cee8d6c ("ec: promote read-around results") had the
first pass clear BCH_READ_may_promote so as not to allocate a promote
the punt frees, then put it back on the rbio for the retry to find.
Have promote_alloc() return nothing for a read that will be punted,
which a forced reconstruct is too: the flags carry across the punt
untouched. The check for a retry with nothing to repair is gone: a
retry only gets here with a failure to heal or with may_promote, which
the read-around retry keeps. __promote_alloc() gates on a failure the
same way, so promote_alloc() passes the failures through.
Comments that restated the code or argued for it at commit-message length are cut to the reason, where the code doesn't give it. The option help says the same in fewer words. The latency helper is an atomic64_read; have_io_error() is a darray_find_p(); the pick checks that the pointer is read directly, and sets the flag that carries its decision to the retry, so the call site is the call, and the trace names the stripe.
A read-around of a checksummed extent checks the data it rebuilt against the extent's checksum. When that fails, one of the blocks it read is bad, and nothing says which. Until scrub gets to the stripe, every read-around of it fails the same way and falls back to the slow device. A reconstruct that finds a block failing the stripe's checksums names it, and leaves it bad too. Queue those blocks for a scrub: after a read-around's checksum error, where it's counted as failed, every block but the one read around; after a reconstruct, the blocks it named. A worker on write_ref_wq walks each block's backpointer with verify_stripes, so scrub_stripe_block() checks it and repairs the stripe as scrub does. The queue holds 16 stripes. A stripe scrubbed in the last 60 s isn't queued again: a repair that narrows the stripe leaves the bad block in place until reconcile has evacuated it, and reads keep failing until then. Each entry holds a write ref, and the worker drops what's left when the filesystem goes read-only.
aderk
force-pushed
the
ec-read-around-scrub
branch
from
October 7, 2026 00:53
f7f866a to
841f113
Compare
This was referenced Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased onto #1116; one commit on top, +175/-2.
A read-around whose result fails the extent checksum read a bad block
but can't say which, so every later read-around of that stripe fails and
falls back to the slow device until scrub gets there. This queues a scrub
of the stripe from bch2_rbio_error(), where the failure is counted, and
from bch2_ec_read_extent() for blocks a reconstruct names. The worker
enters scrub_stripe_block() through the block's backpointers, so the
check and repair are scrub's own.
This is a bit janky: the queue is in memory, 16 slots, and a stripe
scrubbed in the last 60 s isn't queued again, since a repair that
narrows the stripe leaves the bad block until reconcile evacuates it. A
full or lost queue costs only the slow fallback until a scheduled scrub.
The worker runs at once on write_ref_wq, outside reconcile; its moves are
idle IO, but the stripe reads and parity writes go through
bch2_ec_block_io() at the default priority. Each entry holds a write
ref, and each scrubbed block does a write buffer flush.
Possible follow-ups: a persistent "needs scrub" marker on the stripe,
run by reconcile, which survives a remount and drops the queue, the
timer and the write refs; or, smaller, dedupe on the stripe until its
repair finishes, in place of the 60 s timer.
Tests: ktest#137. Three stripes with bad parity and data on
a slow member: after one read no bad block is left, and the second
read's 78 read-arounds all succeed (39 of 39 with data_checksum=none).
The repair reads the bad block, so the scrub tests and
ec_read_around_fallback now expect stripe_read_csum_err.