Skip to content

Commit b6ee146

Browse files
authored
Introduce MmapResult (#1451)
This PR fixes #1450.
1 parent d9b40da commit b6ee146

File tree

17 files changed

+126
-75
lines changed

17 files changed

+126
-75
lines changed

src/policy/space.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
207207
self.get_name(),
208208
))
209209
{
210-
OS::handle_mmap_error::<VM>(mmap_error, tls, res.start, bytes);
210+
OS::handle_mmap_error::<VM>(mmap_error, tls);
211211
}
212212
};
213213
let grow_space = || {

src/util/address.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,11 @@ impl Address {
203203
Address(self.0 + size)
204204
}
205205

206+
/// Wrapping (modular) addition. Computes self + rhs, wrapping around at the boundary of the type.
207+
pub const fn wrapping_add(self, size: usize) -> Address {
208+
Address(self.0.wrapping_add(size))
209+
}
210+
206211
// We implemented the Sub trait but we still keep this sub function.
207212
// The sub() function is const fn, and we can use it to declare Address constants.
208213
// The Sub trait function cannot be const.

src/util/heap/layout/mmapper/csm/byte_map_storage.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ use std::fmt;
99
use std::sync::atomic::Ordering;
1010
use std::sync::Mutex;
1111

12+
use crate::util::os::MmapResult;
1213
use atomic::Atomic;
13-
use std::io::Result;
1414

1515
/// Logarithm of the address space size that [`ByteMapStateStorage`] is able to handle.
1616
/// This is enough for 32-bit architectures.
@@ -60,9 +60,9 @@ impl MapStateStorage for ByteMapStateStorage {
6060
}
6161
}
6262

63-
fn bulk_transition_state<F>(&self, range: ChunkRange, mut update_fn: F) -> Result<()>
63+
fn bulk_transition_state<F>(&self, range: ChunkRange, mut update_fn: F) -> MmapResult<()>
6464
where
65-
F: FnMut(ChunkRange, MapState) -> Result<Option<MapState>>,
65+
F: FnMut(ChunkRange, MapState) -> MmapResult<Option<MapState>>,
6666
{
6767
if range.is_empty() {
6868
return Ok(());

src/util/heap/layout/mmapper/csm/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::util::heap::layout::Mmapper;
55
use crate::util::os::*;
66
use crate::util::Address;
77
use bytemuck::NoUninit;
8-
use std::{io::Result, sync::Mutex};
8+
use std::sync::Mutex;
99

1010
mod byte_map_storage;
1111
#[cfg(target_pointer_width = "64")]
@@ -94,9 +94,9 @@ trait MapStateStorage {
9494
/// - `Ok(Some(new_state))`: Set the state of all chunks within `group_range` to `new_state`.
9595
///
9696
/// Return `Ok(())` if finished visiting all chunks normally.
97-
fn bulk_transition_state<F>(&self, range: ChunkRange, update_fn: F) -> Result<()>
97+
fn bulk_transition_state<F>(&self, range: ChunkRange, update_fn: F) -> MmapResult<()>
9898
where
99-
F: FnMut(ChunkRange, MapState) -> Result<Option<MapState>>;
99+
F: FnMut(ChunkRange, MapState) -> MmapResult<Option<MapState>>;
100100
}
101101

102102
/// A [`Mmapper`] implementation based on a logical array of chunk states.
@@ -148,7 +148,7 @@ impl Mmapper for ChunkStateMmapper {
148148
pages: usize,
149149
huge_page_option: HugePageSupport,
150150
anno: &MmapAnnotation,
151-
) -> Result<()> {
151+
) -> MmapResult<()> {
152152
let _guard = self.transition_lock.lock().unwrap();
153153

154154
let bytes = pages << LOG_BYTES_IN_PAGE;
@@ -189,7 +189,7 @@ impl Mmapper for ChunkStateMmapper {
189189
huge_page_option: HugePageSupport,
190190
prot: MmapProtection,
191191
anno: &MmapAnnotation,
192-
) -> Result<()> {
192+
) -> MmapResult<()> {
193193
let _guard = self.transition_lock.lock().unwrap();
194194

195195
let bytes = pages << LOG_BYTES_IN_PAGE;

src/util/heap/layout/mmapper/csm/two_level_storage.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
use super::MapState;
77
use crate::util::heap::layout::mmapper::csm::{ChunkRange, MapStateStorage};
88
use crate::util::heap::layout::vm_layout::*;
9+
use crate::util::os::MmapResult;
910
use crate::util::rust_util::atomic_box::OnceOptionBox;
1011
use crate::util::rust_util::rev_group::RevisitableGroupByForIterator;
1112
use crate::util::rust_util::zeroed_alloc::new_zeroed_vec;
1213
use crate::util::Address;
1314
use atomic::{Atomic, Ordering};
1415
use std::fmt;
15-
use std::io::Result;
1616

1717
/// Logarithm of the address space size that [`TwoLevelStateStorage`] is able to handle.
1818
/// This is enough for ARM64, x86_64 and some other architectures.
@@ -103,9 +103,9 @@ impl MapStateStorage for TwoLevelStateStorage {
103103
});
104104
}
105105

106-
fn bulk_transition_state<F>(&self, range: ChunkRange, mut update_fn: F) -> Result<()>
106+
fn bulk_transition_state<F>(&self, range: ChunkRange, mut update_fn: F) -> MmapResult<()>
107107
where
108-
F: FnMut(ChunkRange, MapState) -> Result<Option<MapState>>,
108+
F: FnMut(ChunkRange, MapState) -> MmapResult<Option<MapState>>,
109109
{
110110
if range.is_empty() {
111111
return Ok(());

src/util/heap/layout/mmapper/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::util::{os::*, Address};
2-
use std::io::Result;
32

43
#[allow(unused)] // Used in doc comment.
54
use crate::util::constants::LOG_BYTES_IN_PAGE;
@@ -76,7 +75,7 @@ pub trait Mmapper: Sync {
7675
pages: usize,
7776
huge_page_option: HugePageSupport,
7877
anno: &MmapAnnotation,
79-
) -> Result<()>;
78+
) -> MmapResult<()>;
8079

8180
/// Ensure that a range of pages is mmapped (or equivalent). If the
8281
/// pages are not yet mapped, demand-zero map them. Note that mapping
@@ -97,7 +96,7 @@ pub trait Mmapper: Sync {
9796
huge_page_option: HugePageSupport,
9897
prot: MmapProtection,
9998
anno: &MmapAnnotation,
100-
) -> Result<()>;
99+
) -> MmapResult<()>;
101100

102101
/// Is the page pointed to by this address mapped? Returns true if
103102
/// the page at the given address is mapped.

src/util/metadata/side_metadata/global.rs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use crate::util::Address;
1010
use num_traits::FromPrimitive;
1111
use ranges::BitByteRange;
1212
use std::fmt;
13-
use std::io::Result;
1413
use std::sync::atomic::{AtomicU8, Ordering};
1514

1615
/// This struct stores the specification of a side metadata bit-set.
@@ -1391,7 +1390,7 @@ impl SideMetadataContext {
13911390
start: Address,
13921391
size: usize,
13931392
space_name: &str,
1394-
) -> Result<()> {
1393+
) -> MmapResult<()> {
13951394
debug!(
13961395
"try_map_metadata_space({}, 0x{:x}, {}, {})",
13971396
start,
@@ -1414,7 +1413,7 @@ impl SideMetadataContext {
14141413
start: Address,
14151414
size: usize,
14161415
name: &str,
1417-
) -> Result<()> {
1416+
) -> MmapResult<()> {
14181417
debug!(
14191418
"try_map_metadata_address_range({}, 0x{:x}, {}, {})",
14201419
start,
@@ -1441,16 +1440,13 @@ impl SideMetadataContext {
14411440
size: usize,
14421441
no_reserve: bool,
14431442
space_name: &str,
1444-
) -> Result<()> {
1443+
) -> MmapResult<()> {
14451444
for spec in self.global.iter() {
14461445
let anno = MmapAnnotation::SideMeta {
14471446
space: space_name,
14481447
meta: spec.name,
14491448
};
1450-
match try_mmap_contiguous_metadata_space(start, size, spec, no_reserve, &anno) {
1451-
Ok(_) => {}
1452-
Err(e) => return Result::Err(e),
1453-
}
1449+
try_mmap_contiguous_metadata_space(start, size, spec, no_reserve, &anno)?;
14541450
}
14551451

14561452
#[cfg(target_pointer_width = "32")]
@@ -1474,10 +1470,7 @@ impl SideMetadataContext {
14741470
space: space_name,
14751471
meta: spec.name,
14761472
};
1477-
match try_mmap_contiguous_metadata_space(start, size, spec, no_reserve, &anno) {
1478-
Ok(_) => {}
1479-
Err(e) => return Result::Err(e),
1480-
}
1473+
try_mmap_contiguous_metadata_space(start, size, spec, no_reserve, &anno)?;
14811474
}
14821475
#[cfg(target_pointer_width = "32")]
14831476
{
@@ -1500,10 +1493,7 @@ impl SideMetadataContext {
15001493
space: space_name,
15011494
meta: "all",
15021495
};
1503-
match try_map_per_chunk_metadata_space(start, size, lsize, no_reserve, &anno) {
1504-
Ok(_) => {}
1505-
Err(e) => return Result::Err(e),
1506-
}
1496+
try_map_per_chunk_metadata_space(start, size, lsize, no_reserve, &anno)?;
15071497
}
15081498

15091499
Ok(())

src/util/metadata/side_metadata/helpers.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use crate::util::metadata::side_metadata::address_to_chunked_meta_address;
99
use crate::util::os::*;
1010
use crate::util::Address;
1111
use crate::MMAPPER;
12-
use std::io::Result;
1312

1413
/// Performs address translation in contiguous metadata spaces (e.g. global and policy-specific in 64-bits, and global in 32-bits)
1514
pub(super) fn address_to_contiguous_meta_address(
@@ -127,7 +126,7 @@ pub(super) fn try_mmap_contiguous_metadata_space(
127126
spec: &SideMetadataSpec,
128127
no_reserve: bool,
129128
anno: &MmapAnnotation,
130-
) -> Result<usize> {
129+
) -> MmapResult<usize> {
131130
debug_assert!(start.is_aligned_to(BYTES_IN_PAGE));
132131
debug_assert!(size % BYTES_IN_PAGE == 0);
133132

src/util/metadata/side_metadata/helpers_32.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use crate::util::{
55
os::*,
66
Address,
77
};
8-
use std::io::Result;
98

109
use super::constants::{
1110
LOCAL_SIDE_METADATA_BASE_ADDRESS, LOCAL_SIDE_METADATA_PER_CHUNK,
@@ -113,7 +112,7 @@ pub(super) fn try_map_per_chunk_metadata_space(
113112
local_per_chunk: usize,
114113
no_reserve: bool,
115114
anno: &MmapAnnotation,
116-
) -> Result<usize> {
115+
) -> MmapResult<usize> {
117116
let mut aligned_start = start.align_down(BYTES_IN_CHUNK);
118117
let aligned_end = (start + size).align_up(BYTES_IN_CHUNK);
119118

@@ -152,11 +151,12 @@ pub(super) fn try_map_per_chunk_metadata_space(
152151
local_per_chunk,
153152
res
154153
);
155-
return Result::Err(res.err().unwrap());
154+
return Err(res.err().unwrap());
156155
}
157156
if munmap_first_chunk.is_none() {
158157
// if first chunk is newly mapped, it needs munmap on failure
159-
let map_exists = res.is_err_and(|e| e.kind() == std::io::ErrorKind::AlreadyExists);
158+
let map_exists =
159+
res.is_err_and(|e| e.error.kind() == std::io::ErrorKind::AlreadyExists);
160160
munmap_first_chunk = Some(map_exists);
161161
}
162162
aligned_start += BYTES_IN_CHUNK;
@@ -178,7 +178,7 @@ pub(super) fn try_mmap_metadata_chunk(
178178
local_per_chunk: usize,
179179
no_reserve: bool,
180180
anno: &MmapAnnotation,
181-
) -> Result<()> {
181+
) -> MmapResult<()> {
182182
debug_assert!(start.is_aligned_to(BYTES_IN_CHUNK));
183183

184184
let policy_meta_start = address_to_meta_chunk_addr(start);

src/util/os/imp/unix_like/linux_like/android.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ impl OSMemory for Android {
1414
size: usize,
1515
strategy: MmapStrategy,
1616
annotation: &MmapAnnotation<'_>,
17-
) -> Result<Address> {
17+
) -> MmapResult<Address> {
1818
linux_common::dzmmap(start, size, strategy, annotation)
1919
}
2020

0 commit comments

Comments
 (0)