DMA: erase trace before re-recording it.

Or we could needlessly OOM replacing a large trace.

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)
 
 

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<u8>) {
+    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<u8>, 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<u8>, duration: i64) {
 
         DMA_RECORDER.active = false;
         send(&DmaRecordStop {
-            name:     name,
             duration: duration as u64
         });
     }

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
     },
 

artiq/firmware/runtime/rtio_dma.rs

@@ -8,54 +8,62 @@ const ALIGNMENT: usize = 64;
 
 #[derive(Debug)]
 struct Entry {
-    data: Vec<u8>,
-    padding: usize,
+    trace: Vec<u8>,
+    padding_len: usize,
     duration: u64
 }
 
 #[derive(Debug)]
 pub struct Manager {
     entries: BTreeMap<String, Entry>,
-    recording: Vec<u8>
+    recording_name: String,
+    recording_trace: Vec<u8>
 }
 
 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<F, R>(&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)
         }
     }

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()
             }