ref: ca28c300245586fe5e9c7df037b3bcac377063e8
parent: 4b2ffdef1f8ca76d8a608f4c43f6a0ae13997bf7
author: Noam Preil <noam@pixelhero.dev>
date: Sat Dec 28 00:21:35 EST 2024
disk: handle map parsing failure correctly
--- a/disk.c
+++ b/disk.c
@@ -245,8 +245,8 @@
sysfatal("failed to init biobuf: %r");if(Bseek(&bio, tabbase, 0) != tabbase)
sysfatal("seek failed: %r");- parsemap(&bio, &map, &nmap);
- arenas = realloc(arenas, sizeof(VtArena) * (nmap + numarenas));
+ if(!parsemap(&bio, &map, &nmap))
+ sysfatal("failed to parse arena map of tabbase %d: %r", tabbase); arenas = realloc(arenas, sizeof(VtArena) * (nmap + numarenas));if(!arenas)
sysfatal("oom"); for(; nmap > 0; nmap -= 1){--- a/notebook
+++ b/notebook
@@ -2162,3 +2162,61 @@
4 microseconds. Holding a lock for 4 microseconds on read should be _completely fine. That's on the Reform!! I'm not worried that there will be heavy contention here. I can revisit this if proven wrong.
Performance of the simple `walk` test is unchanged, at least. This seems more than fine. committing :)
+
+Okay, read side is now working, very performant, and multi-reader. Now for the real fun: writing!
+
+There's a few things we need to do to facilitate this. The overall idea is to have the VtWrite call. A VtWrite call should, _for now_, be totally synchronous; we can optimize it later. It should use the cache in a write-through fashion, and immediately write data to disk. For testing, we also need to be able to report what writes we _would_ do _without actually doing them_, and... preferably, to test NOT ON THE LIVE SYSTEM. No hurt Jasnah.
+
+Actually, no, it can just bypass the cache! Addresses are unique. So, if we write to disk _without going through the cache_, any subsequent read will simply be uncached!
+
+... Though, no, we're hash-addressed. We need to compute the hash _and check if it already exists. I missed a step here, let's just go over the desired properties of the VtWrite call:
+
+ VtTwrite tag[1] type[1] pad[3] data[]
+ VtRwrite tag[1] score[20]
+
+Data comes in, with a type and 3 bytes of padding, which is stupid, but whatever. Type is more of a debugging tool than anything else and I dn't think I need it so we'll ignore it.
+
+So the basic call, then, is
+
+VtScore VtWrite([]byte data);
+
+We're given a block to insert. We need to:
+
+- Hash the block
+- When done, return that hash, or an error.
+- Check if we already have a copy (attempt to read it)
+ - Double check that the data matches (is not a hash collision)
+ - If hash collision, return an error.
+ - Otherwise, exit early.
+- If we don't already have a copy:
+ - Reserve space in the arena
+ - Write the block to the arena
+ - Append to the log
+ - Handle arena maintenance
+ - e.g. if finishing this arena:
+ - jump the address to the next one
+ - in the background, start the process of "sealing" the current arena.
+
+This implies, necessarily, that we need to know where the latest write finished. To do that, we need to find the active arena, and then scan it to find the correct position.
+
+Orrrr neoventi could fail to start up on here. Lovely.
+
+ADded debug logs annnd
+
+./7.neoventi: failed to parse arena map
+
+readarenatable is... OH! I reverted the devfs change, didn't I?
+
+This isn't a neoventi bug, this is an unaligned read bug in devfs/crypt!
+
+What if I make this an aligned read? Can I do that with biobuf? Maybe read 512 bytes, then Bseek back?
+
+Confirmed that it's a buffer read failure. I think it's midblock.
+
+Tabbase 270336; it's a multiple of 512, which I think is the block size.
+
+Confirmed that the patch is reverted, though. I guess I'll just apply it for now?
+
+Yep, works with the patch reapplied, hello from Jasnah/14!
+
+I'll add "make neoventi work with disks that require aligned reads" to the todo list and ignore for now :)
--
⑨