zephyr: work: Change WorkQueue from dynamic to static

Change this declaration from a dynamic allocated type that must never be
dropped to a static declaration.  This is in preparation for a macro to
help with the declaration.

Signed-off-by: David Brown <[email protected]>

Author: d3zd3z

zephyr/src/work.rs

@@ -178,149 +178,105 @@
 extern crate alloc;
 
 use core::{
-    cell::UnsafeCell,
-    ffi::{c_int, c_uint, CStr},
+    cell::{RefCell, UnsafeCell},
+    ffi::{c_int, c_uint},
     future::Future,
     mem,
     pin::Pin,
-    ptr,
+    sync::atomic::Ordering,
     task::Poll,
 };
 
+use critical_section::Mutex;
+use portable_atomic::AtomicBool;
+use portable_atomic_util::Arc;
 use zephyr_sys::{
     k_poll_signal, k_poll_signal_check, k_poll_signal_init, k_poll_signal_raise,
     k_poll_signal_reset, k_work, k_work_init, k_work_q, k_work_queue_config, k_work_queue_init,
-    k_work_queue_start, k_work_submit, k_work_submit_to_queue, ETIMEDOUT,
+    k_work_queue_start, k_work_submit, k_work_submit_to_queue, z_thread_stack_element, ETIMEDOUT,
 };
 
 use crate::{
-    error::to_result_void,
-    kio::ContextExt,
-    object::Fixed,
-    simpletls::SimpleTls,
-    sync::{Arc, Mutex},
-    sys::thread::ThreadStack,
-    time::Timeout,
+    error::to_result_void, kio::ContextExt, object::Fixed, simpletls::SimpleTls, time::Timeout,
 };
 
 pub mod futures;
 
-/// A builder for work queues themselves.
-///
-/// A work queue is a Zephyr thread that instead of directly running a piece of code, manages a work
-/// queue.  Various types of `Work` can be submitted to these queues, along with various types of
-/// triggering conditions.
-pub struct WorkQueueBuilder {
-    /// The "config" value passed in.
+/// A static declaration of a work-queue.  This associates a work queue, with a stack, and an atomic
+/// to determine if it has been initialized.
+// TODO: Remove the pub on the fields, and make a constructor.
+pub struct WorkQueueDecl<const SIZE: usize> {
+    queue: WorkQueue,
+    stack: &'static crate::thread::ThreadStack<SIZE>,
     config: k_work_queue_config,
-    /// Priority for the work queue thread.
     priority: c_int,
+    started: AtomicBool,
 }
 
-impl WorkQueueBuilder {
-    /// Construct a new WorkQueueBuilder with default values.
-    pub fn new() -> Self {
+// SAFETY: Sync is needed here to make a static declaration, despite the `*const i8` that is burried
+// in the config.
+unsafe impl<const SIZE: usize> Sync for WorkQueueDecl<SIZE> {}
+
+impl<const SIZE: usize> WorkQueueDecl<SIZE> {
+    /// Static constructor.  Mostly for use by the macro.
+    pub const fn new(
+        stack: &'static crate::thread::ThreadStack<SIZE>,
+        config: k_work_queue_config,
+        priority: c_int,
+    ) -> Self {
         Self {
-            config: k_work_queue_config {
-                name: ptr::null(),
-                no_yield: false,
-                essential: false,
-            },
-            priority: 0,
+            queue: unsafe { mem::zeroed() },
+            stack,
+            config,
+            priority,
+            started: AtomicBool::new(false),
         }
     }
 
-    /// Set the name for the WorkQueue thread.
-    ///
-    /// This name shows up in debuggers and some analysis tools.
-    pub fn set_name(&mut self, name: &'static CStr) -> &mut Self {
-        self.config.name = name.as_ptr();
-        self
-    }
-
-    /// Set the "no yield" flag for the created worker.
-    ///
-    /// If this is not set, the work queue will call `k_yield` between each enqueued work item.  For
-    /// non-preemptible threads, this will allow other threads to run.  For preemptible threads,
-    /// this will allow other threads at the same priority to run.
-    ///
-    /// This method has a negative in the name, which goes against typical conventions.  This is
-    /// done to match the field in the Zephyr config.
-    pub fn set_no_yield(&mut self, value: bool) -> &mut Self {
-        self.config.no_yield = value;
-        self
-    }
-
-    /// Set the "essential" flag for the created worker.
-    ///
-    /// This sets the essential flag on the running thread.  The system considers the termination of
-    /// an essential thread to be a fatal error.
-    pub fn set_essential(&mut self, value: bool) -> &mut Self {
-        self.config.essential = value;
-        self
-    }
-
-    /// Set the priority for the worker thread.
-    ///
-    /// See the Zephyr docs for the meaning of priority.
-    pub fn set_priority(&mut self, value: c_int) -> &mut Self {
-        self.priority = value;
-        self
-    }
-
-    /// Start the given work queue thread.
-    ///
-    /// TODO: Implement a 'start' that works from a static work queue.
-    pub fn start(&self, stack: ThreadStack) -> WorkQueue {
-        let item: Fixed<k_work_q> = Fixed::new(unsafe { mem::zeroed() });
-        unsafe {
-            // SAFETY: Initialize zeroed memory.
-            k_work_queue_init(item.get());
-
-            // SAFETY: This associates the workqueue with the thread ID that runs it.  The thread is
-            // a pointer into this work item, which will not move, because of the Fixed.
-            let this = &mut *item.get();
-            WORK_QUEUES
-                .lock()
-                .unwrap()
-                .insert(&this.thread, WorkQueueRef(item.get()));
-
-            // SAFETY: Start work queue thread.  The main issue here is that the work queue cannot
-            // be deallocated once the thread has started.  We enforce this by making Drop panic.
-            k_work_queue_start(
-                item.get(),
-                stack.base,
-                stack.size,
-                self.priority,
-                &self.config,
-            );
-        }
-
-        WorkQueue { item }
+    /// Start the work queue thread, if needed, and return a reference to it.
+    pub fn start(&'static self) -> &'static WorkQueue {
+        critical_section::with(|cs| {
+            if self.started.load(Ordering::Relaxed) {
+                // Already started, just return it.
+                return &self.queue;
+            }
+
+            // SAFETY: Starting is coordinated by the atomic, as well as being protected in a
+            // critical section.
+            unsafe {
+                let this = &mut *self.queue.item.get();
+
+                k_work_queue_init(self.queue.item.get());
+
+                // Add to the WORK_QUEUES data.  That needs to be changed to a critical
+                // section Mutex from a Zephyr Mutex, as that would deadlock if called while in a
+                // critrical section.
+                let mut tls = WORK_QUEUES.borrow_ref_mut(cs);
+                tls.insert(&this.thread, WorkQueueRef(self.queue.item.get()));
+
+                // Start the work queue thread.
+                k_work_queue_start(
+                    self.queue.item.get(),
+                    self.stack.data.get() as *mut z_thread_stack_element,
+                    self.stack.size(),
+                    self.priority,
+                    &self.config,
+                );
+            }
+
+            &self.queue
+        })
     }
 }
 
 /// A running work queue thread.
 ///
-/// # Panic
-///
-/// Allowing a work queue to drop will result in a panic.  There are two ways to handle this,
-/// depending on whether the WorkQueue is in a Box, or an Arc:
-/// ```
-/// // Leak a work queue in an Arc.
-/// let wq = Arc::new(WorkQueueBuilder::new().start(...));
-/// // If the Arc is used after this:
-/// let _ = Arc::into_raw(wq.clone());
-/// // If the Arc is no longer needed:
-/// let _ = Arc::into_raw(wq);
-///
-/// // Leak a work queue in a Box.
-/// let wq = Box::new(WorkQueueBuilder::new().start(...));
-/// let _ = Box::leak(wq);
-/// ```
+/// This must be declared statically, and initialized once.  Please see the macro
+/// [`define_work_queue`] which declares this with a [`StaticWorkQueue`] to help with the
+/// association with a stack, and making sure the queue is only started once.
 pub struct WorkQueue {
     #[allow(dead_code)]
-    item: Fixed<k_work_q>,
+    item: UnsafeCell<k_work_q>,
 }
 
 /// Work queues can be referenced from multiple threads, and thus are Send and Sync.
@@ -348,7 +304,8 @@ impl Drop for WorkQueue {
 ///
 /// This is a little bit messy as we don't have a lazy mechanism, so we have to handle this a bit
 /// manually right now.
-static WORK_QUEUES: Mutex<SimpleTls<WorkQueueRef>> = Mutex::new(SimpleTls::new());
+static WORK_QUEUES: Mutex<RefCell<SimpleTls<WorkQueueRef>>> =
+    Mutex::new(RefCell::new(SimpleTls::new()));
 
 /// For the queue mapping, we need a simple wrapper around the underlying pointer, one that doesn't
 /// implement stop.
@@ -361,7 +318,7 @@ unsafe impl Sync for WorkQueueRef {}
 
 /// Retrieve the current work queue, if we are running within one.
 pub fn get_current_workq() -> Option<*mut k_work_q> {
-    WORK_QUEUES.lock().unwrap().get().map(|wq| wq.0)
+    critical_section::with(|cs| WORK_QUEUES.borrow_ref(cs).get().map(|wq| wq.0))
 }
 
 /// A Rust wrapper for `k_poll_signal`.
@@ -628,7 +585,10 @@ impl<T: SimpleAction + Send> Work<T> {
     /// Submit this work to a specified work queue.
     ///
     /// TODO: Change when we have better wrappers for work queues.
-    pub fn submit_to_queue(this: Pin<Arc<Self>>, queue: &WorkQueue) -> crate::Result<SubmitResult> {
+    pub fn submit_to_queue(
+        this: Pin<Arc<Self>>,
+        queue: &'static WorkQueue,
+    ) -> crate::Result<SubmitResult> {
         let work = this.work.get();
 
         // "leak" the arc to give to C.  We'll reconstruct it in the handler.