8000 Adding the new memory subsystem by Meandres · Pull Request #47 · miniosv/miniosv · GitHub
[go: up one dir, main page]
Skip to content

Adding the new memory subsystem - #47

Open
Meandres wants to merge 7 commits into
masterfrom
mem/subsystem
Open

Adding the new memory subsystem#47
Meandres wants to merge 7 commits into
masterfrom
mem/subsystem

Conversation

@Meandres
@Meandres Meandres commented Sep 10, 2026
Copy link
Copy Markdown
Collaborator

The big PR that we talked about.
I'm not exactly sure how to properly merge this.
Open to comments and review of course.

@Martin-Lndbl
Martin-Lndbl commented Sep 10, 2026
Copy link
Copy Markdown
Collaborator

I ran into a segfault using this on c6in.8xlarge, TPCH query 10 sf=10 & let claude diagnose this. Sorry for the AI generated description below:

Still seeing memory corruption after 065362f, on kite/mininet, which uses the memory subsystem. Workload is heavy mmap churn under memory pressure (DuckDB TPC-H, 8–32 threads, 64 GiB). Three faces of what looks like one bug: page fault on first touch of freshly mmap'd memory, Assertion failed: frame < self->lower.frames (llfree_put: 544), and silent data corruption.

Patching sigsegv() in core/mem/fault.cc to dump the faulting address's provenance gives:

page fault outside application, addr: 0x000020806c3c1010
  in region [0x000020806c3c1000, 0x000020806c57d000) 1776 KiB perm=3 fault_ops=0
  leaf: present level=0 perm=0 present_bit=0 phys=0x0003581663b50000
  fault: error=0x2 (write, data)

That's the first page of a live anon region populate() reported as mapped — and the PTE slot is non-empty but has present_bit=0, perm=0, and a physical address of 856 TiB on a 64 GiB machine.

A PTE with a garbage address and no present bit isn't a torn-down mapping; it's someone else's data being read as a page table. Reads as a page-table page freed while still referenced, then reused — which would also explain the other two faces: a garbage frame number from such a PTE is exactly what trips llfree_put, and garbage that happens to be readable becomes corruption rather than a fault.

res.retire()/res.settle() targets this class but doesn't cover it — all of the above is post-merge. No minimal reproducer yet; test/os-memory{,-primitives} pass at default memory. Happy to share the fault.cc diagnostic patch or run experiments.

@Meandres
Meandres commented Sep 10, 2026
Copy link
Copy Markdown
Collaborator Author

I reproduced the problem with a local NVMe. Working on a fix.
By the way, this reminds me that this version only accepts mmap calls from the libc for anonymous memory (since there is no filesystem by default in mainline). Since its limited to anonymous (unreclaimable) memory, it also eagerly populates the region (allocate frames + map them).
I believe this is safe but I should mention this somewhere.

@Meandres
Meandres commented Sep 10, 2026
Copy link
Copy Markdown
Collaborator Author

I believe that this crash is due to the lazy TLB invalidation bug that is fixed by 9bc546e on the fork and by cbf7b76 (first commit of this PR. Can you try to rebase TUM-DSE#3 to the HEAD of the fork ?

@Martin-Lndbl
Martin-Lndbl commented Sep 14, 2026
Copy link
Copy Markdown
Collaborator

You are right, this is already resolved

@seb711
seb711 commented Sep 16, 2026
Copy link
Copy Markdown
Collaborator

Took a look into the docs; not sure if we should merge the memory pressure feature and the page cache module. IMO this is not what miniOSv should supply as this already inserts too many interfaces / dogmas that are not necessarily needed.

I would rather just push the everything besides this and then push the memory pressure feature / page cache module later. Can we split the PR?

@Meandres
Meandres commented Sep 22, 2026
Copy link
Copy Markdown
Collaborator Author

This implements the proposed changes. It removes the page cache and the heap allocator and instead adds a simplistic heap allocator. I also updated the docs and the tests to corresponds to this new version.

@Meandres
Merge branch 'master' into mem/subsystem
b88de49 
Resolution: the suite moved to tests/ and gained os-string, the arch
mmu.o line is still gone while master's x64 memcmp rule stays, and the
overview points at tests/.
@Meandres Meandres mentioned this pull request Oct 22, 2026
Open
@Meandres Meandres mentioned this pull request Nov 6, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Meandres @Martin-Lndbl @seb711
0