storage: fix VFS corruption, ISO9660 hang, OOM; add notepad, file picker, fdtable, GPT, async I/O
VFS fixes: - GetDirectoryListing(otVDIRECTORY) now returns a heap-allocated snapshot instead of a raw live-tree pointer; FreeDirectoryListing no longer corrupts the VFS tree (fixes page fault on repeated navigation) - volumeGetDirectories: guard LL_Size > 0 before loop to prevent uint32 underflow (0-1 = 0xFFFFFFFF) causing 4B kalloc iterations -> OOM - Free entry^.fileName and dirList in all branches (memory leak fix) iso9660: - equalsIgnoreCase: guard la=0 before loop (same uint32 underflow bug) - readDirectoryEntries/resolvePath: add 65536/256-iter safety caps storagemanager: - Strip debug syslog calls that were allocating 6+ strings per I/O (hundreds of I/Os x 6 allocs exhausted the heap) Process manager: - Per-process FDTable and Cwd initialised on create, freed on reap - proc_await: use pointer-indirection spin per lesson #6 (FPC caches non-volatile reads; raw while flag=0 returns immediately) - graphicsrefresh: move LVGL handler from timer ISR (IF=0) to a dedicated process so LVGL callbacks can call blocking I/O New units: - fdtable.pas: per-process file descriptor table (fd_alloc/get/close) - iorequest.pas: heap allocation helpers for TIORequest - gpt.pas: GPT header/entry read + CRC validation - filepicker.pas: reusable OS-level open/save file picker dialog - notepad.pas: GUI text editor with selection, indent, save-as Shell commands (diskcmd, partcmd, volcmd, diskutil): - Route output to stdout_buf instead of syslog (lesson #4) - diskutil: async add/remove/format partition; cache-based device display Add Apache 2.0 license headers to storagemanager.pas, iso9660.pas, fat32.pas, storagetest.pas Lessons learnt: #9 VFS listing must return snapshot (not live pointer)
This commit is contained in:
+70
-48
File diff suppressed because it is too large
Load Diff
Binary file not shown.
@@ -0,0 +1,26 @@
|
||||
# Lessons Learnt
|
||||
|
||||
## 1. File Pickers Must Be Proper Reusable OS Components
|
||||
Inline pickers hacked into app code are broken and not reusable. Build file pickers as a standalone unit (`filepicker.pas`) that any program can call via a callback.
|
||||
|
||||
## 2. All Errors Need Visible In-App Feedback
|
||||
Never use `syslog.logln` as the only error output. Always show the user a dialog, msgbox, or status bar message. Syslog is supplemental only.
|
||||
|
||||
## 4. Shell Command Output Goes to stdout_buf, Not syslog
|
||||
Use `stdio.bufWriteStr(stdout_buf, ...)` for all shell command output. `syslog` is for kernel diagnostics only.
|
||||
|
||||
## 5. Unit Test Convention
|
||||
New subsystem units get a `procedure UnitTest` (called at boot from kernel.pas) and a `procedure init` that registers a CLI command so tests can be re-run at runtime. Tests use a nested `Assert(condition, name)` that increments `passed`/`failed` and logs failures via `syslog.logln`. At-boot tests must not do disk I/O — only test in-memory/virtual structures.
|
||||
|
||||
## 6. ISR-Visible Spin-Wait Flags Must Use Pointer Dereferencing
|
||||
FPC has no `volatile` keyword. Any `while flag = 0 do asm hlt end` loop where `flag` is set by an ISR will hang forever because FPC hoists the load into a register before the loop and never re-reads memory. Always access the flag through a pointer: `while puint32(@flag)^ = 0 do proc_yield()`. This applies everywhere in the kernel that spins waiting for ISR-written state (e.g. `submit_io_wait`, `proc_await`-style loops, DMA done flags).
|
||||
|
||||
## 7. Never Spawn a Process Per I/O Operation
|
||||
Spawning a new kernel process for each FAT32 read/write/dir operation is pure overhead: process creation (kalloc 8 KB stack, fake IRET frame, DList insertion), an extra context switch, and a double-wait (VFS spins on the worker, worker spins on AHCI). Instead, submit the I/O directly and park the calling process via `psAwaiting` until the AHCI ISR fires the completion callback — zero extra processes, zero wasted scheduler ticks.
|
||||
|
||||
## 8. Use psAwaiting to Park a Process Pending Async I/O (not proc_yield spin)
|
||||
The correct way to block a process on an async I/O result is: (1) set up a completion callback that sets `State := psReady` then `Done := 1`, (2) atomically check if the operation is already done (under CLI) and set `CurrentProcess^.State := psAwaiting` if not, (3) spin on `Done` via `puint32(@wait.Done)^` per Lesson #6 with `asm hlt end` inside the loop. The process is removed from the runnable set while hardware works. **Do NOT call `proc_await` from `submit_io_wait`** — `proc_await` spins on `CurrentProcess^.State = psAwaiting` without pointer indirection and FPC caches the value in a register, causing it to return immediately before the I/O completes. The calling code then reads garbage from the DMA buffer, causing invalid pointer dereferences and page faults.
|
||||
|
||||
## 9. VFS GetDirectoryListing Must Return a Snapshot, Not a Live Tree Reference
|
||||
`GetDirectoryListing` for `otVDIRECTORY` nodes previously returned `PHashMap(Obj^.Reference)` — a direct pointer into the live VFS tree. When callers called `FreeDirectoryListing` on the returned map, it freed the live VFSObjects and their name strings, destroying the VFS tree. Any subsequent `GetObjectFromPath` call would then dereference the trashed memory and page fault. **Fix**: always return a heap-allocated snapshot copy of the directory (new `PHashMap`, new `PVFSObject` entries with `stringCopy`'d keys). `FreeDirectoryListing` can then safely own and free the snapshot without touching the live tree. The rule: any function that returns a `PHashMap` to a caller who will call `FreeDirectoryListing` must allocate a fresh map — never return a raw interior pointer to a live data structure.
|
||||
|
||||
-603
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -23,15 +23,15 @@ unit flatfs;
|
||||
interface
|
||||
|
||||
uses
|
||||
tracer,
|
||||
strings,
|
||||
filesystemmanager,
|
||||
lists,
|
||||
syslog,
|
||||
lmemorymanager,
|
||||
stdio,
|
||||
storagemanager,
|
||||
storagetypes,
|
||||
lmemorymanager,
|
||||
strings,
|
||||
syslog,
|
||||
tracer,
|
||||
util,
|
||||
volumemanager;
|
||||
|
||||
@@ -592,15 +592,18 @@ end;
|
||||
procedure detect_volumes(disk : PStorage_Device);
|
||||
var
|
||||
buffer : puint32;
|
||||
bufSize : uint32;
|
||||
volume : PStorage_volume;
|
||||
begin
|
||||
push_trace('flatfs.detectVolumes()');
|
||||
|
||||
buffer := puint32(kalloc(512));
|
||||
memset(uint32(buffer), 0, 512);
|
||||
bufSize := disk^.sectorSize;
|
||||
if bufSize < 512 then bufSize := 512;
|
||||
buffer := puint32(kalloc(bufSize));
|
||||
memset(uint32(buffer), 0, bufSize);
|
||||
|
||||
if (disk^.readCallback = nil) and (disk^.readCallbackAsync = nil) then begin
|
||||
syslog.writestringln('FlatFS: detect_volumes: device has no read callback.');
|
||||
if disk^.dispatchRead = nil then begin
|
||||
syslog.writestringln('FlatFS: detect_volumes: device has no read dispatch.');
|
||||
kfree(buffer);
|
||||
exit;
|
||||
end;
|
||||
@@ -759,14 +762,17 @@ end;
|
||||
|
||||
function identify_volume(volume : PStorage_Volume) : boolean;
|
||||
var
|
||||
buffer : puint32;
|
||||
buffer : puint32;
|
||||
bufSize : uint32;
|
||||
begin
|
||||
push_trace('flatfs.identify_volume');
|
||||
identify_volume := false;
|
||||
if (volume^.device^.readCallback = nil) and (volume^.device^.readCallbackAsync = nil) then exit;
|
||||
if volume^.device^.dispatchRead = nil then exit;
|
||||
|
||||
buffer := puint32(kalloc(512));
|
||||
memset(uint32(buffer), 0, 512);
|
||||
bufSize := volume^.device^.sectorSize;
|
||||
if bufSize < 512 then bufSize := 512;
|
||||
buffer := puint32(kalloc(bufSize));
|
||||
memset(uint32(buffer), 0, bufSize);
|
||||
|
||||
storagemanager.storage_read(volume^.device, volume^.sectorStart, 1, buffer);
|
||||
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -38,12 +38,10 @@ begin
|
||||
if size = 0 then exit;
|
||||
|
||||
{ Round up to a whole number of sectors so DMA / block I/O is safe }
|
||||
sectorSize := volume^.device^.sectorSize; //TODO need fileystsem function to get thing size, like cluster size, for file-level IO buffer allocation. For now we just assume sector size.
|
||||
sectorSize := volume^.device^.sectorSize;
|
||||
if sectorSize = 0 then
|
||||
sectorSize := 512;
|
||||
|
||||
sectorSize := 512*4; { safe default } //TODO
|
||||
|
||||
alignedSize := ((size + sectorSize - 1) div sectorSize) * sectorSize;
|
||||
|
||||
buf := puint32(kalloc(alignedSize));
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
{
|
||||
Driver->Storage->IORequest - I/O request allocation and lifecycle helpers.
|
||||
|
||||
Provides heap allocation/free for TIORequest records used by the
|
||||
submit_io / complete_io path in storagemanager.pas.
|
||||
|
||||
@author(Aaron Hance <[email protected]>)
|
||||
}
|
||||
unit iorequest;
|
||||
|
||||
interface
|
||||
|
||||
uses
|
||||
lmemorymanager,
|
||||
storagetypes;
|
||||
|
||||
{ Allocate a new TIORequest on the heap and initialise its core fields.
|
||||
Caller, State, Error and ByteCount are zeroed (caller fills as needed). }
|
||||
function ioreq_alloc(reqType : TIORequestType;
|
||||
device : PStorage_Device;
|
||||
lba : uint32;
|
||||
sectors : uint32;
|
||||
buf : pointer) : PIORequest;
|
||||
|
||||
{ Free a heap-allocated TIORequest. Safe to call with nil. }
|
||||
procedure ioreq_free(req : PIORequest);
|
||||
|
||||
{ Copy an existing TIORequest into a new heap allocation (deep copy of the
|
||||
record, NOT the buffer it points to). Used by dispatch_next to create an
|
||||
ISR-safe copy from the CFIFO value-element. }
|
||||
function ioreq_copy(src : PIORequest) : PIORequest;
|
||||
|
||||
implementation
|
||||
|
||||
function ioreq_alloc(reqType : TIORequestType;
|
||||
device : PStorage_Device;
|
||||
lba : uint32;
|
||||
sectors : uint32;
|
||||
buf : pointer) : PIORequest;
|
||||
var
|
||||
req : PIORequest;
|
||||
begin
|
||||
req := PIORequest(kalloc(SizeOf(TIORequest)));
|
||||
if req = nil then begin
|
||||
ioreq_alloc := nil;
|
||||
exit;
|
||||
end;
|
||||
req^.RequestType := reqType;
|
||||
req^.State := iosPending;
|
||||
req^.Device := device;
|
||||
req^.LBA := lba;
|
||||
req^.SectorCount := sectors;
|
||||
req^.Buffer := buf;
|
||||
req^.ByteCount := 0;
|
||||
req^.Error := eNone;
|
||||
req^.Caller := nil;
|
||||
req^.UserData := nil;
|
||||
req^.Callback := nil;
|
||||
req^.CallbackData := nil;
|
||||
ioreq_alloc := req;
|
||||
end;
|
||||
|
||||
procedure ioreq_free(req : PIORequest);
|
||||
begin
|
||||
if req <> nil then
|
||||
kfree(void(req));
|
||||
end;
|
||||
|
||||
function ioreq_copy(src : PIORequest) : PIORequest;
|
||||
var
|
||||
dst : PIORequest;
|
||||
begin
|
||||
dst := PIORequest(kalloc(SizeOf(TIORequest)));
|
||||
if dst = nil then begin
|
||||
ioreq_copy := nil;
|
||||
exit;
|
||||
end;
|
||||
dst^.RequestType := src^.RequestType;
|
||||
dst^.State := src^.State;
|
||||
dst^.Device := src^.Device;
|
||||
dst^.LBA := src^.LBA;
|
||||
dst^.SectorCount := src^.SectorCount;
|
||||
dst^.Buffer := src^.Buffer;
|
||||
dst^.ByteCount := src^.ByteCount;
|
||||
dst^.Error := src^.Error;
|
||||
dst^.Caller := src^.Caller;
|
||||
dst^.UserData := src^.UserData;
|
||||
dst^.Callback := src^.Callback;
|
||||
dst^.CallbackData := src^.CallbackData;
|
||||
ioreq_copy := dst;
|
||||
end;
|
||||
|
||||
end.
|
||||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
+638
-234
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -5,6 +5,7 @@
|
||||
bindings used by processmanager.pas and contextswitcher.pas.
|
||||
|
||||
@author(Kieron Morris <kjm@kieronmorris.me>)
|
||||
@author(Aaron Hance <ah@aaronhance.me>)
|
||||
}
|
||||
unit proctypes;
|
||||
|
||||
@@ -105,6 +106,12 @@ type
|
||||
{ Resource bindings (PDList of TResourceBinding) }
|
||||
Resources : void;
|
||||
|
||||
{ Per-process file descriptor table (PFDTable from fdtable.pas) }
|
||||
FDTable : void;
|
||||
|
||||
{ Per-process working directory — heap-allocated, defaults to '/' }
|
||||
Cwd : pchar;
|
||||
|
||||
{ User-defined state }
|
||||
Local : void;
|
||||
end;
|
||||
|
||||
+37
-10
@@ -16,6 +16,7 @@
|
||||
Include->Util - Data Manipulation Utlities.
|
||||
|
||||
@author(Kieron Morris <kjm@kieronmorris.me>)
|
||||
@author(Aaron Hance <ah@aaronhance.me>)
|
||||
}
|
||||
unit util;
|
||||
|
||||
@@ -24,7 +25,8 @@ unit util;
|
||||
interface
|
||||
|
||||
uses
|
||||
bios_data_area, tracer;
|
||||
bios_data_area,
|
||||
tracer;
|
||||
|
||||
function INTE : boolean;
|
||||
procedure CLI();
|
||||
@@ -87,7 +89,12 @@ var
|
||||
implementation
|
||||
|
||||
uses
|
||||
syslog, RTC, cpu, serial, strings, isr_types;
|
||||
cpu,
|
||||
isr_types,
|
||||
RTC,
|
||||
serial,
|
||||
strings,
|
||||
syslog;
|
||||
|
||||
function abs(x : sint32) : uint32;
|
||||
var
|
||||
@@ -274,14 +281,35 @@ end;
|
||||
procedure psleep(t : uint16);
|
||||
var
|
||||
t1, t2 : uint16;
|
||||
|
||||
spin : uint32;
|
||||
begin
|
||||
t1:= BDA^.Ticks;
|
||||
t2:= BDA^.Ticks;
|
||||
while t2-t1 < t do begin
|
||||
break;
|
||||
t2:= BDA^.Ticks;
|
||||
if t2 < t1 then break;
|
||||
t1 := BDA^.Ticks;
|
||||
|
||||
{ Busy-spin briefly to give the tick counter a chance to advance.
|
||||
If after ~50 000 iterations the counter hasn't moved, the timer
|
||||
ISR isn't installed yet — fall back to a rough CPU busy-wait
|
||||
(~1 ms per requested tick at typical Bochs/VBox speed). }
|
||||
spin := 0;
|
||||
t2 := t1;
|
||||
while t2 = t1 do begin
|
||||
spin := spin + 1;
|
||||
if spin > 50000 then begin
|
||||
{ Timer not running — approximate delay with busy loop.
|
||||
Each outer iteration ≈ a few µs; 50000 * t gives a
|
||||
very rough ms-scale delay that is good enough for the
|
||||
hardware settle times that call psleep. }
|
||||
for spin := 1 to uint32(t) * 50000 do
|
||||
asm nop end;
|
||||
exit;
|
||||
end;
|
||||
t2 := BDA^.Ticks;
|
||||
end;
|
||||
|
||||
{ Timer is running — use real ticks }
|
||||
t1 := BDA^.Ticks;
|
||||
while t2 - t1 < t do begin
|
||||
t2 := BDA^.Ticks;
|
||||
if t2 < t1 then break; { tick counter wrapped }
|
||||
end;
|
||||
end;
|
||||
|
||||
@@ -324,7 +352,6 @@ begin
|
||||
//serial.sendString('[outb]');
|
||||
//serial.sendHex(port);
|
||||
//serial.sendHex(val);
|
||||
psleep(1);
|
||||
asm
|
||||
PUSH EAX
|
||||
PUSH EDX
|
||||
|
||||
Some files were not shown because too many files have changed in this diff Show More
Reference in New Issue
Block a user