feat(kernel): wake tasks on service registration (#268)

Depends on #267

Currently, the `Registry::connect_*` methods return an error immediately
if the requested service is not found in the registry. Most client types
implement a `from_registry` method which retry in a loop when the
requested service is not found in the registry. These methods will wait
for a fixed (short) amount of time and then try again until the registry
returns the service.

This approach is quite inefficient, as we have to run a bunch of retry
loops that keep trying to access a service that may not be there. This
may happen several times before the service actually is registered,
especially when registering a service requires connecting to another
service.

This branch improves the efficiency of waiting for a service to be
registered. Now, rather than retrying with a fixed-duration sleep, we
instead have the `Registry` own a `WaitCell` which is woken whenever a
new service is registered. This wakes all takes potentially waiting to
connect, allowing them to re-check whether the service they want is in
the registry. This idea was initially proposed by @jamesmunns in a
[comment] on PR #259

Connections are now established using either `Registry::connect`, which
retries whenever a new service is registered, or `Registry::try_connect`
which never retries.

Additionally, we now have the capacity to indicate that a service is not
found *and* that the registry is full, by closing the `WaitCell`. In
this case, retrying will never succeed, because the registry is full and
if the service isn't already there, it will never be added. In this
case, the retrying methods will also return an error, rather than never
completing, so we avoid a potential task leak.

In order to make this change, we need to move the `RwLock` from being
around the entire `Registry` to being inside the registry, around
`items`. This allows the `WaitCell` to be accessed regardless. It also
allows us to shorten the duration for which the lock is held. This
requires changing all methods on `Registry` to take `&self`.
Therefore, I've removed the wrapper methods on `Kernel` for connecting
and registering, since they can now just be called on `kernel.registry`
without a bunch of extra boilerplate for lock management. I've also
simplified the API surface of the registry a bit by removing the
`connect` methods that don't take a `Hello`, and just using
`Registry::connect(())` in those cases. IMO, those methods weren't
really pulling their weight, and they required us to have a method named
`Registry::try_connect_userspace_with_hello` if we were going to add a
non-retrying `connect` variant. Now, we can just have
`Registry::try_connect_userspace`, `Registry::connect_userspace`,
`Registry::connect`, and `Registry::try_connect`, which feels much less
egregious.

[comment]:
#258 (comment)

platforms/allwinner-d1/boards/src/bin/mq-pro.rs

@@ -88,7 +88,9 @@ fn main() -> ! {
         .initialize(async move {
             // i2c_puppet demo: print each keypress to the console.
             i2c_puppet_up.await.unwrap();
-            let mut i2c_puppet = I2cPuppetClient::from_registry(d1.kernel).await;
+            let mut i2c_puppet = I2cPuppetClient::from_registry(d1.kernel)
+                .await
+                .expect("no i2c_puppet driver");
             tracing::info!("got i2c puppet client");
 
             let mut keys = i2c_puppet
@@ -108,7 +110,9 @@ fn main() -> ! {
             // LED to display useful information, but this is fun for now.
             let mut hue = 0;
 
-            let mut i2c_puppet = I2cPuppetClient::from_registry(d1.kernel).await;
+            let mut i2c_puppet = I2cPuppetClient::from_registry(d1.kernel)
+                .await
+                .expect("no i2c_puppet driver");
 
             i2c_puppet
                 .toggle_led(true)

platforms/allwinner-d1/core/src/drivers/sharp_display.rs

@@ -30,15 +30,15 @@ use embedded_graphics::{pixelcolor::Gray8, prelude::*, primitives::Rectangle};
 use kernel::{
     maitake::sync::{Mutex, WaitQueue},
     mnemos_alloc::containers::{Arc, FixedVec},
-    registry::listener,
+    registry::{self, listener},
     services::emb_display::{
         DisplayMetadata, EmbDisplayService, FrameChunk, FrameError, FrameKind, MonoChunk, Request,
         Response,
     },
     Kernel,
 };
 
-use crate::spim::SpiSenderClient;
+use crate::spim::{SpiSender, SpiSenderClient};
 
 const WIDTH: usize = 400;
 const HEIGHT: usize = 240;
@@ -70,6 +70,15 @@ mod commands {
 /// Implements the [`EmbDisplayService`] service interface
 pub struct SharpDisplay;
 
+#[derive(Debug)]
+pub enum RegistrationError {
+    /// Failed to register a display: either the kernel reported that there is
+    /// already an existing EmbDisplay, or the registry is full.
+    Registration(registry::RegistrationError),
+    /// No SPI sender service exists.
+    NoSpiSender(registry::ConnectError<SpiSender>),
+}
+
 impl SharpDisplay {
     pub const WIDTH: usize = WIDTH;
     pub const HEIGHT: usize = HEIGHT;
@@ -78,7 +87,22 @@ impl SharpDisplay {
     ///
     /// Registration will also start the simulated display, meaning that the display
     /// window will appear.
-    pub async fn register(kernel: &'static Kernel) -> Result<(), FrameError> {
+    pub async fn register(kernel: &'static Kernel) -> Result<(), RegistrationError> {
+        // acquire a SPI client first, so that we don't register the display
+        // service unless we can get a SPI client.
+        let spim = SpiSenderClient::from_registry(kernel)
+            .await
+            .map_err(RegistrationError::NoSpiSender)?;
+
+        // bind a listener
+        let cmd = kernel
+            .registry()
+            .bind_konly(2)
+            .await
+            .map_err(RegistrationError::Registration)?
+            .into_request_stream(2)
+            .await;
+
         let linebuf = FixedVec::new(FRAME_BYTES).await;
 
         let ctxt = Arc::new(Mutex::new(Context {
@@ -87,12 +111,6 @@ impl SharpDisplay {
         }))
         .await;
 
-        let cmd = kernel
-            .bind_konly_service(2)
-            .await
-            .map_err(|_| FrameError::DisplayAlreadyExists)?
-            .into_request_stream(2)
-            .await;
         let commander = CommanderTask {
             cmd,
             ctxt: ctxt.clone(),
@@ -107,7 +125,7 @@ impl SharpDisplay {
         let draw = Draw {
             kernel,
             buf: linebuf,
-            spim: SpiSenderClient::from_registry(kernel).await.unwrap(),
+            spim,
             ctxt,
         };
 

platforms/allwinner-d1/core/src/drivers/spim.rs

@@ -110,7 +110,8 @@ impl SpiSenderServer {
         queued: usize,
     ) -> Result<(), registry::RegistrationError> {
         let reqs = kernel
-            .bind_konly_service::<SpiSender>(queued)
+            .registry()
+            .bind_konly::<SpiSender>(queued)
             .await?
             .into_request_stream(queued)
             .await;
@@ -225,7 +226,18 @@ impl SpiSenderClient {
     pub async fn from_registry(
         kernel: &'static Kernel,
     ) -> Result<SpiSenderClient, registry::ConnectError<SpiSender>> {
-        let hdl = kernel.registry().await.connect().await?;
+        let hdl = kernel.registry().connect(()).await?;
+
+        Ok(SpiSenderClient {
+            hdl,
+            osc: Reusable::new_async().await,
+        })
+    }
+
+    pub async fn from_registry_no_retry(
+        kernel: &'static Kernel,
+    ) -> Result<SpiSenderClient, registry::ConnectError<SpiSender>> {
+        let hdl = kernel.registry().try_connect(()).await?;
 
         Ok(SpiSenderClient {
             hdl,

platforms/allwinner-d1/core/src/drivers/twi.rs

@@ -305,7 +305,8 @@ impl I2c0 {
         queued: usize,
     ) -> Result<(), registry::RegistrationError> {
         let rx = kernel
-            .bind_konly_service::<I2cService>(queued)
+            .registry()
+            .bind_konly::<I2cService>(queued)
             .await?
             .into_request_stream(queued)
             .await;

platforms/allwinner-d1/core/src/drivers/uart.rs

@@ -184,7 +184,8 @@ impl D1Uart {
         let (fifo_a, fifo_b) = new_bidi_channel(cap_in, cap_out).await;
 
         let reqs = k
-            .bind_konly_service::<SimpleSerialService>(4)
+            .registry()
+            .bind_konly::<SimpleSerialService>(4)
             .await?
             .into_request_stream(4)
             .await;

platforms/beepy/src/i2c_puppet.rs

@@ -15,10 +15,10 @@ use kernel::{
     registry::{self, Envelope, KernelHandle, RegisteredDriver},
     retry::{AlwaysRetry, ExpBackoff, Retry, WithMaxRetries},
     services::{
-        i2c::{I2cClient, I2cError},
+        i2c::{I2cClient, I2cError, I2cService},
         keyboard::{
             key_event::{self, KeyEvent, Modifiers},
-            mux::KeyboardMuxClient,
+            mux::{KeyboardMuxClient, KeyboardMuxService},
         },
     },
     tracing::{self, instrument, Instrument, Level},
@@ -108,19 +108,15 @@ impl I2cPuppetClient {
     ///
     /// If the [`I2cPuppetService`] hasn't been registered yet, we will retry until it
     /// has been registered.
-    pub async fn from_registry(kernel: &'static Kernel) -> Self {
-        loop {
-            match I2cPuppetClient::from_registry_no_retry(kernel).await {
-                Ok(port) => return port,
-                Err(registry::ConnectError::Rejected(_)) => {
-                    unreachable!("the KeyboardMuxService does not return connect errors!")
-                }
-                Err(_) => {
-                    // I2C probably isn't registered yet. Try again in a bit
-                    kernel.sleep(Duration::from_millis(10)).await;
-                }
-            }
-        }
+    pub async fn from_registry(
+        kernel: &'static Kernel,
+    ) -> Result<Self, registry::ConnectError<I2cPuppetService>> {
+        let handle = kernel.registry().connect(()).await?;
+
+        Ok(I2cPuppetClient {
+            handle,
+            reply: Reusable::new_async().await,
+        })
     }
 
     /// Obtain an `I2cPuppetClient`
@@ -132,11 +128,7 @@ impl I2cPuppetClient {
     pub async fn from_registry_no_retry(
         kernel: &'static Kernel,
     ) -> Result<Self, registry::ConnectError<I2cPuppetService>> {
-        let handle = kernel
-            .registry()
-            .await
-            .connect::<I2cPuppetService>()
-            .await?;
+        let handle = kernel.registry().try_connect(()).await?;
 
         Ok(I2cPuppetClient {
             handle,
@@ -243,6 +235,8 @@ pub struct I2cPuppetServer {
 #[derive(Debug)]
 pub enum RegistrationError {
     Registry(registry::RegistrationError),
+    NoI2c(registry::ConnectError<I2cService>),
+    NoKeymux(registry::ConnectError<KeyboardMuxService>),
     NoI2cPuppet(I2cError),
     InvalidSettings(&'static str),
 }
@@ -351,7 +345,9 @@ impl I2cPuppetServer {
         irq_waker: impl Into<Option<&'static WaitCell>>,
     ) -> Result<(), RegistrationError> {
         let keymux = if settings.keymux {
-            let keymux = KeyboardMuxClient::from_registry(kernel).await;
+            let keymux = KeyboardMuxClient::from_registry(kernel)
+                .await
+                .map_err(RegistrationError::NoKeymux)?;
             tracing::debug!("acquired keyboard mux client");
             Some(keymux)
         } else {
@@ -362,7 +358,10 @@ impl I2cPuppetServer {
             // The longest read or write operation we will perform is two bytes
             // long. Thus, we can reuse a single 2-byte buffer forever.
             let buf = FixedVec::new(2).await;
-            I2cClient::from_registry(kernel).await.with_cached_buf(buf)
+            I2cClient::from_registry(kernel)
+                .await
+                .map_err(RegistrationError::NoI2c)?
+                .with_cached_buf(buf)
         };
         let subscriptions = FixedVec::new(settings.max_subscriptions).await;
 
@@ -390,7 +389,8 @@ impl I2cPuppetServer {
             .map_err(RegistrationError::NoI2cPuppet)?;
 
         let rx = kernel
-            .bind_konly_service(settings.channel_capacity)
+            .registry()
+            .bind_konly(settings.channel_capacity)
             .await
             .map_err(RegistrationError::Registry)?
             .into_request_stream(settings.channel_capacity)

platforms/esp32c3-buddy/src/drivers/uart.rs

@@ -122,7 +122,8 @@ impl<T: Instance> C3Uart<T> {
         let old = UART_RX.swap(leaked_prod, Ordering::AcqRel);
         assert_eq!(old, null_mut());
 
-        k.register_konly::<SimpleSerialService>(registration)
+        k.registry()
+            .register_konly::<SimpleSerialService>(registration)
             .await?;
 
         Ok(())

platforms/esp32c3-buddy/src/drivers/usb_serial.rs

@@ -181,7 +181,8 @@ impl UsbSerialServer {
 
         k.spawn(self.worker(fifo_a)).await;
 
-        k.register_konly::<SimpleSerialService>(registration)
+        k.registry()
+            .register_konly::<SimpleSerialService>(registration)
             .await?;
 
         Ok(())

platforms/melpomene/src/sim_drivers/emb_display.rs

@@ -36,8 +36,7 @@ use mnemos_kernel::{
     registry,
     services::{
         emb_display::{
-            DisplayMetadata, EmbDisplayService, FrameChunk, FrameError, FrameKind, MonoChunk,
-            Request, Response,
+            DisplayMetadata, EmbDisplayService, FrameChunk, FrameKind, MonoChunk, Request, Response,
         },
         keyboard::{
             key_event::{self, KeyCode, Modifiers},
@@ -63,12 +62,12 @@ impl SimDisplay {
         settings: DisplayConfig,
         width: u32,
         height: u32,
-    ) -> Result<(), FrameError> {
+    ) -> Result<(), registry::RegistrationError> {
         tracing::debug!("initializing SimDisplay server ({width}x{height})...");
         let cmd = kernel
-            .bind_konly_service(settings.kchannel_depth)
-            .await
-            .map_err(|_| FrameError::DisplayAlreadyExists)?
+            .registry()
+            .bind_konly(settings.kchannel_depth)
+            .await?
             .into_request_stream(settings.kchannel_depth)
             .await;
 
@@ -245,7 +244,9 @@ async fn render_loop(
     frames_per_second: usize,
 ) {
     let mut idle_ticks = 0;
-    let mut keymux = KeyboardMuxClient::from_registry(kernel).await;
+    let mut keymux = KeyboardMuxClient::from_registry(kernel)
+        .await
+        .expect("no keyboard mux service!");
     let mut first_done = false;
     let sleep_time = Duration::from_micros(1_000_000 / (frames_per_second as u64));
     loop {

platforms/melpomene/src/sim_drivers/tcp_serial.rs

@@ -26,7 +26,8 @@ impl TcpSerial {
         let (a_ring, b_ring) =
             new_bidi_channel(settings.incoming_size, settings.outgoing_size).await;
         let reqs = kernel
-            .bind_konly_service::<SimpleSerialService>(settings.kchannel_depth)
+            .registry()
+            .bind_konly::<SimpleSerialService>(settings.kchannel_depth)
             .await?
             .into_request_stream(settings.kchannel_depth)
             .await;

platforms/pomelo/src/sim_drivers/emb_display.rs

@@ -39,7 +39,7 @@ use mnemos_kernel::{
         },
         keyboard::{
             key_event::{self, KeyCode, Modifiers},
-            mux::KeyboardMuxClient,
+            mux::{KeyboardMuxClient, KeyboardMuxService},
             KeyEvent,
         },
     },
@@ -77,6 +77,13 @@ async fn draw_complete_handler(rx: KConsumer<DrawCompleteData>) {
         }
     }
 }
+
+#[derive(Debug)]
+pub enum RegistrationError {
+    Register(registry::RegistrationError),
+    NoKeymux(registry::ConnectError<KeyboardMuxService>),
+}
+
 impl SimDisplay {
     /// Register the driver instance
     ///
@@ -87,25 +94,29 @@ impl SimDisplay {
         kernel: &'static Kernel,
         width: u32,
         height: u32,
-    ) -> Result<(), FrameError> {
+    ) -> Result<(), RegistrationError> {
         tracing::debug!("initializing SimDisplay server ({width}x{height})...");
 
+        let keymux = KeyboardMuxClient::from_registry(kernel)
+            .await
+            .map_err(RegistrationError::NoKeymux)?;
+
+        let cmd = kernel
+            .registry()
+            .bind_konly(2)
+            .await
+            .map_err(RegistrationError::Register)?
+            .into_request_stream(2)
+            .await;
+
         // TODO settings.kchannel_depth
-        let (listener, registration) = registry::Listener::new(2).await;
-        let cmd = listener.into_request_stream(2).await;
         let commander = CommanderTask { cmd, width, height };
 
         kernel.spawn(commander.run(kernel, width, height)).await;
 
         let (key_tx, key_rx) = KChannel::new_async(32).await.split();
-        let keymux = KeyboardMuxClient::from_registry(kernel).await;
         kernel.spawn(key_event_handler(key_rx, keymux)).await;
 
-        kernel
-            .register_konly::<EmbDisplayService>(registration)
-            .await
-            .map_err(|_| FrameError::DisplayAlreadyExists)?;
-
         // listen for key events
         let on_keydown = Closure::<dyn FnMut(_)>::new(move |event: web_sys::KeyboardEvent| {
             event.prevent_default();