Fix blk_meta_t/BAM accounting reconciliation flagged in FABRIC-2.md §I.2

FIRSTTOUCH ownership (blk_meta_t's BLK_FLAG_CLAIMED/owner_fp) and the
generic block-allocation bitmap (BAM, blk_bam_entry_t) were two parallel,
unreconciled accounting systems: blk_firsttouch_claim() never touched the
BAM, and blk_allocate()/devblock_is_free() never checked BLK_FLAG_CLAIMED.
A FIRSTTOUCH claim could be silently overwritten by a later blk_allocate()
call, or could itself steal a devblock already in ordinary use via
BLOCK/UPDATE.

- devblock_is_free() (shared by blk_firsttouch_claim() and
  blk_migration_idle_check()) now also checks the BAM entries of all
  BLK_PACK_RATIO member LBNs, not just blk_meta_t.
- New devblock_claimed_by_lbn() helper wired into blk_allocate()'s
  free-scan, so it skips any LBN whose devblock is BLK_FLAG_CLAIMED.
- blk_firsttouch_claim() now marks the BAM allocated for all 3 member
  LBNs of each devblock it claims, which also fixes vol_meta.free_blocks
  never decrementing for FIRSTTOUCH claims.
- blk_meta_relocate_devblock() traced and confirmed NOT part of the bug —
  it already keeps BAM in sync via blk_subsys_relocate_block()'s own
  blk_mark_free()/blk_update() calls.

Both boundary cases (the reserved/user LBN split at a slot's start_lbn,
and BAM-array bounds) are guarded explicitly.

Verified with the mandatory 3-arch QEMU acceptance (identical dictionary
hashes, clean BYE) plus a live logic test: a temporary TEST-BAM-RECON
word, run once via SK_CMD and fully reverted, confirmed on running code
that ordinary allocation and a FIRSTTOUCH claim land on disjoint LBN
ranges in both directions. FABRIC-2.md §I.2 updated in place with the
closure note, per this project's documentation discipline.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EjXFo7mPXjUMjfJeuUUz4
This commit is contained in:
Robert Allan James
2026-09-05 15:57:03 -04:00
co-authored by Claude Sonnet 5
parent 04cc17920e
commit aa33aedca8
3 changed files with 90 additions and 6 deletions
+24
View File
@@ -4441,6 +4441,30 @@ stale the way the original carry-forwards did.
Not fixed in this pass; flagged precisely so it isn't rediscovered as a mystery later.
*(Same location as above.)*
**CLOSED 2026-09-05.** Traced against current code before writing anything, per this
document's own discipline: `blk_meta_relocate_devblock()` turned out **not** to be part of
the gap after all — it calls `blk_subsys_relocate_block()` per member block, which already
calls `blk_mark_free()` on the home LBN and `blk_update()` (which sets the BAM bit) on the
target, so relocation was already keeping BAM in sync. The real gap was entirely in
`blk_firsttouch_claim()`'s side (never touched BAM at all) and in the reverse direction
(`devblock_is_free()`/`blk_allocate()` never checked `BLK_FLAG_CLAIMED`), confirmed by a
full-documentation audit ([[project_fabric012_full_audit_20260905]] in memory) before
fixing. **Fix** (`block_subsystem.c`): `devblock_is_free()` (shared by
`blk_firsttouch_claim()` and `blk_migration_idle_check()`) now also checks the BAM entries of
all `BLK_PACK_RATIO` member LBNs, not just `blk_meta_t`; a new `devblock_claimed_by_lbn()`
helper is wired into `blk_allocate()`'s free-scan so it skips any LBN whose devblock is
`BLK_FLAG_CLAIMED`; `blk_firsttouch_claim()` now calls `blk_mark_allocated()` on all 3 member
LBNs of each devblock it claims, which also fixes a quiet side effect where
`vol_meta.free_blocks` never decremented for FIRSTTOUCH claims. Both boundary edge cases
(the reserved/user LBN split at a slot's own `start_lbn`, and BAM-array bounds) are guarded
explicitly rather than assumed safe. **Verified, not just compiled clean**: full 3-arch QEMU
acceptance (identical dictionary hashes, clean `BYE`, no regression) plus a live logic test —
a temporary `TEST-BAM-RECON` word (written, run once via `SK_CMD`, then fully reverted,
per this project's own probe discipline) ordinary-allocated 4 LBNs, `FIRSTTOUCH`-claimed 2
devblocks, and confirmed on real running code that neither direction of the gap reproduces:
the ordinary allocations and the claim landed on disjoint LBN ranges, and `blk_allocate()`
never returned an LBN from the claimed range afterward. `PASS`, no `FAIL` lines.
### I.3 — Milestone 5 remainder (BINDSTEP) — CLOSED 2026-09-04
- [x] **`EJECT` / detach behavior.** Scoped 2026-08-27 (§F.10): flush via
Binary file not shown.
+66 -6
View File
@@ -1078,6 +1078,34 @@ int blk_mark_free(uint32_t block_num) {
return BLK_OK;
}
/* blk_meta_t/BAM reconciliation, 2026-09-05 (closes the other half of the
* gap §I.2 flagged): blk_allocate()'s BAM scan below must not hand out an
* LBN whose devblock a FIRSTTOUCH claim already owns. BMAPFMT ownership
* lives only on the devblock's pack_offset==0 "representative" LBN's own
* blk_meta_t (blk_firsttouch_claim() never stamps the other two members),
* so an arbitrary candidate lbn must walk back to its representative
* before checking BLK_FLAG_CLAIMED.
*
* Reserved/user boundary case: lbn_to_slot_pbn() adds the fixed
* BLK_DISK_SYS_RESERVED (32) offset before computing pack_offset, so a
* slot's own start_lbn is not guaranteed to land on pack_offset==0 --
* walking back by pack_offset can land before slot->start_lbn entirely
* (the representative sits in the reserved system region, which has no
* LBN and thus no blk_meta_t at all). FIRSTTOUCH's own scan already
* skips any lbn where slot_pbn_pack_offset(rel_pbn) != 0, so it can never
* reach that representative to claim it -- such an lbn is guaranteed
* unclaimed; returning 0 here without calling blk_get_meta() on an
* address outside the slot is both correct and the safe choice. */
static int devblock_claimed_by_lbn(blk_dev_slot_t *slot, uint32_t lbn) {
uint32_t rel_pbn = lbn_to_slot_pbn(slot, lbn);
uint32_t pack = slot_pbn_pack_offset(rel_pbn);
uint32_t offset = lbn - slot->start_lbn;
if (offset < pack) return 0;
blk_meta_t meta;
if (blk_get_meta(lbn - pack, &meta) != BLK_OK) return 0;
return (meta.flags & BLK_FLAG_CLAIMED) != 0;
}
int blk_allocate(uint32_t *block_num) {
if (!block_num) return BLK_EINVAL;
if (!g.initialized) return BLK_ENODEV;
@@ -1094,7 +1122,7 @@ int blk_allocate(uint32_t *block_num) {
for (uint32_t i = 0; i < limit; i++) {
uint32_t k = (hint + i) % limit;
if (!s->bam[k].allocated) {
if (!s->bam[k].allocated && !devblock_claimed_by_lbn(s, s->start_lbn + k)) {
s->bam[k].allocated = 1;
s->bam_dirty = 1;
if (s->vol_meta.free_blocks) s->vol_meta.free_blocks--;
@@ -1360,10 +1388,31 @@ int blk_flags_set(uint32_t block_num, uint64_t flags) {
* misclassify a claimed-but-not-yet-fp-stamped or fp-stamped-but-
* evicted devblock, neither of which should exist given owner_fp is
* always stamped/cleared alongside the flag below, but checking both is
* one extra memcmp for a real safety margin, not paranoia without cost. */
static int devblock_is_free(const blk_meta_t *m) {
* one extra memcmp for a real safety margin, not paranoia without cost.
*
* blk_meta_t/BAM reconciliation, 2026-09-05 (closes the gap §I.2 itself
* flagged, not fixed): a devblock can be "unowned" by blk_meta_t and
* still be in active use via the generic BAM (ordinary BLOCK/UPDATE
* traffic, unrelated to BMAPFMT ownership) -- claiming it anyway would
* silently corrupt that content. Checks all BLK_PACK_RATIO member LBNs,
* not just rep_lbn itself, since BAM is tracked per-1KiB-block while
* BMAPFMT ownership is tracked per-devblock. rep_lbn is always the
* pack_offset==0 member (both call sites already filter to that), so
* rep_lbn+1/+2 are its devblock siblings. Bounds-checked against
* slot->user_blocks rather than assumed divisible by BLK_PACK_RATIO --
* the very first user LBN in a slot can belong to a devblock whose other
* members fall below start_lbn entirely (see the reserved/user boundary
* note in blk_allocate()'s new claimed-check below). */
static int devblock_is_free(blk_dev_slot_t *slot, uint32_t rep_lbn, const blk_meta_t *m) {
static const uint8_t zero_fp[8] = {0};
return !(m->flags & BLK_FLAG_CLAIMED) && memcmp(m->owner_fp, zero_fp, 8) == 0;
if ((m->flags & BLK_FLAG_CLAIMED) || memcmp(m->owner_fp, zero_fp, 8) != 0)
return 0;
for (uint32_t i = 0; i < BLK_PACK_RATIO; i++) {
uint32_t off = (rep_lbn - slot->start_lbn) + i;
if (off >= slot->user_blocks) return 0;
if (slot->bam[off].allocated) return 0;
}
return 1;
}
/* Sane upper bound on one claim, independent of how large count is --
@@ -1398,7 +1447,7 @@ int blk_firsttouch_claim(const uint8_t owner_fp[8], uint32_t count, uint32_t *ou
blk_meta_t meta;
if (blk_get_meta(lbn, &meta) != BLK_OK) continue;
if (devblock_is_free(&meta)) found[nfound++] = lbn;
if (devblock_is_free(slot, lbn, &meta)) found[nfound++] = lbn;
}
if (nfound < count) return BLK_ENOSPC; /* fail outright, no partial claim (§F.11 decision 3) */
@@ -1415,6 +1464,17 @@ int blk_firsttouch_claim(const uint8_t owner_fp[8], uint32_t count, uint32_t *ou
meta.next_block = (i + 1 == nfound) ? 0 : found[i + 1];
meta.chain_length = nfound;
if (blk_set_meta(found[i], &meta) != BLK_OK) return BLK_EIO;
/* blk_meta_t/BAM reconciliation, 2026-09-05: blk_set_meta() above
* only touches blk_meta_t -- mark the BAM allocated too, for all
* BLK_PACK_RATIO member LBNs of this devblock (not just found[i]
* itself), so blk_allocate() can never hand this devblock's
* content out from under this claim, and so vol_meta.free_blocks
* (blk_mark_allocated()'s own bookkeeping) correctly reflects the
* claim instead of silently overstating free space. */
for (uint32_t j = 0; j < BLK_PACK_RATIO; j++) {
(void) blk_mark_allocated(found[i] + j);
}
}
*out_chain_head = found[0];
@@ -1504,7 +1564,7 @@ void blk_migration_idle_check(void) {
hot_lbn = lbn;
have_hot = 1;
}
if (!have_free && devblock_is_free(&meta)) {
if (!have_free && devblock_is_free(slot, lbn, &meta)) {
free_lbn = lbn;
have_free = 1;
}