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: <877c102pxs.fsf@kernel.org>
Date: Wed, 25 Jun 2025 10:19:59 +0200
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "FUJITA Tomonori" <fujita.tomonori@...il.com>
Cc: <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

"FUJITA Tomonori" <fujita.tomonori@...il.com> writes:

> 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).

Let's wait for the whole naming discussion to resolve before we do
anything. I am honestly a bit confused as to what is the most idiomatic
resolution here.

I think taking `&self` vs `self` makes not difference in codegen if we
mark the function `#[inline(always)]`.


Best regards,
Andreas Hindborg