Non atomic RTT

CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+- [#699]: `defmt-rtt`: Non atomic RTT
 - [#695]: `defmt-rtt`: Refactor rtt [3/2]
 - [#689]: `defmt-rtt`: Update to critical-section 1.0
 - [#692]: `defmt-macros`: Wrap const fn in const item to ensure compile-time-evaluation.
@@ -21,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [#679]: `CI`: Add changelog enforcer
 - [#678]: Satisfy clippy
 
+[#699]: https://github.com/knurling-rs/defmt/pull/699
 [#695]: https://github.com/knurling-rs/defmt/pull/695
 [#692]: https://github.com/knurling-rs/defmt/pull/692
 [#690]: https://github.com/knurling-rs/defmt/pull/690

firmware/defmt-rtt/build.rs

@@ -15,7 +15,14 @@ fn main() {
 
  std::fs::write(
  out_file_path,
- format!("pub(crate) const BUF_SIZE: usize = {};", size),
+ format!(
+ "/// RTT buffer size (default: 1024).
+ ///
+ /// Can be customized by setting the `DEFMT_RTT_BUFFER_SIZE` environment variable.
+ /// Use a power of 2 for best performance.
+ pub(crate) const BUF_SIZE: usize = {};",
+ size
+ ),
  )
  .unwrap();
 }

firmware/defmt-rtt/src/channel.rs

@@ -1,7 +1,4 @@
-use core::{
- ptr,
- sync::atomic::{AtomicUsize, Ordering},
-};
+use core::{cell::RefCell, ptr};
 
 use crate::{consts::BUF_SIZE, MODE_BLOCK_IF_FULL, MODE_MASK};
 
@@ -13,13 +10,13 @@ pub(crate) struct Channel {
  pub buffer: *mut u8,
  pub size: usize,
  /// Written by the target.
- pub write: AtomicUsize,
+ pub write: critical_section::Mutex<RefCell<usize>>,
  /// Written by the host.
- pub read: AtomicUsize,
+ pub read: critical_section::Mutex<RefCell<usize>>,
  /// Channel properties.
  ///
  /// Currently, only the lowest 2 bits are used to set the channel mode (see constants below).
- pub flags: AtomicUsize,
+ pub flags: critical_section::Mutex<RefCell<usize>>,
 }
 
 impl Channel {
@@ -44,9 +41,9 @@ impl Channel {
  return 0;
  }
 
+ let (read, write) = critical_section::with(|cs| (self.read.take(cs), self.write.take(cs)));
+
  // calculate how much space is left in the buffer
- let read = self.read.load(Ordering::Relaxed);
- let write = self.write.load(Ordering::Acquire);
  let available = available_buffer_size(read, write);
 
  // abort if buffer is full
@@ -58,7 +55,7 @@ impl Channel {
  }
 
  fn nonblocking_write(&self, bytes: &[u8]) -> usize {
- let write = self.write.load(Ordering::Acquire);
+ let write = critical_section::with(|cs| self.write.take(cs));
 
  // NOTE truncate at BUF_SIZE to avoid more than one "wrap-around" in a single `write` call
  self.write_impl(bytes, write, BUF_SIZE)
@@ -81,8 +78,9 @@ impl Channel {
  }
 
  // adjust the write pointer, so the host knows that there is new data
- self.write
- .store(cursor.wrapping_add(len) % BUF_SIZE, Ordering::Release);
+ critical_section::with(|cs| {
+ *self.write.borrow_ref_mut(cs) = cursor.wrapping_add(len) % BUF_SIZE
+ });
 
  // return the number of bytes written
  len
@@ -95,14 +93,19 @@ impl Channel {
  }
 
  // busy wait, until the read- catches up with the write-pointer
- let read = || self.read.load(Ordering::Relaxed);
- let write = || self.write.load(Ordering::Relaxed);
- while read() != write() {}
+ while {
+ critical_section::with(|cs| {
+ let read = self.read.take(cs);
+ let write = self.write.take(cs);
+ read != write
+ })
+ } {}
  }
 
  fn host_is_connected(&self) -> bool {
  // we assume that a host is connected if we are in blocking-mode. this is what probe-run does.
- self.flags.load(Ordering::Relaxed) & MODE_MASK == MODE_BLOCK_IF_FULL
+ let flags = critical_section::with(|cs| self.flags.take(cs));
+ flags & MODE_MASK == MODE_BLOCK_IF_FULL
  }
 }
 

firmware/defmt-rtt/src/lib.rs

@@ -34,22 +34,18 @@
 #![no_std]
 
 mod channel;
-
-use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
-
-use crate::channel::Channel;
-
 mod consts;
 
-/// RTT buffer size. Default: 1024; can be customized by setting the `DEFMT_RTT_BUFFER_SIZE` environment variable.
-/// Use a power of 2 for best performance.
-use crate::consts::BUF_SIZE;
+use core::cell::RefCell;
+
+use crate::{channel::Channel, consts::BUF_SIZE};
 
 #[defmt::global_logger]
 struct Logger;
 
 /// Global logger lock.
-static TAKEN: AtomicBool = AtomicBool::new(false);
+static TAKEN: critical_section::Mutex<RefCell<bool>> =
+ critical_section::Mutex::new(RefCell::new(false));
 static mut CS_RESTORE: critical_section::RestoreState = critical_section::RestoreState::invalid();
 static mut ENCODER: defmt::Encoder = defmt::Encoder::new();
 
@@ -58,12 +54,14 @@ unsafe impl defmt::Logger for Logger {
  // safety: Must be paired with corresponding call to release(), see below
  let restore = unsafe { critical_section::acquire() };
 
- if TAKEN.load(Ordering::Relaxed) {
- panic!("defmt logger taken reentrantly")
- }
-
- // no need for CAS because interrupts are disabled
- TAKEN.store(true, Ordering::Relaxed);
+ critical_section::with(|cs| {
+ // safety: accessing the `static mut` is OK because we have acquired a critical section.
+ let mut taken = TAKEN.borrow_ref_mut(cs);
+ if *taken {
+ panic!("defmt logger taken reentrantly")
+ }
+ *taken = true;
+ });
 
  // safety: accessing the `static mut` is OK because we have acquired a critical section.
  unsafe { CS_RESTORE = restore };
@@ -73,15 +71,15 @@ unsafe impl defmt::Logger for Logger {
  }
 
  unsafe fn flush() {
- // SAFETY: if we get here, the global logger mutex is currently acquired
+ // safety: accessing the `&'static _` is OK because we have acquired a critical section.
  handle().flush();
  }
 
  unsafe fn release() {
  // safety: accessing the `static mut` is OK because we have acquired a critical section.
  ENCODER.end_frame(do_write);
 
- TAKEN.store(false, Ordering::Relaxed);
+ critical_section::with(|cs| *TAKEN.borrow_ref_mut(cs) = false);
 
  // safety: accessing the `static mut` is OK because we have acquired a critical section.
  let restore = CS_RESTORE;
@@ -132,9 +130,9 @@ unsafe fn handle() -> &'static Channel {
  name: &NAME as *const _ as *const u8,
  buffer: unsafe { &mut BUFFER as *mut _ as *mut u8 },
  size: BUF_SIZE,
- write: AtomicUsize::new(0),
- read: AtomicUsize::new(0),
- flags: AtomicUsize::new(MODE_NON_BLOCKING_TRIM),
+ write: critical_section::Mutex::new(RefCell::new(0)),
+ read: critical_section::Mutex::new(RefCell::new(0)),
+ flags: critical_section::Mutex::new(RefCell::new(MODE_NON_BLOCKING_TRIM)),
  },
  };