Skip to content

Commit 682e44c

Browse files
jonathanpwangnyunyunyunyu
authored andcommitted
fix: force a sync on drop subtree for safety
1 parent 9709a94 commit 682e44c

File tree

2 files changed

+11
-14
lines changed

2 files changed

+11
-14
lines changed

crates/vm/src/system/cuda/memory.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,12 @@ impl MemoryInventoryGPU {
203203
}
204204
mem.tracing_info("merkle update");
205205
persistent.merkle_tree.finalize();
206-
Some(persistent.merkle_tree.update_with_touched_blocks(
206+
let merkle_tree_ctx = persistent.merkle_tree.update_with_touched_blocks(
207207
unpadded_merkle_height,
208208
&d_touched_memory,
209209
empty,
210-
))
210+
);
211+
Some(merkle_tree_ctx)
211212
}
212213
TouchedMemory::Volatile(partition) => {
213214
assert!(self.persistent.is_none(), "TouchedMemory enum mismatch");

crates/vm/src/system/cuda/merkle_tree/mod.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -309,24 +309,20 @@ impl MemoryMerkleTree {
309309

310310
/// Drops all massive buffers to free memory. Used at the end of an execution segment.
311311
///
312-
/// Caution: this method destroys all subtree streams and events. If events have not been
313-
/// manually removed after completion, they are enqueued to the default stream and then a forced
314-
/// synchronization is performed on the default stream. The forced synchronization is avoided if
315-
/// all events are manually removed when they are known to be completed (e.g., after some other
316-
/// synchronization like D2H transfer).
312+
/// Caution: this method destroys all subtree streams and events. For safety, we force
313+
/// synchronize all subtree streams and the default stream (cudaStreamPerThread) with host
314+
/// before deallocating buffers.
317315
pub fn drop_subtrees(&mut self) {
318-
let mut needs_sync = false;
319316
// Make sure all streams are synchronized before destroying events
320317
for subtree in self.subtrees.iter() {
321318
subtree.stream.synchronize().unwrap();
322-
if let Some(event) = subtree.build_completion_event.as_ref() {
323-
needs_sync = true;
324-
default_stream_wait(event).unwrap();
319+
if let Some(_event) = subtree.build_completion_event.as_ref() {
320+
tracing::warn!(
321+
"Dropping merkle subtree before build_async event has been destroyed"
322+
);
325323
}
326324
}
327-
if needs_sync {
328-
current_stream_sync().unwrap();
329-
}
325+
current_stream_sync().unwrap();
330326
// Clearing will drop streams (which calls synchronize again) and drops events (which
331327
// destroys them)
332328
self.subtrees.clear();

0 commit comments

Comments
 (0)