Skip to content

Commit 1d32c1c

Browse files
committed
avoid excessive Arc<Mapping> contention
Conceptually it wasn't wrong for submappings to `Arc::clone` the pointed-to base mapping, but in practice all the accessors off of MemCtx return SubMapping with lifetimes constrained by that MemCtx. The sheer number of submappings being created and destroyed during I/O operations makes the refcount very hot under load. Lean into the de facto lifetime constraint on SubMapping and just take a borrow of the backing Mapping instead of Arc::clone (and implicit ref decrement in later drop).
1 parent 2dc6437 commit 1d32c1c

File tree

1 file changed

+6
-9
lines changed

1 file changed

+6
-9
lines changed

lib/propolis/src/vmm/mem.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ unsafe impl Sync for Mapping {}
378378
// not reference them directly as a field.
379379
#[allow(dead_code)]
380380
enum Backing<'a> {
381-
Base(Arc<Mapping>),
381+
Base(&'a Mapping),
382382
Sub(&'a SubMapping<'a>),
383383
}
384384

@@ -401,12 +401,9 @@ pub struct SubMapping<'a> {
401401
impl SubMapping<'_> {
402402
/// Create `SubMapping` using the entire region offered by an underlying
403403
/// `Mapping` object.
404-
fn new_base<'a>(
405-
_mem: &'a MemCtx,
406-
base: &'_ Arc<Mapping>,
407-
) -> SubMapping<'a> {
404+
fn new_base<'a>(base: &'a Mapping) -> SubMapping<'a> {
408405
SubMapping {
409-
backing: Backing::Base(base.clone()),
406+
backing: Backing::Base(base),
410407

411408
ptr: base.ptr,
412409
len: base.len,
@@ -1121,7 +1118,7 @@ impl MemCtx {
11211118
format!("memory region {} not found", name),
11221119
)
11231120
})?;
1124-
Ok(SubMapping::new_base(self, ent).constrain_access(Prot::WRITE))
1121+
Ok(SubMapping::new_base(ent).constrain_access(Prot::WRITE))
11251122
}
11261123

11271124
/// Like `writable_region`, but accesses the underlying memory segment
@@ -1167,12 +1164,12 @@ impl MemCtx {
11671164
MapKind::MmioReserve => None,
11681165
}?;
11691166

1170-
let guest_map = SubMapping::new_base(self, &seg.map_guest)
1167+
let guest_map = SubMapping::new_base(&seg.map_guest)
11711168
.constrain_access(prot)
11721169
.constrain_region(req_offset, len)
11731170
.expect("mapping offset should be valid");
11741171

1175-
let seg_map = SubMapping::new_base(self, &seg.map_seg)
1172+
let seg_map = SubMapping::new_base(&seg.map_seg)
11761173
.constrain_region(req_offset, len)
11771174
.expect("mapping offset should be valid");
11781175

0 commit comments

Comments
 (0)