mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1] rust: hrtimer: Remove HrTimer::expires()
@ 2026-11-02  0:01 FUJITA Tomonori
  0 siblings, 0 replies; only message in thread
From: FUJITA Tomonori @ 2026-11-02  0:01 UTC (permalink / raw)
  To: a.hindborg, ojeda
  Cc: acourbot, aliceryhl, anna-maria, bjorn3_gh, boqun, dakr,
	daniel.almeida, frederic, gary, jstultz, lossin, lyude, sboyd,
	tamird, tglx, tmgross, work, linux-kernel, rust-for-linux,
	FUJITA Tomonori

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

HrTimer::expires() reads node.expires without the base lock. The
hrtimer core writes node.expires in hrtimer_start() and
hrtimer_forward(). A timer started through Arc<T> or Pin<&T> can be
started or forwarded while expires() runs. The read then races with
the write. That is a data race, which is undefined behavior in Rust.
The volatile read does not help.

Even if the read were done under the base lock, the value would be a
snapshot. Another CPU can start the timer right after the read and
change the expiry.

A driver that needs to know the expiry should start the timer in
absolute mode. Then the driver already knows the expiry. A callback
that forwards the timer gets the overrun count from forward(). If a
driver still needs the remaining time, a binding for
hrtimer_get_remaining(), which takes the base lock, should be used
instead.

HrTimer::expires() has no users. It is safe to remove it.

Remove Instant::from_ktime() too. HrTimer::expires() was its only user.

Fixes: 4b0147494275 ("rust: hrtimer: Add HrTimer::expires()")
Reported-by: Boqun Feng <boqun@kernel.org>
Closes: https://lore.kernel.org/rust-for-linux/aV5IwaxcIF4XJvg3@tardis-2.local/
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
This patch does not depend on the hrtimer_forward_safe_from_callback()
series [1].

[1] https://lore.kernel.org/rust-for-linux/20261101232829.593680-1-tomo@flapping.org/
---
 rust/kernel/time.rs         | 22 ----------------------
 rust/kernel/time/hrtimer.rs | 23 -----------------------
 2 files changed, 45 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index b6a2aab12a50..7837c26fbca9 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -219,28 +219,6 @@ pub fn elapsed(&self) -> Delta {
     pub(crate) fn as_nanos(&self) -> i64 {
         self.inner
     }
-
-    /// Create an [`Instant`] from a `ktime_t` without checking if it is non-negative.
-    ///
-    /// # Panics
-    ///
-    /// On debug builds, this function will panic if `ktime` is not in the range from 0 to
-    /// `KTIME_MAX`.
-    ///
-    /// # Safety
-    ///
-    /// The caller promises that `ktime` is in the range from 0 to `KTIME_MAX`.
-    #[inline]
-    pub(crate) unsafe fn from_ktime(ktime: bindings::ktime_t) -> Self {
-        debug_assert!(ktime >= 0);
-
-        // INVARIANT: Our safety contract ensures that `ktime` is in the range from 0 to
-        // `KTIME_MAX`.
-        Self {
-            inner: ktime,
-            _c: PhantomData,
-        }
-    }
 }
 
 impl<C: ClockId> ops::Sub for Instant<C> {
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index a53f7f1341be..395810c46910 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -570,29 +570,6 @@ pub fn forward_now(self: Pin<&mut Self>, interval: Delta) -> u64
     {
         self.forward(HrTimerInstant::<T>::now(), interval)
     }
-
-    /// Return the time expiry for this [`HrTimer`].
-    ///
-    /// This value should only be used as a snapshot, as the actual expiry time could change after
-    /// this function is called.
-    pub fn expires(&self) -> HrTimerInstant<T>
-    where
-        T: HasHrTimer<T>,
-    {
-        // SAFETY: `self` is an immutable reference and thus always points to a valid `HrTimer`.
-        let c_timer_ptr = unsafe { HrTimer::raw_get(self) };
-
-        // SAFETY:
-        // - Timers cannot have negative ktime_t values as their expiration time.
-        // - There's no actual locking here, a racy read is fine and expected
-        unsafe {
-            Instant::from_ktime(
-                // This `read_volatile` is intended to correspond to a READ_ONCE call.
-                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
-                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
-            )
-        }
-    }
 }
 
 /// Implemented by pointer types that point to structs that contain a [`HrTimer`].

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2026-11-02  0:01 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02  0:01 [PATCH v1] rust: hrtimer: Remove HrTimer::expires() FUJITA Tomonori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®