Skip to content

Commit 8db599b

Browse files
committed
refactor: move TLS reset into Vcpu::Drop
- The reset must be called only once on vcpu drop, so move it directly into the Drop impl. - Replace errors with asserts since there is an assumption that TLS will always hold correct vcpu pointer for a given thread. - Mark Drop impl as non test only as our unit tests create multiple vcpus which will panic on Drop since they all will be referencing same TLS. Signed-off-by: Egor Lazarchuk <[email protected]>
1 parent 7ee2175 commit 8db599b

File tree

1 file changed

+12
-25
lines changed

1 file changed

+12
-25
lines changed

src/vmm/src/vstate/vcpu.rs

Lines changed: 12 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.
@@ -613,9 +593,19 @@ fn handle_kvm_exit(
613593
}
614594
}
615595

596+
// During normal runtime there is a strong assumption that vcpus will be
597+
// put on their own threads, thus TLS will be initialized.
598+
// In test we do not put vcpus on separate threads.
599+
#[cfg(not(test))]
616600
impl Drop for Vcpu {
617601
fn drop(&mut self) {
618-
let _ = self.reset_thread_local_data();
602+
Self::TLS_VCPU_PTR.with(|cell| {
603+
let vcpu_ptr = cell.get().unwrap();
604+
assert!(std::ptr::eq(vcpu_ptr, self));
605+
// Have to do this trick because `update` method is
606+
// not stable on Cell.
607+
Self::TLS_VCPU_PTR.with(|cell| cell.take());
608+
})
619609
}
620610
}
621611

@@ -1039,15 +1029,12 @@ pub(crate) mod tests {
10391029
}
10401030

10411031
// Reset vcpu TLS.
1042-
vcpu.reset_thread_local_data().unwrap();
1032+
Vcpu::TLS_VCPU_PTR.with(|cell| cell.take());
10431033

10441034
// Running on the TLS vcpu after TLS reset should fail.
10451035
unsafe {
10461036
Vcpu::run_on_thread_local(|_| ()).unwrap_err();
10471037
}
1048-
1049-
// Second reset should return error.
1050-
vcpu.reset_thread_local_data().unwrap_err();
10511038
}
10521039

10531040
#[test]

0 commit comments

Comments
 (0)