Artemis Milestone 2h: hot-detach -- 2h complete
blk_subsys_detach_device() (block_subsystem.c) walks the device chain, refuses removal of anything but the current tail (a mid-chain removal would corrupt every later slot's start_lbn -- this architecture's own doc already argues USB stays last specifically to avoid that), unlinks, shrinks total_user_lbn, closes and frees the slot. Discards rather than flushes dirty state -- the device is physically gone by the time this runs (PORTSC disconnect only). Trigger wiring mirrors the attach path: bot_msc_attached (set only once attach actually succeeds) gates a new bot_msc_detach_pending flag set at PORTSC disconnect (not Disable Slot completion, which is conditionally skipped and would miss concurrent connect/disconnect pairs), consumed in sk_repl_idle(). Advisor flagged the real hazard ahead of time: block_words.c's VM block window (blk_vm_lbn[]/blk_vm_cbuf[]) can go stale across a detach then a same-LBN re-attach, and suggested a pointer-identity re-check in blk_vm_load() as a minimal fix. That fix was implemented, then directly falsified by its own designed-for-this test: attach a blank device, read a block (populating the cache), detach, re-attach a device with distinct content at the identical LBN, read again -- served stale content from the first device. Root cause, confirmed live: glibc's allocator hands free(slot) straight back to the very next same-size calloc(), so the "fresh" and stale pointers were bitwise identical despite being two different devices. Fixed properly with a monotonic blk_subsys_epoch() counter (bumped on every attach/detach) checked by a new blk_vm_check_epoch() helper at the one choke point (blk_vm_find(), plus blk_vm_flush_all() which reads the same arrays directly) that covers every path touching the window cache -- unfooled by address reuse. Verified live with a new disk/usb-thumbdrive-test2.img fixture (distinct content from the existing blank test image): attach A, read (cache hit populated), detach, re-attach B at the same LBN, read again -- correctly ran a fresh device read and returned B's real content, not A's stale cached zeros. The failing pointer-comparison attempt's own capture log kept as evidence, not deleted. All three architectures re-verified clean. FABRIC-2.md Section X 2h marked complete -- enumeration through hot-detach all live and verified; only WRITE(10) (2g's own still-open item) remains unimplemented in the driver, not blocking anything here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CXjAPTEKrgY2Mrk25KoLDn
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
3b085dd875
commit
af267a52a6
@@ -152,6 +152,19 @@ static struct {
|
||||
uint64_t total_user_lbn; /* total user-visible LBNs across RAM + all device slots */
|
||||
blk_dev_slot_t *head; /* linked list of device slots */
|
||||
|
||||
/* Bumped on every attach/detach (Milestone 2h). Freeing a slot then
|
||||
* immediately allocating a new one for a same-LBN re-attach can hand
|
||||
* back the *same* heap address (confirmed live: glibc's allocator does
|
||||
* exactly this for a free() followed immediately by a same-size
|
||||
* calloc(), with nothing else allocated in between) -- so a raw
|
||||
* pointer comparison against a cached blk_get_buffer() result cannot
|
||||
* reliably detect "this LBN's device changed underneath a caller
|
||||
* holding a stale cached pointer." A monotonic epoch can't be fooled
|
||||
* by address reuse the way a pointer comparison was found to be (see
|
||||
* block_words.c's blk_vm_find(), the one place outside this file that
|
||||
* caches a blk_get_buffer() result across calls). */
|
||||
uint64_t epoch;
|
||||
|
||||
int initialized;
|
||||
} g = {0};
|
||||
|
||||
@@ -533,6 +546,7 @@ int blk_subsys_add_raw_device(uint8_t *buf, uint32_t nblocks) {
|
||||
|
||||
chain_append(slot);
|
||||
g.total_user_lbn += nblocks;
|
||||
g.epoch++;
|
||||
|
||||
log_message(LOG_INFO, "blk: raw device LBN %u..%u (%u blocks)",
|
||||
slot->start_lbn, slot->start_lbn + nblocks - 1, nblocks);
|
||||
@@ -562,6 +576,7 @@ int blk_subsys_attach_device(struct blkio_dev *dev) {
|
||||
|
||||
chain_append(slot);
|
||||
g.total_user_lbn += slot->user_blocks;
|
||||
g.epoch++;
|
||||
|
||||
log_message(LOG_INFO,
|
||||
"blk: disk '%s' v2 LBN %u..%u (%u user blocks); "
|
||||
@@ -576,6 +591,55 @@ int blk_subsys_attach_device(struct blkio_dev *dev) {
|
||||
return BLK_OK;
|
||||
}
|
||||
|
||||
/* Milestone 2h hot-detach. Deliberately refuses anything but the current
|
||||
* chain tail: block_subsystem.c's own architecture doc (top of this file)
|
||||
* has USB/future devices as the *last* link specifically so a removal
|
||||
* never has to renumber any other slot's start_lbn -- a mid-chain removal
|
||||
* would corrupt every later slot's LBN range, so this is refused outright
|
||||
* rather than attempted.
|
||||
*
|
||||
* Deliberately discards rather than flushes any dirty cache/BAM/vol_meta
|
||||
* state: the device is physically gone by the time this runs (called only
|
||||
* after a real PORTSC disconnect), so a flush attempt cannot succeed --
|
||||
* pretending to try would just call blkio_write() against a vanished
|
||||
* device for no benefit. Revisit if a future graceful-unmount path (as
|
||||
* opposed to a surprise removal) wants a best-effort flush first; today
|
||||
* every removal this driver can observe is a surprise removal.
|
||||
*/
|
||||
int blk_subsys_detach_device(struct blkio_dev *dev) {
|
||||
if (!g.initialized) return BLK_ENODEV;
|
||||
if (!dev) return BLK_EINVAL;
|
||||
|
||||
blk_dev_slot_t *prev = NULL;
|
||||
blk_dev_slot_t *s = g.head;
|
||||
while (s && s->dev != dev) { prev = s; s = s->next; }
|
||||
if (!s) return BLK_ENODEV;
|
||||
|
||||
if (s->next) {
|
||||
log_message(LOG_WARN, "blk: refusing detach of non-tail device (LBN %u..%u)",
|
||||
s->start_lbn, s->start_lbn + s->user_blocks - 1);
|
||||
return BLK_EINVAL;
|
||||
}
|
||||
|
||||
log_message(LOG_INFO, "blk: detaching disk '%s' LBN %u..%u (%u user blocks)",
|
||||
s->vol_meta.label, s->start_lbn, s->start_lbn + s->user_blocks - 1,
|
||||
s->user_blocks);
|
||||
|
||||
if (prev) prev->next = NULL; else g.head = NULL;
|
||||
g.total_user_lbn -= s->user_blocks;
|
||||
g.epoch++;
|
||||
|
||||
blkio_close(dev);
|
||||
if (s->bam) free(s->bam);
|
||||
free(s);
|
||||
|
||||
return BLK_OK;
|
||||
}
|
||||
|
||||
uint64_t blk_subsys_epoch(void) {
|
||||
return g.epoch;
|
||||
}
|
||||
|
||||
int blk_subsys_shutdown(void) {
|
||||
if (!g.initialized) return BLK_OK;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user