Skip to content

Commit 107d822

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 (except in tests). Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
1 parent a08efb7 commit 107d822

File tree

1 file changed

+18
-25
lines changed

1 file changed

+18
-25
lines changed

src/vmm/src/vstate/vcpu.rs

Lines changed: 18 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,23 @@ 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+
// The reason for not asserting TLS being set here is that
600+
// it can happen that Vcpu::Drop is called on vcpus which never were
601+
// put on their threads. This can happen if some error occurs during Vmm
602+
// setup before `start_threaded` call.
603+
if let Some(_vcpu_ptr) = cell.get() {
604+
// During normal runtime there is a strong assumption that vcpus will be
605+
// put on their own threads, thus TLS will be initialized with the
606+
// correct pointer.
607+
// In test we do not put vcpus on separate threads, so TLS will have a value
608+
// of the last created vcpu.
609+
#[cfg(not(test))]
610+
assert!(std::ptr::eq(_vcpu_ptr, self));
611+
612+
Self::TLS_VCPU_PTR.take();
613+
}
614+
})
619615
}
620616
}
621617

@@ -1039,15 +1035,12 @@ pub(crate) mod tests {
10391035
}
10401036

10411037
// Reset vcpu TLS.
1042-
vcpu.reset_thread_local_data().unwrap();
1038+
Vcpu::TLS_VCPU_PTR.with(|cell| cell.take());
10431039

10441040
// Running on the TLS vcpu after TLS reset should fail.
10451041
unsafe {
10461042
Vcpu::run_on_thread_local(|_| ()).unwrap_err();
10471043
}
1048-
1049-
// Second reset should return error.
1050-
vcpu.reset_thread_local_data().unwrap_err();
10511044
}
10521045

10531046
#[test]

0 commit comments

Comments
 (0)