From 0531dc45c36ab464a3c8b74f99550bc9521554ae Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 15 Apr 2017 07:27:09 +0000 Subject: [PATCH] DMA: erase trace before re-recording it. Or we could needlessly OOM replacing a large trace. --- artiq/coredevice/dma.py | 8 ++--- artiq/firmware/ksupport/lib.rs | 11 +++--- artiq/firmware/libproto/kernel_proto.rs | 3 +- artiq/firmware/runtime/rtio_dma.rs | 48 ++++++++++++++----------- artiq/firmware/runtime/session.rs | 8 ++--- 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/artiq/coredevice/dma.py b/artiq/coredevice/dma.py index 070f2c2d8..9148863d3 100644 --- a/artiq/coredevice/dma.py +++ b/artiq/coredevice/dma.py @@ -12,11 +12,11 @@ from numpy import int64 @syscall -def dma_record_start() -> TNone: +def dma_record_start(name: TStr) -> TNone: raise NotImplementedError("syscall not simulated") @syscall -def dma_record_stop(name: TStr, duration: TInt64) -> TNone: +def dma_record_stop(duration: TInt64) -> TNone: raise NotImplementedError("syscall not simulated") @syscall @@ -45,13 +45,13 @@ class DMARecordContextManager: @kernel def __enter__(self): - dma_record_start() # this may raise, so do it before altering now + dma_record_start(self.name) # this may raise, so do it before altering now self.saved_now_mu = now_mu() at_mu(0) @kernel def __exit__(self, type, value, traceback): - dma_record_stop(self.name, now_mu()) # see above + dma_record_stop(now_mu()) # see above at_mu(self.saved_now_mu) diff --git a/artiq/firmware/ksupport/lib.rs b/artiq/firmware/ksupport/lib.rs index fd2027185..620115121 100644 --- a/artiq/firmware/ksupport/lib.rs +++ b/artiq/firmware/ksupport/lib.rs @@ -262,7 +262,9 @@ fn dma_record_flush() { } } -extern fn dma_record_start() { +extern fn dma_record_start(name: CSlice) { + let name = str::from_utf8(name.as_ref()).unwrap(); + unsafe { if DMA_RECORDER.active { raise!("DMAError", "DMA is already recording") @@ -275,13 +277,11 @@ extern fn dma_record_start() { dma_record_output_wide as *const () as u32).unwrap(); DMA_RECORDER.active = true; - send(&DmaRecordStart); + send(&DmaRecordStart(name)); } } -extern fn dma_record_stop(name: CSlice, duration: i64) { - let name = str::from_utf8(name.as_ref()).unwrap(); - +extern fn dma_record_stop(duration: i64) { unsafe { dma_record_flush(); @@ -297,7 +297,6 @@ extern fn dma_record_stop(name: CSlice, duration: i64) { DMA_RECORDER.active = false; send(&DmaRecordStop { - name: name, duration: duration as u64 }); } diff --git a/artiq/firmware/libproto/kernel_proto.rs b/artiq/firmware/libproto/kernel_proto.rs index cc8ca12ba..d5d6ffbe2 100644 --- a/artiq/firmware/libproto/kernel_proto.rs +++ b/artiq/firmware/libproto/kernel_proto.rs @@ -28,10 +28,9 @@ pub enum Message<'a> { RtioInitRequest, - DmaRecordStart, + DmaRecordStart(&'a str), DmaRecordAppend(&'a [u8]), DmaRecordStop { - name: &'a str, duration: u64 }, diff --git a/artiq/firmware/runtime/rtio_dma.rs b/artiq/firmware/runtime/rtio_dma.rs index e0dc86e41..4a024e929 100644 --- a/artiq/firmware/runtime/rtio_dma.rs +++ b/artiq/firmware/runtime/rtio_dma.rs @@ -8,54 +8,62 @@ const ALIGNMENT: usize = 64; #[derive(Debug)] struct Entry { - data: Vec, - padding: usize, + trace: Vec, + padding_len: usize, duration: u64 } #[derive(Debug)] pub struct Manager { entries: BTreeMap, - recording: Vec + recording_name: String, + recording_trace: Vec } impl Manager { pub fn new() -> Manager { Manager { entries: BTreeMap::new(), - recording: Vec::new() + recording_name: String::new(), + recording_trace: Vec::new(), } } - pub fn record_start(&mut self) { - self.recording = Vec::new(); + pub fn record_start(&mut self, name: &str) { + self.recording_name = String::from(name); + self.recording_trace = Vec::new(); + + // or we could needlessly OOM replacing a large trace + self.entries.remove(name); } pub fn record_append(&mut self, data: &[u8]) { - self.recording.write_all(data).unwrap(); + self.recording_trace.write_all(data).unwrap(); } - pub fn record_stop(&mut self, name: &str, duration: u64) { - let mut recorded = Vec::new(); - mem::swap(&mut self.recording, &mut recorded); - recorded.push(0); - let data_len = recorded.len(); + pub fn record_stop(&mut self, duration: u64) { + let mut trace = Vec::new(); + mem::swap(&mut self.recording_trace, &mut trace); + trace.push(0); + let data_len = trace.len(); // Realign. - recorded.reserve(ALIGNMENT - 1); - let padding = ALIGNMENT - recorded.as_ptr() as usize % ALIGNMENT; + trace.reserve(ALIGNMENT - 1); + let padding = ALIGNMENT - trace.as_ptr() as usize % ALIGNMENT; let padding = if padding == ALIGNMENT { 0 } else { padding }; for _ in 0..padding { // Vec guarantees that this will not reallocate - recorded.push(0) + trace.push(0) } for i in 1..data_len + 1 { - recorded[data_len + padding - i] = recorded[data_len - i] + trace[data_len + padding - i] = trace[data_len - i] } - self.entries.insert(String::from(name), Entry { - data: recorded, - padding: padding, + let mut name = String::new(); + mem::swap(&mut self.recording_name, &mut name); + self.entries.insert(name, Entry { + trace: trace, + padding_len: padding, duration: duration }); } @@ -67,7 +75,7 @@ impl Manager { pub fn with_trace(&self, name: &str, f: F) -> R where F: FnOnce(Option<&[u8]>, u64) -> R { match self.entries.get(name) { - Some(entry) => f(Some(&entry.data[entry.padding..]), entry.duration), + Some(entry) => f(Some(&entry.trace[entry.padding_len..]), entry.duration), None => f(None, 0) } } diff --git a/artiq/firmware/runtime/session.rs b/artiq/firmware/runtime/session.rs index db58ed655..b80b01d1b 100644 --- a/artiq/firmware/runtime/session.rs +++ b/artiq/firmware/runtime/session.rs @@ -394,16 +394,16 @@ fn process_kern_message(io: &Io, mut stream: Option<&mut TcpStream>, kern_acknowledge() } - &kern::DmaRecordStart => { - session.congress.dma_manager.record_start(); + &kern::DmaRecordStart(name) => { + session.congress.dma_manager.record_start(name); kern_acknowledge() } &kern::DmaRecordAppend(data) => { session.congress.dma_manager.record_append(data); kern_acknowledge() } - &kern::DmaRecordStop { name, duration } => { - session.congress.dma_manager.record_stop(name, duration); + &kern::DmaRecordStop { duration } => { + session.congress.dma_manager.record_stop(duration); board::cache::flush_l2_cache(); kern_acknowledge() }