All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns
@ 2025-07-10 22:51 ` Lyude Paul
  2025-07-11  6:18   ` Andreas Hindborg
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Lyude Paul @ 2025-07-10 22:51 UTC (permalink / raw)
  To: rust-for-linux
  Cc: FUJITA Tomonori, Andreas Hindborg, Boqun Feng,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, open list

While rebasing rvkms I noticed that timers I was setting seemed to have
pretty random timer values that amounted slightly over 2x the time value I
set each time. After a lot of debugging, I finally managed to figure out
why: it seems that since we moved to Instant and Delta, we mistakenly
began passing the clocksource ID to hrtimer_start_range_ns, when we should
be passing the timer mode instead. Presumably, this works fine for simple
relative timers - but immediately breaks on other types of timers.

So, fix this by passing the ID for the timer mode instead.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
Fixes: fcc1dd8c8656 ("rust: time: Make HasHrTimer generic over HrTimerMode")
---
 rust/kernel/time/hrtimer.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 8818775afaf69..e227fa95ab05c 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -398,7 +398,7 @@ unsafe fn start(this: *const Self, expires: <Self::TimerMode as HrTimerMode>::Ex
                 Self::c_timer_ptr(this).cast_mut(),
                 expires.as_nanos(),
                 0,
-                <Self::TimerMode as HrTimerMode>::Clock::ID as u32,
+                <Self::TimerMode as HrTimerMode>::C_MODE as u32
             );
         }
     }

base-commit: d4b29ddf82a458935f1bd4909b8a7a13df9d3bdc
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns
  2025-07-10 22:51 ` [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns Lyude Paul
@ 2025-07-11  6:18   ` Andreas Hindborg
  2025-07-11  6:37     ` FUJITA Tomonori
  2025-07-11  6:28   ` FUJITA Tomonori
  2025-07-17  0:05   ` Miguel Ojeda
  2 siblings, 1 reply; 7+ messages in thread
From: Andreas Hindborg @ 2025-07-11  6:18 UTC (permalink / raw)
  To: Lyude Paul
  Cc: rust-for-linux, FUJITA Tomonori, Boqun Feng, Frederic Weisbecker,
	Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
	Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	linux-kernel

"Lyude Paul" <lyude@redhat.com> writes:

> While rebasing rvkms I noticed that timers I was setting seemed to have
> pretty random timer values that amounted slightly over 2x the time value I
> set each time. After a lot of debugging, I finally managed to figure out
> why: it seems that since we moved to Instant and Delta, we mistakenly
> began passing the clocksource ID to hrtimer_start_range_ns, when we should
> be passing the timer mode instead. Presumably, this works fine for simple
> relative timers - but immediately breaks on other types of timers.
>
> So, fix this by passing the ID for the timer mode instead.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
> Fixes: fcc1dd8c8656 ("rust: time: Make HasHrTimer generic over HrTimerMode")

Wow, thanks! Miguel, can you take this through rust-fixes?


Acked-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns
  2025-07-10 22:51 ` [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns Lyude Paul
  2025-07-11  6:18   ` Andreas Hindborg
@ 2025-07-11  6:28   ` FUJITA Tomonori
  2025-07-17  0:05   ` Miguel Ojeda
  2 siblings, 0 replies; 7+ messages in thread
From: FUJITA Tomonori @ 2025-07-11  6:28 UTC (permalink / raw)
  To: lyude
  Cc: rust-for-linux, fujita.tomonori, a.hindborg, boqun.feng, frederic,
	tglx, anna-maria, jstultz, sboyd, ojeda, alex.gaynor, gary,
	bjorn3_gh, lossin, aliceryhl, tmgross, dakr, linux-kernel

On Thu, 10 Jul 2025 18:51:13 -0400
Lyude Paul <lyude@redhat.com> wrote:

> While rebasing rvkms I noticed that timers I was setting seemed to have
> pretty random timer values that amounted slightly over 2x the time value I
> set each time. After a lot of debugging, I finally managed to figure out
> why: it seems that since we moved to Instant and Delta, we mistakenly
> began passing the clocksource ID to hrtimer_start_range_ns, when we should
> be passing the timer mode instead. Presumably, this works fine for simple
> relative timers - but immediately breaks on other types of timers.
> 
> So, fix this by passing the ID for the timer mode instead.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
> Fixes: fcc1dd8c8656 ("rust: time: Make HasHrTimer generic over HrTimerMode")
> ---
>  rust/kernel/time/hrtimer.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Ah, my bad! Thanks a lot for the fix.

Reviewed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns
  2025-07-11  6:18   ` Andreas Hindborg
@ 2025-07-11  6:37     ` FUJITA Tomonori
  2025-07-11  6:49       ` Andreas Hindborg
  0 siblings, 1 reply; 7+ messages in thread
From: FUJITA Tomonori @ 2025-07-11  6:37 UTC (permalink / raw)
  To: a.hindborg
  Cc: lyude, rust-for-linux, fujita.tomonori, boqun.feng, frederic,
	tglx, anna-maria, jstultz, sboyd, ojeda, alex.gaynor, gary,
	bjorn3_gh, lossin, aliceryhl, tmgross, dakr, linux-kernel

On Fri, 11 Jul 2025 08:18:37 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "Lyude Paul" <lyude@redhat.com> writes:
> 
>> While rebasing rvkms I noticed that timers I was setting seemed to have
>> pretty random timer values that amounted slightly over 2x the time value I
>> set each time. After a lot of debugging, I finally managed to figure out
>> why: it seems that since we moved to Instant and Delta, we mistakenly
>> began passing the clocksource ID to hrtimer_start_range_ns, when we should
>> be passing the timer mode instead. Presumably, this works fine for simple
>> relative timers - but immediately breaks on other types of timers.
>>
>> So, fix this by passing the ID for the timer mode instead.
>>
>> Signed-off-by: Lyude Paul <lyude@redhat.com>
>> Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> Fixes: fcc1dd8c8656 ("rust: time: Make HasHrTimer generic over HrTimerMode")
> 
> Wow, thanks! Miguel, can you take this through rust-fixes?

I think that this patch fixes the commit in timekeeping-next.

`fcc1dd8c8656` doesn't match to the commit in the current
timekeeping-next (this patch might have been made against the tree
before it was rebased).

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns
  2025-07-11  6:37     ` FUJITA Tomonori
@ 2025-07-11  6:49       ` Andreas Hindborg
  2025-07-11  9:53         ` Miguel Ojeda
  0 siblings, 1 reply; 7+ messages in thread
From: Andreas Hindborg @ 2025-07-11  6:49 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: lyude, rust-for-linux, boqun.feng, frederic, tglx, anna-maria,
	jstultz, sboyd, ojeda, alex.gaynor, gary, bjorn3_gh, lossin,
	aliceryhl, tmgross, dakr, linux-kernel

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

> On Fri, 11 Jul 2025 08:18:37 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> "Lyude Paul" <lyude@redhat.com> writes:
>>
>>> While rebasing rvkms I noticed that timers I was setting seemed to have
>>> pretty random timer values that amounted slightly over 2x the time value I
>>> set each time. After a lot of debugging, I finally managed to figure out
>>> why: it seems that since we moved to Instant and Delta, we mistakenly
>>> began passing the clocksource ID to hrtimer_start_range_ns, when we should
>>> be passing the timer mode instead. Presumably, this works fine for simple
>>> relative timers - but immediately breaks on other types of timers.
>>>
>>> So, fix this by passing the ID for the timer mode instead.
>>>
>>> Signed-off-by: Lyude Paul <lyude@redhat.com>
>>> Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>> Fixes: fcc1dd8c8656 ("rust: time: Make HasHrTimer generic over HrTimerMode")
>>
>> Wow, thanks! Miguel, can you take this through rust-fixes?
>
> I think that this patch fixes the commit in timekeeping-next.
>
> `fcc1dd8c8656` doesn't match to the commit in the current
> timekeeping-next (this patch might have been made against the tree
> before it was rebased).

Maybe Miguel can put the correct hash when he applies the patch.


Best regards,
Andreas Hindborg



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns
  2025-07-11  6:49       ` Andreas Hindborg
@ 2025-07-11  9:53         ` Miguel Ojeda
  0 siblings, 0 replies; 7+ messages in thread
From: Miguel Ojeda @ 2025-07-11  9:53 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: FUJITA Tomonori, lyude, rust-for-linux, boqun.feng, frederic,
	tglx, anna-maria, jstultz, sboyd, ojeda, alex.gaynor, gary,
	bjorn3_gh, lossin, aliceryhl, tmgross, dakr, linux-kernel

On Fri, Jul 11, 2025 at 8:49 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Maybe Miguel can put the correct hash when he applies the patch.

Sure:

Fixes: e0c0ab04f678 ("rust: time: Make HasHrTimer generic over HrTimerMode")

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns
  2025-07-10 22:51 ` [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns Lyude Paul
  2025-07-11  6:18   ` Andreas Hindborg
  2025-07-11  6:28   ` FUJITA Tomonori
@ 2025-07-17  0:05   ` Miguel Ojeda
  2 siblings, 0 replies; 7+ messages in thread
From: Miguel Ojeda @ 2025-07-17  0:05 UTC (permalink / raw)
  To: Lyude Paul
  Cc: rust-for-linux, FUJITA Tomonori, Andreas Hindborg, Boqun Feng,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, open list

On Fri, Jul 11, 2025 at 12:51 AM Lyude Paul <lyude@redhat.com> wrote:
>
> While rebasing rvkms I noticed that timers I was setting seemed to have
> pretty random timer values that amounted slightly over 2x the time value I
> set each time. After a lot of debugging, I finally managed to figure out
> why: it seems that since we moved to Instant and Delta, we mistakenly
> began passing the clocksource ID to hrtimer_start_range_ns, when we should
> be passing the timer mode instead. Presumably, this works fine for simple
> relative timers - but immediately breaks on other types of timers.
>
> So, fix this by passing the ID for the timer mode instead.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>
> Fixes: fcc1dd8c8656 ("rust: time: Make HasHrTimer generic over HrTimerMode")

Applied to `rust-next` (on top of the timekeeping merge) -- thanks everyone!

    [ Removed cast, applied `rustfmt`, fixed `Fixes:` tag. - Miguel ]

I assume the cast was there just because the original line (`Clock::ID`) had it.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-17  0:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <U1GgPfpPwSjzh5jPpFlN22bT2BSjlaH8vLHYY6hAt_vJ5w4irwIYRPV46r73Cs5Dx73Ikz5ku7_qPWrl8Tntfw==@protonmail.internalid>
2025-07-10 22:51 ` [PATCH] rust: time: Pass correct timer mode ID to hrtimer_start_range_ns Lyude Paul
2025-07-11  6:18   ` Andreas Hindborg
2025-07-11  6:37     ` FUJITA Tomonori
2025-07-11  6:49       ` Andreas Hindborg
2025-07-11  9:53         ` Miguel Ojeda
2025-07-11  6:28   ` FUJITA Tomonori
2025-07-17  0:05   ` Miguel Ojeda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.