Skip to content

Commit ceaf94d

Browse files
committed
refactor: move TLS reset into Vcpu::Drop
The reset must be called only once on vcpu drop, so directly move it into the Drop impl. Tests can use a simplified version. Replace errors with asserts since there is an assumption that TLS will always hold correct vcpu pointer for a given thread. Signed-off-by: Egor Lazarchuk <[email protected]>
1 parent 972e217 commit ceaf94d

File tree

1 file changed

+25
-25
lines changed

1 file changed

+25
-25
lines changed

src/vmm/src/vstate/vcpu.rs

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -124,26 +124,6 @@ impl Vcpu {
124124
})
125125
}
126126

127-
/// Deassociates `self` from the current thread.
128-
///
129-
/// Should be called if the current `self` had called `init_thread_local_data()` and
130-
/// now needs to move to a different thread.
131-
///
132-
/// Fails if `self` was not previously associated with the current thread.
133-
fn reset_thread_local_data(&mut self) -> Result<(), VcpuError> {
134-
// Best-effort to clean up TLS. If the `Vcpu` was moved to another thread
135-
// _before_ running this, then there is nothing we can do.
136-
Self::TLS_VCPU_PTR.with(|cell: &VcpuCell| {
137-
if let Some(vcpu_ptr) = cell.get() {
138-
if std::ptr::eq(vcpu_ptr, self) {
139-
Self::TLS_VCPU_PTR.with(|cell: &VcpuCell| cell.take());
140-
return Ok(());
141-
}
142-
}
143-
Err(VcpuError::VcpuTlsNotPresent)
144-
})
145-
}
146-
147127
/// Runs `func` for the `Vcpu` associated with the current thread.
148128
///
149129
/// It requires that `init_thread_local_data()` was run on this thread.
@@ -615,7 +595,13 @@ fn handle_kvm_exit(
615595

616596
impl Drop for Vcpu {
617597
fn drop(&mut self) {
618-
let _ = self.reset_thread_local_data();
598+
Self::TLS_VCPU_PTR.with(|cell| {
599+
let vcpu_ptr = cell.get().unwrap();
600+
assert!(std::ptr::eq(vcpu_ptr, self));
601+
// Have to do this trick because `update` method is
602+
// not stable on Cell.
603+
Self::TLS_VCPU_PTR.with(|cell| cell.take());
604+
})
619605
}
620606
}
621607

@@ -1011,6 +997,12 @@ pub(crate) mod tests {
1011997
(vm, vcpu_handle, vcpu_exit_evt)
1012998
}
1013999

1000+
impl Vcpu {
1001+
fn reset_thread_local_data(&mut self) {
1002+
Self::TLS_VCPU_PTR.with(|cell| cell.take());
1003+
}
1004+
}
1005+
10141006
#[test]
10151007
fn test_set_mmio_bus() {
10161008
let (_, _, mut vcpu) = setup_vcpu(0x1000);
@@ -1039,15 +1031,12 @@ pub(crate) mod tests {
10391031
}
10401032

10411033
// Reset vcpu TLS.
1042-
vcpu.reset_thread_local_data().unwrap();
1034+
vcpu.reset_thread_local_data();
10431035

10441036
// Running on the TLS vcpu after TLS reset should fail.
10451037
unsafe {
10461038
Vcpu::run_on_thread_local(|_| ()).unwrap_err();
10471039
}
1048-
1049-
// Second reset should return error.
1050-
vcpu.reset_thread_local_data().unwrap_err();
10511040
}
10521041

10531042
#[test]
@@ -1058,6 +1047,17 @@ pub(crate) mod tests {
10581047
vcpu.init_thread_local_data();
10591048
}
10601049

1050+
#[test]
1051+
#[should_panic]
1052+
fn test_vcpu_tls_double_reset() {
1053+
let (_, _, mut vcpu) = setup_vcpu(0x1000);
1054+
vcpu.init_thread_local_data();
1055+
// This reset will succeed as the TLS
1056+
// was initialized
1057+
vcpu.reset_thread_local_data();
1058+
// The Drop in vcpu will panic now
1059+
}
1060+
10611061
#[test]
10621062
fn test_vcpu_kick() {
10631063
let (_, vm, mut vcpu) = setup_vcpu(0x1000);

0 commit comments

Comments
 (0)