Skip to content

Conversation

@mkroening
Copy link
Member

@mkroening mkroening commented Oct 22, 2025

Wasmtime allocates 4 GiB of PROT_NONE (Data cannot be accessed) memory.

Since we don't allocate any backing physical frames in mmap for PROT_NONE, we should neither try to deallocate any physical frames in munmap in those cases.

@mkroening mkroening requested a review from stlankes October 22, 2025 13:45
@mkroening mkroening self-assigned this Oct 22, 2025
@mkroening
Copy link
Member Author

CC: @m-mueller678

@m-mueller678
Copy link
Contributor

This looks like an ok fix for the issue at hand to me. However, I think mmap has a bunch of problems and this is more of a band-aid solution. For example, this implementaiton breaks if the affected area contains both pages mapped with PROT_NONE and pages with some access allowed. From my reading of the man pages, it is perfectly fine to use linux mmap like that.

For a proper solution, we should decide on which level mmap should operate on:

If it is a high level api, we should track the state of all pages mapped via mmap. munmap should refuse to unmap pages which have not been allocated via mmap. This could be done via a hashtable or using the extra bits available in the pagetable. As an example of why this is needed, you could use munmap to unmap the identity mappings with the current implementation.

If it is a low level api, it should interact directly with the free lists and the page table. In that case, munmap should just walk the page table and add anything it finds to the physical free list. We should specify incorrect use of mmap to be UB, as we can not really detect it reliably. This would make our mmap incompatible with the linux one.

I would prefer mmap to be an optional high level feature and to have an additional set of syscalls that allows direct access to the free lists.

@mkroening
Copy link
Member Author

Yeah, this was meant to be a stopgap solution without rewriting mmap now to be able to move forward with infallible deallocations for #2002.

I think our goal would be a POSIX-compatible mmap implementation, but currently we don't even have the correct signature. Stefan started working on improving the situation but did not get around to finishing it yet (#1885). Your idea sounds good to me, though. 👍

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark Results

Benchmark Current: 0cc67df Previous: e747b5a Performance Ratio
startup_benchmark Build Time 113.67 s 114.77 s 0.99
startup_benchmark File Size 0.89 MB 0.89 MB 1.00
Startup Time - 1 core 0.92 s (±0.04 s) 0.92 s (±0.03 s) 1.00
Startup Time - 2 cores 0.93 s (±0.03 s) 0.90 s (±0.04 s) 1.03
Startup Time - 4 cores 0.92 s (±0.04 s) 0.91 s (±0.03 s) 1.00
multithreaded_benchmark Build Time 114.29 s 114.10 s 1.00
multithreaded_benchmark File Size 1.00 MB 1.00 MB 1.00
Multithreaded Pi Efficiency - 2 Threads 85.38 % (±9.31 %) 89.62 % (±6.95 %) 0.95
Multithreaded Pi Efficiency - 4 Threads 42.74 % (±4.41 %) 45.18 % (±3.88 %) 0.95
Multithreaded Pi Efficiency - 8 Threads 24.63 % (±2.28 %) 25.55 % (±1.65 %) 0.96
micro_benchmarks Build Time 237.81 s 231.77 s 1.03
micro_benchmarks File Size 1.01 MB 1.01 MB 1.00
Scheduling time - 1 thread 139.36 ticks (±28.83 ticks) 113.09 ticks (±35.98 ticks) 1.23
Scheduling time - 2 threads 76.40 ticks (±16.60 ticks) 70.15 ticks (±23.09 ticks) 1.09
Micro - Time for syscall (getpid) 8.56 ticks (±4.58 ticks) 6.31 ticks (±2.49 ticks) 1.36
Memcpy speed - (built_in) block size 4096 57964.41 MByte/s (±42036.72 MByte/s) 60024.89 MByte/s (±42578.47 MByte/s) 0.97
Memcpy speed - (built_in) block size 1048576 23280.56 MByte/s (±21198.90 MByte/s) 23888.20 MByte/s (±20500.48 MByte/s) 0.97
Memcpy speed - (built_in) block size 16777216 14754.68 MByte/s (±12183.73 MByte/s) 13939.73 MByte/s (±11551.16 MByte/s) 1.06
Memset speed - (built_in) block size 4096 56529.06 MByte/s (±41465.35 MByte/s) 60259.47 MByte/s (±42726.53 MByte/s) 0.94
Memset speed - (built_in) block size 1048576 23653.33 MByte/s (±21380.78 MByte/s) 24990.35 MByte/s (±21422.35 MByte/s) 0.95
Memset speed - (built_in) block size 16777216 15191.54 MByte/s (±12461.50 MByte/s) 14008.28 MByte/s (±11573.03 MByte/s) 1.08
Memcpy speed - (rust) block size 4096 55805.58 MByte/s (±40388.88 MByte/s) 55064.94 MByte/s (±38976.39 MByte/s) 1.01
Memcpy speed - (rust) block size 1048576 19831.67 MByte/s (±16980.91 MByte/s) 28247.94 MByte/s (±24357.11 MByte/s) 0.70
Memcpy speed - (rust) block size 16777216 14892.23 MByte/s (±12361.28 MByte/s) 14239.45 MByte/s (±11877.65 MByte/s) 1.05
Memset speed - (rust) block size 4096 56643.79 MByte/s (±40940.21 MByte/s) 55273.06 MByte/s (±39093.56 MByte/s) 1.02
Memset speed - (rust) block size 1048576 20148.48 MByte/s (±17110.00 MByte/s) 29360.47 MByte/s (±25105.97 MByte/s) 0.69
Memset speed - (rust) block size 16777216 15199.58 MByte/s (±12505.15 MByte/s) 14321.88 MByte/s (±11903.00 MByte/s) 1.06
alloc_benchmarks Build Time 239.83 s 215.99 s 1.11
alloc_benchmarks File Size 0.96 MB 0.96 MB 1.00
Allocations - Allocation success 100.00 % 100.00 % 1
Allocations - Deallocation success 70.00 % (±0.22 %) 70.04 % (±0.38 %) 1.00
Allocations - Pre-fail Allocations 100.00 % 100.00 % 1
Allocations - Average Allocation time 10396.52 Ticks (±357.13 Ticks) 18387.00 Ticks (±564.30 Ticks) 0.57
Allocations - Average Allocation time (no fail) 10396.52 Ticks (±357.13 Ticks) 18387.00 Ticks (±564.30 Ticks) 0.57
Allocations - Average Deallocation time 1382.00 Ticks (±154.75 Ticks) 1278.91 Ticks (±218.11 Ticks) 1.08
mutex_benchmark Build Time 239.20 s 225.69 s 1.06
mutex_benchmark File Size 1.01 MB 1.01 MB 1.00
Mutex Stress Test Average Time per Iteration - 1 Threads 26.86 ns (±5.33 ns) 21.70 ns (±3.91 ns) 1.24
Mutex Stress Test Average Time per Iteration - 2 Threads 23.88 ns (±2.92 ns) 22.60 ns (±3.10 ns) 1.06

This comment was automatically generated by workflow using github-action-benchmark.

@mkroening mkroening mentioned this pull request Oct 23, 2025
@mkroening mkroening added this pull request to the merge queue Oct 23, 2025
Merged via the queue into main with commit 0e904b3 Oct 23, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants