[PATCH v1 1/4] rust: hrtimer: Introduce HrTimerArc to make arming exclusive

From: FUJITA Tomonori <[email protected]>

A hrtimer callback may change its own expiry time with
hrtimer_forward(), which is sound only because __run_hrtimer() dequeues
the timer for the duration of the callback. Arming the same timer from
another CPU puts it back into the rbtree while the callback runs, so
that expiry update lands on a queued timer and the tree ordering goes
stale. Arc<T> is Clone and Arc<T>::start() is safe, so safe Rust can arm
a timer on any CPU, including while its callback is running.

"No arming while the callback runs" cannot be expressed in the type
system, because the callback begins when the timer expires rather than
at any point in the Rust program, so use the stronger "no arming while
armed" instead. hrtimer_cancel() waits for the handler to return, which
makes that the point where the right to arm can be handed back.
Pin<Box<T, A>> and Pin<&mut T> already work this way through ownership
and exclusive borrow.

Split the right to arm out of Arc<T> into HrTimerArc<T>, a non-clonable
wrapper modelled on ListArc that is created from a Pin<UniqueArc<T>> and
consumed by start(); the object stays shareable through plain Arc
references.

Fixes: 3f2a5ba784b8 ("rust: hrtimer: Add HrTimerCallbackContext and ::forward()")
Signed-off-by: FUJITA Tomonori <[email protected]>
---
 rust/kernel/time/hrtimer.rs     |  33 +++++-----
 rust/kernel/time/hrtimer/arc.rs | 111 +++++++++++++++++++++++++-------
 2 files changed, 103 insertions(+), 41 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 2d7f1131a813..a7587db1d552 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -172,12 +172,12 @@
 //! #     sync::{
 //! #         atomic::{ordering, Atomic},
 //! #         completion::Completion,
-//! #         Arc, ArcBorrow,
+//! #         ArcBorrow,
 //! #     },
 //! #     time::{
 //! #         hrtimer::{
-//! #             RelativeMode, HrTimer, HrTimerCallback, HrTimerPointer, HrTimerRestart,
-//! #             HasHrTimer, HrTimerCallbackContext
+//! #             RelativeMode, HrTimer, HrTimerArc, HrTimerCallback, HrTimerPointer,
+//! #             HrTimerRestart, HasHrTimer, HrTimerCallbackContext
 //! #         },
 //! #         Delta, Monotonic,
 //! #     },
@@ -204,7 +204,7 @@
 //! }
 //!
 //! impl HrTimerCallback for ArcIntrusiveHrTimer {
-//!     type Pointer<'a> = Arc<Self>;
+//!     type Pointer<'a> = HrTimerArc<Self>;
 //!
 //!     fn run(
 //!         this: ArcBorrow<'_, Self>,
@@ -229,11 +229,12 @@
 //!     }
 //! }
 //!
-//! let has_timer = Arc::pin_init(ArcIntrusiveHrTimer::new(), GFP_KERNEL)?;
-//! let _handle = has_timer.clone().start(Delta::from_micros(200));
+//! let has_timer = HrTimerArc::pin_init(ArcIntrusiveHrTimer::new(), GFP_KERNEL)?;
+//! let shared = has_timer.clone_arc();
+//! let _handle = has_timer.start(Delta::from_micros(200));
 //!
-//! while has_timer.flag.load(ordering::Relaxed) != 5 {
-//!     has_timer.cond.wait_for_completion();
+//! while shared.flag.load(ordering::Relaxed) != 5 {
+//!     shared.cond.wait_for_completion();
 //! }
 //!
 //! pr_info!("Counted to 5\n");
@@ -589,18 +590,17 @@ pub fn expires(&self) -> HrTimerInstant<T>
 /// `Self` must be [`Sync`] because it is passed to timer callbacks in another
 /// thread of execution (hard or soft interrupt context).
 ///
-/// Starting a timer returns a [`HrTimerHandle`] that can be used to manipulate
-/// the timer. Note that it is OK to call the start function repeatedly, and
-/// that more than one [`HrTimerHandle`] associated with a [`HrTimerPointer`] may
-/// exist. A timer can be manipulated through any of the handles, and a handle
-/// may represent a cancelled timer.
+/// Starting a timer consumes `Self` and returns a [`HrTimerHandle`] that can be
+/// used to manipulate the timer. As a timer in the **started** or **running**
+/// state cannot be started again, at most one [`HrTimerHandle`] for a timer
+/// exists at a time. A handle may represent a cancelled timer.
 pub trait HrTimerPointer: Sync + Sized {
     /// The operational mode associated with this timer.
     ///
     /// This defines how the expiration value is interpreted.
     type TimerMode: HrTimerMode;
 
-    /// A handle representing a started or restarted timer.
+    /// A handle representing a started timer.
     ///
     /// If the timer is running or if the timer callback is executing when the
     /// handle is dropped, the drop method of [`HrTimerHandle`] should not return
@@ -610,8 +610,7 @@ pub trait HrTimerPointer: Sync + Sized {
     /// leak the handle.
     type TimerHandle: HrTimerHandle;
 
-    /// Start the timer with expiry after `expires` time units. If the timer was
-    /// already running, it is restarted with the new expiry time.
+    /// Start the timer with expiry after `expires` time units.
     fn start(self, expires: <Self::TimerMode as HrTimerMode>::Expires) -> Self::TimerHandle;
 }
 
@@ -1110,7 +1109,7 @@ unsafe fn timer_container_of(
 }
 
 mod arc;
-pub use arc::ArcHrTimerHandle;
+pub use arc::{ArcHrTimerHandle, HrTimerArc};
 mod pin;
 pub use pin::PinHrTimerHandle;
 mod pin_mut;
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 7be82bcb352a..2134d12d558c 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -8,47 +8,71 @@
 use super::HrTimerMode;
 use super::HrTimerPointer;
 use super::RawHrTimerCallback;
-use crate::sync::Arc;
-use crate::sync::ArcBorrow;
+use crate::alloc::Flags;
+use crate::error::{Error, Result};
+use crate::init::InPlaceInit;
+use crate::sync::{Arc, ArcBorrow, UniqueArc};
+use core::pin::Pin;
+use pin_init::PinInit;
 
-/// A handle for an `Arc<HasHrTimer<T>>` returned by a call to
-/// [`HrTimerPointer::start`].
-pub struct ArcHrTimerHandle<T>
+/// A wrapper around [`Arc`] that's guaranteed unique.
+///
+/// The `HrTimerArc` type can be thought of as a special reference to a refcounted object that owns
+/// the permission to arm the [`HrTimer`] stored in the refcounted object. By ensuring that each
+/// object has only one `HrTimerArc` reference, the owner of that reference is assured exclusive
+/// access to the arming operation. When a timer is started, the returned [`ArcHrTimerHandle`] takes
+/// ownership of the `HrTimerArc` reference.
+///
+/// While this `HrTimerArc` is unique, there still might exist normal [`Arc`] references to the
+/// object. Use [`HrTimerArc::clone_arc`] to obtain one.
+///
+/// # Invariants
+///
+/// * Each reference counted object has at most one `HrTimerArc`.
+pub struct HrTimerArc<T>
 where
     T: HasHrTimer<T>,
 {
-    pub(crate) inner: Arc<T>,
+    arc: Arc<T>,
 }
 
-// SAFETY: We implement drop below, and we cancel the timer in the drop
-// implementation.
-unsafe impl<T> HrTimerHandle for ArcHrTimerHandle<T>
+impl<T> HrTimerArc<T>
 where
     T: HasHrTimer<T>,
 {
-    fn cancel(&mut self) -> bool {
-        let self_ptr = Arc::as_ptr(&self.inner);
-
-        // SAFETY: As we obtained `self_ptr` from a valid reference above, it
-        // must point to a valid `T`.
-        let timer_ptr = unsafe { <T as HasHrTimer<T>>::raw_get_timer(self_ptr) };
+    /// Use the given pin-initializer to pin-initialize a `T` inside of a new `HrTimerArc`.
+    #[inline]
+    pub fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self>
+    where
+        Error: From<E>,
+    {
+        Ok(Self::from(UniqueArc::pin_init(init, flags)?))
+    }
 
-        // SAFETY: As `timer_ptr` points into `T` and `T` is valid, `timer_ptr`
-        // must point to a valid `HrTimer` instance.
-        unsafe { HrTimer::<T>::raw_cancel(timer_ptr) }
+    /// Clone an [`Arc`] from this `HrTimerArc`.
+    ///
+    /// The returned [`Arc`] can be used to access the object, but not to arm its timer.
+    #[inline]
+    pub fn clone_arc(&self) -> Arc<T> {
+        self.arc.clone()
     }
 }
 
-impl<T> Drop for ArcHrTimerHandle<T>
+impl<T> From<Pin<UniqueArc<T>>> for HrTimerArc<T>
 where
     T: HasHrTimer<T>,
 {
-    fn drop(&mut self) {
-        self.cancel();
+    /// Convert a pinned [`UniqueArc`] into a [`HrTimerArc`].
+    #[inline]
+    fn from(unique: Pin<UniqueArc<T>>) -> Self {
+        // INVARIANT: We have a `UniqueArc`, so there is no `HrTimerArc` for this object.
+        Self {
+            arc: Arc::from(unique),
+        }
     }
 }
 
-impl<T> HrTimerPointer for Arc<T>
+impl<T> HrTimerPointer for HrTimerArc<T>
 where
     T: 'static,
     T: Send + Sync,
@@ -66,12 +90,51 @@ fn start(
         //  - We keep `self` alive by wrapping it in a handle below.
         //  - Since we generate the pointer passed to `start` from a valid
         //    reference, it is a valid pointer.
-        unsafe { T::start(Arc::as_ptr(&self), expires) };
+        unsafe { T::start(Arc::as_ptr(&self.arc), expires) };
         ArcHrTimerHandle { inner: self }
     }
 }
 
-impl<T> RawHrTimerCallback for Arc<T>
+/// A handle for a [`HrTimerArc`] returned by a call to [`HrTimerPointer::start`].
+///
+/// This handle owns the [`HrTimerArc`] reference for the object, so the timer cannot be armed
+/// again while this handle exists.
+pub struct ArcHrTimerHandle<T>
+where
+    T: HasHrTimer<T>,
+{
+    pub(crate) inner: HrTimerArc<T>,
+}
+
+// SAFETY: We implement drop below, and we cancel the timer in the drop
+// implementation.
+unsafe impl<T> HrTimerHandle for ArcHrTimerHandle<T>
+where
+    T: HasHrTimer<T>,
+{
+    fn cancel(&mut self) -> bool {
+        let self_ptr = Arc::as_ptr(&self.inner.arc);
+
+        // SAFETY: As we obtained `self_ptr` from a valid reference above, it
+        // must point to a valid `T`.
+        let timer_ptr = unsafe { <T as HasHrTimer<T>>::raw_get_timer(self_ptr) };
+
+        // SAFETY: As `timer_ptr` points into `T` and `T` is valid, `timer_ptr`
+        // must point to a valid `HrTimer` instance.
+        unsafe { HrTimer::<T>::raw_cancel(timer_ptr) }
+    }
+}
+
+impl<T> Drop for ArcHrTimerHandle<T>
+where
+    T: HasHrTimer<T>,
+{
+    fn drop(&mut self) {
+        self.cancel();
+    }
+}
+
+impl<T> RawHrTimerCallback for HrTimerArc<T>
 where
     T: 'static,
     T: HasHrTimer<T>,
-- 
2.43.0