linux-kernel - Re: [PATCH v3 0/5] rust: time: Convert hrtimer to use Instant and Delta

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250625.082020.1714542193051382332.fujita.tomonori@gmail.com>
Date: Wed, 25 Jun 2025 08:20:20 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: a.hindborg@...nel.org
Cc: fujita.tomonori@...il.com, miguel.ojeda.sandonis@...il.com,
 alex.gaynor@...il.com, ojeda@...nel.org, aliceryhl@...gle.com,
 anna-maria@...utronix.de, bjorn3_gh@...tonmail.com, boqun.feng@...il.com,
 dakr@...nel.org, frederic@...nel.org, gary@...yguo.net,
 jstultz@...gle.com, linux-kernel@...r.kernel.org, lossin@...nel.org,
 lyude@...hat.com, rust-for-linux@...r.kernel.org, sboyd@...nel.org,
 tglx@...utronix.de, tmgross@...ch.edu
Subject: Re: [PATCH v3 0/5] rust: time: Convert hrtimer to use Instant and
 Delta

On Tue, 24 Jun 2025 21:03:24 +0200
Andreas Hindborg <a.hindborg@...nel.org> wrote:

> Andreas Hindborg <a.hindborg@...nel.org> writes:
> 
>> "FUJITA Tomonori" <fujita.tomonori@...il.com> writes:
>>
>>> On Tue, 24 Jun 2025 15:11:31 +0200
>>> Andreas Hindborg <a.hindborg@...nel.org> wrote:
>>>
>>>>> and already introduces pain for
>>>>> others (and likely even more pain when we need to rename it back next
>>>>> cycle), it doesn't look like a good idea to keep it.
>>>>
>>>> Ok, I'll drop it.
>>>
>>> Do you want me to send the updated hrtimer conversion patchset
>>> (using as_* names)?
>>
>> No, I am just about finished fixing up the rest. You can check if it is
>> OK when I push.
> 
> I pushed it, please check.

Thanks!

The commit d9fc00dc7354 ("rust: time: Add HrTimerExpires trait") adds
to Instant structure:

+    #[inline]
+    pub(crate) fn as_nanos(&self) -> i64 {
+        self.inner
+    }

Would it be better to take self instead of &self?

pub(crate) fn as_nanos(self) -> i64 {

Because the as_nanos method on the Delta struct takes self, wouldn’t it
be better to keep it consistent? I think that my original patch adds
into_nanos() that takes self. 

This commit also adds HrTimerExpire strait, which as_nanos() method
takes &self:

+/// Time representations that can be used as expiration values in [`HrTimer`].
+pub trait HrTimerExpires {
+    /// Converts the expiration time into a nanosecond representation.
+    ///
+    /// This value corresponds to a raw ktime_t value, suitable for passing to kernel
+    /// timer functions. The interpretation (absolute vs relative) depends on the
+    /// associated [HrTimerMode] in use.
+    fn as_nanos(&self) -> i64;
+}

That's because as I reported, Clippy warns if as_* take self.

As Alice pointed out, Clippy doesn't warn if a type implements
Copy. So we can add Copy to HrTimerExpires trait, then Clippy doesn't
warn about as_nanos method that takes self:

+/// Time representations that can be used as expiration values in [`HrTimer`].
+pub trait HrTimerExpires: Copy {
+    /// Converts the expiration time into a nanosecond representation.
+    ///
+    /// This value corresponds to a raw ktime_t value, suitable for passing to kernel
+    /// timer functions. The interpretation (absolute vs relative) depends on the
+    /// associated [HrTimerMode] in use.
+    fn as_nanos(self) -> i64;
+}

I'm fine with either (taking &self or Adding Copy).