From 08ba1d1e1fb72952db3bcb9639295dda7521ebc8 Mon Sep 17 00:00:00 2001 From: nashzhou Date: Thu, 15 Oct 2026 12:24:20 +0800 Subject: [PATCH] perf(ublk): replace ArcSwap target state with RwLock to unblock swaps ArcSwap::store is swap + wait_for_readers: the writer pays a global per-process thread-node traversal and spins until readers of the old value clear their debt slots. Under high pause/resume concurrency, every warm-pool swap_state on acquire and every placeholder swap on release paid that cost even though each swapped device is idle; profiles showed swap_state dominated by atomic/spin time. parking_lot::RwLock> keeps the reader contract (an owned Arc snapshot that stays valid across awaits and later swaps) while writers never wait for readers. Reader cost becomes a shared lock plus an Arc clone; readers hold the lock only for the clone and swaps are rare, so shared-lock traffic is negligible. Drop the arc-swap dependency, which had no other user in the crate. Verified with cargo check/test/clippy/fmt on uvm-ublk; the overlaybd_target unit suite (5 tests, including the io_request discard path) passes. --- storage/ublk/Cargo.toml | 1 - storage/ublk/src/impls/overlaybd_target.rs | 38 ++++++++++++++-------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/storage/ublk/Cargo.toml b/storage/ublk/Cargo.toml index 58b8b17..61128b1 100644 --- a/storage/ublk/Cargo.toml +++ b/storage/ublk/Cargo.toml @@ -20,7 +20,6 @@ parking_lot = "0.12.5" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" uuid = { version = "1", features = ["v5"] } -arc-swap = "1.7" tracing = "0.1.41" tracing-log = "0.2.0" diff --git a/storage/ublk/src/impls/overlaybd_target.rs b/storage/ublk/src/impls/overlaybd_target.rs index 41d85fa..73d9a7e 100644 --- a/storage/ublk/src/impls/overlaybd_target.rs +++ b/storage/ublk/src/impls/overlaybd_target.rs @@ -1,10 +1,10 @@ use anyhow::{bail, Context, Result}; -use arc_swap::{ArcSwap, ArcSwapOption}; use async_trait::async_trait; use clap::Args; use overlaybd::image_file::ImageFile; use overlaybd::image_service::ImageService; use overlaybd::virtual_file::{IoCtx, VirtualFile}; +use parking_lot::RwLock; use std::fmt; use std::io::ErrorKind; use std::mem::size_of; @@ -46,13 +46,18 @@ pub struct OverlaybdTargetConfig { } pub struct OverlaybdTarget { - state: ArcSwap, - recorder: ArcSwapOption, + // RwLock of Arc instead of ArcSwap: an ArcSwap writer must wait for all + // readers of the old value (its global thread-node traversal spins), which + // stalls every warm-pool swap_state under high pause/resume concurrency. + // Swaps are rare and readers hold the lock for a clone, so shared-lock + // traffic is negligible. + state: RwLock>, + recorder: RwLock>>, } impl fmt::Debug for OverlaybdTarget { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let state = self.state.load(); + let state = self.current_state(); f.debug_struct("OverlaybdTarget") .field("image_config_path", &state.image_config_path) .field("dev_sectors", &state.dev_sectors) @@ -121,16 +126,22 @@ impl OverlaybdTarget { }; Ok(Self { - state: ArcSwap::new(Arc::new(state)), - recorder: ArcSwapOption::new(None), + state: RwLock::new(Arc::new(state)), + recorder: RwLock::new(None), }) } + /// Snapshot the current target state; the returned Arc stays valid across + /// concurrent swaps and awaits. + fn current_state(&self) -> Arc { + self.state.read().clone() + } + /// Attach (or detach with `None`) a startup-pack first-touch recorder. /// Only the dedicated memory device of a pack-recording VM arms this; /// rootfs and normal memory devices never do. pub fn set_recorder(&self, recorder: Option>) { - self.recorder.store(recorder); + *self.recorder.write() = recorder; } /// Swap the target's backing image and capacity atomically. @@ -167,8 +178,8 @@ impl OverlaybdTarget { // Per-image attachments must not survive a swap: a pooled device // reused for another image would otherwise keep recording for the // previous image. - self.recorder.store(None); - self.state.store(Arc::new(new_state)); + *self.recorder.write() = None; + *self.state.write() = Arc::new(new_state); Ok(()) } @@ -203,7 +214,8 @@ impl OverlaybdTarget { // the guard settles in-flight and latency accounting on completion. // The remote estimate only counts reads that touched new pages, so // re-reads of already-recorded pages cannot trip the abort guard. - let _read_guard = self.recorder.load_full().map(|recorder| { + let recorder = self.recorder.read().clone(); + let _read_guard = recorder.map(|recorder| { let touched_new = recorder.observe_submit(offset, len); recorder.read_guard(len as u64, touched_new) }); @@ -342,7 +354,7 @@ impl UVMUblkTarget for OverlaybdTarget { } fn ublk_dev_params(&self, dev_info: &ublk_sys::ublksrv_ctrl_dev_info) -> ublk_sys::ublk_params { - let state = self.state.load(); + let state = self.current_state(); build_ublk_params( state.dev_sectors, state.logical_bs_shift, @@ -361,7 +373,7 @@ impl UVMUblkTarget for OverlaybdTarget { _extra: Option<&mut IOBuffer>, io_ring: &AsyncIoRing, ) -> i32 { - let state = self.state.load_full(); + let state = self.current_state(); let (op, offset, len) = match self.request_meta(&state, io_desc) { Ok(meta) => meta, Err(err) => { @@ -582,7 +594,7 @@ mod tests { let reopened = OverlaybdTarget::open(&global_config, &image_config) .await .expect("reopen target"); - let state = reopened.state.load_full(); + let state = reopened.current_state(); let got = state .image .read_at(0, 4096) -- 2.43.0