Re: [PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

Re: [PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Miguel Ojeda]

On Wed, Apr 16, 2025 at 11:05 AM Ingo Molnar <mingo@kernel.org> wrote:
>
> This primitive is dealing with 'struct hrtimer' objects, not
> 'struct timer_list' objects - so clarify the name.
>
> We want to introduce the timer_container_of() symbol in the kernel
> for timer_list, make sure there's no clash of namespaces, at least
> on the conceptual plane.

Andreas should Ack, but I quickly build-tested it and it looks OK:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cc'ing the rust-for-linux ML too.

Thanks!

Cheers,
Miguel

Re: [PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Ingo Molnar]

* Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Wed, Apr 16, 2025 at 11:05 AM Ingo Molnar <mingo@kernel.org> wrote:
> >
> > This primitive is dealing with 'struct hrtimer' objects, not
> > 'struct timer_list' objects - so clarify the name.
> >
> > We want to introduce the timer_container_of() symbol in the kernel
> > for timer_list, make sure there's no clash of namespaces, at least
> > on the conceptual plane.
> 
> Andreas should Ack, but I quickly build-tested it and it looks OK:
> 
> Acked-by: Miguel Ojeda <ojeda@kernel.org>

Thank you!

	Ingo

[PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Ingo Molnar]

This primitive is dealing with 'struct hrtimer' objects, not
'struct timer_list' objects - so clarify the name.

We want to introduce the timer_container_of() symbol in the kernel
for timer_list, make sure there's no clash of namespaces, at least
on the conceptual plane.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: Andreas Hindborg <a.hindborg@kernel.org>
Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rust ML <rust-for-linux@vger.kernel.org>
---
 rust/kernel/time/hrtimer.rs         | 4 ++--
 rust/kernel/time/hrtimer/arc.rs     | 2 +-
 rust/kernel/time/hrtimer/pin.rs     | 2 +-
 rust/kernel/time/hrtimer/pin_mut.rs | 2 +-
 rust/kernel/time/hrtimer/tbox.rs    | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index ce53f8579d18..f3fb7a0caf2f 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -338,7 +338,7 @@ pub unsafe trait HasHrTimer<T> {
     /// # Safety
     ///
     /// `ptr` must point to a [`HrTimer<T>`] field in a struct of type `Self`.
-    unsafe fn timer_container_of(ptr: *mut HrTimer<T>) -> *mut Self
+    unsafe fn hrtimer_container_of(ptr: *mut HrTimer<T>) -> *mut Self
     where
         Self: Sized;
 
@@ -498,7 +498,7 @@ unsafe fn raw_get_timer(
             }
 
             #[inline]
-            unsafe fn timer_container_of(
+            unsafe fn hrtimer_container_of(
                 ptr: *mut $crate::time::hrtimer::HrTimer<$timer_type>,
             ) -> *mut Self {
                 // SAFETY: As per the safety requirement of this function, `ptr`
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 4a984d85b4a1..5cfe6c27795f 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -80,7 +80,7 @@ impl<T> RawHrTimerCallback for Arc<T>
 
         // SAFETY: By C API contract `ptr` is the pointer we passed when
         // queuing the timer, so it is a `HrTimer<T>` embedded in a `T`.
-        let data_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let data_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - `data_ptr` is derived form the pointer to the `T` that was used to
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index f760db265c7b..d16a676b0639 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -83,7 +83,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a T>
 
         // SAFETY: By the safety requirement of this function, `timer_ptr`
         // points to a `HrTimer<T>` contained in an `T`.
-        let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let receiver_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - By the safety requirement of this function, `timer_ptr`
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 90c0351d62e4..17c68f8fbb37 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -87,7 +87,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
 
         // SAFETY: By the safety requirement of this function, `timer_ptr`
         // points to a `HrTimer<T>` contained in an `T`.
-        let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let receiver_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - By the safety requirement of this function, `timer_ptr`
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index 2071cae07234..9dace895ce58 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -103,7 +103,7 @@ impl<T, A> RawHrTimerCallback for Pin<Box<T, A>>
 
         // SAFETY: By C API contract `ptr` is the pointer we passed when
         // queuing the timer, so it is a `HrTimer<T>` embedded in a `T`.
-        let data_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let data_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - As per the safety requirements of the trait `HrTimerHandle`, the
-- 
2.45.2

Re: [PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Andreas Hindborg]

"Ingo Molnar" <mingo@kernel.org> writes:

> This primitive is dealing with 'struct hrtimer' objects, not
> 'struct timer_list' objects - so clarify the name.
>
> We want to introduce the timer_container_of() symbol in the kernel
> for timer_list, make sure there's no clash of namespaces, at least
> on the conceptual plane.

Is this a resend?

Again, I think this change is unnecessary. Please see my earlier reply [1].

Best regards,
Andreas Hindborg


[1] https://lore.kernel.org/all/877c3cbdq2.fsf@kernel.org

Re: [PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Ingo Molnar]

* Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "Ingo Molnar" <mingo@kernel.org> writes:
> 
> > This primitive is dealing with 'struct hrtimer' objects, not
> > 'struct timer_list' objects - so clarify the name.
> >
> > We want to introduce the timer_container_of() symbol in the kernel
> > for timer_list, make sure there's no clash of namespaces, at least
> > on the conceptual plane.
> 
> Is this a resend?

I noted the changes in the boilerplate:

  Changes in -v3:

    - Picked up review tags
    - Rebased to v6.15-rc5

This particular patch didn't change.

> Again, I think this change is unnecessary. Please see my earlier reply [1].
> 
> Best regards,
> Andreas Hindborg
> 
> 
> [1] https://lore.kernel.org/all/877c3cbdq2.fsf@kernel.org

Yeah, saw that, but you said you are fine with it if I insist, and I'd 
like to have this to free up the timer_* namespace. Since I think we'd 
like to introduce the timer_container_of() in the future it would be 
nice to do this rename, as:

	$ git grep -w timer_container_of

will have hrtimer related false positive hits in rust/ code, even 
though the namespaces are obviously independent.

The Rust method is arguably a minor misnomer as well: you have 
work_container_of around struct work, but timer_container_of is around 
struct hrtimer?

Thanks,

	Ingo

Re: [PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Andreas Hindborg]

"Ingo Molnar" <mingo@kernel.org> writes:

> * Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> "Ingo Molnar" <mingo@kernel.org> writes:
>>
>> > This primitive is dealing with 'struct hrtimer' objects, not
>> > 'struct timer_list' objects - so clarify the name.
>> >
>> > We want to introduce the timer_container_of() symbol in the kernel
>> > for timer_list, make sure there's no clash of namespaces, at least
>> > on the conceptual plane.
>>
>> Is this a resend?
>
> I noted the changes in the boilerplate:
>
>   Changes in -v3:
>
>     - Picked up review tags
>     - Rebased to v6.15-rc5
>
> This particular patch didn't change.

Thanks. I didn't get the cover letter, but I should have looked for it.

>
>> Again, I think this change is unnecessary. Please see my earlier reply [1].
>>
>> Best regards,
>> Andreas Hindborg
>>
>>
>> [1] https://lore.kernel.org/all/877c3cbdq2.fsf@kernel.org
>
> Yeah, saw that, but you said you are fine with it if I insist, and I'd
> like to have this to free up the timer_* namespace.

Yes. I did not hear any proper insisting till now though.

> Since I think we'd
> like to introduce the timer_container_of() in the future it would be
> nice to do this rename, as:
>
> 	$ git grep -w timer_container_of
>
> will have hrtimer related false positive hits in rust/ code, even
> though the namespaces are obviously independent.

Ok, I see. I'm not used to grepping like that, but I see how that can be
annoying.

>
> The Rust method is arguably a minor misnomer as well: you have
> work_container_of around struct work, but timer_container_of is around
> struct hrtimer?

Yes, you are right.

Feel free to take this through tip. Otherwise maybe Miguel can pick it
up in the rust PR for 6.15.


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


Best regards,
Andreas Hindborg

Re: [PATCH 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> "Ingo Molnar" <mingo@kernel.org> writes:
>
>> * Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>>> "Ingo Molnar" <mingo@kernel.org> writes:
>>>
>>> > This primitive is dealing with 'struct hrtimer' objects, not
>>> > 'struct timer_list' objects - so clarify the name.
>>> >
>>> > We want to introduce the timer_container_of() symbol in the kernel
>>> > for timer_list, make sure there's no clash of namespaces, at least
>>> > on the conceptual plane.
>>>
>>> Is this a resend?
>>
>> I noted the changes in the boilerplate:
>>
>>   Changes in -v3:
>>
>>     - Picked up review tags
>>     - Rebased to v6.15-rc5
>>
>> This particular patch didn't change.
>
> Thanks. I didn't get the cover letter, but I should have looked for it.
>
>>
>>> Again, I think this change is unnecessary. Please see my earlier reply [1].
>>>
>>> Best regards,
>>> Andreas Hindborg
>>>
>>>
>>> [1] https://lore.kernel.org/all/877c3cbdq2.fsf@kernel.org
>>
>> Yeah, saw that, but you said you are fine with it if I insist, and I'd
>> like to have this to free up the timer_* namespace.
>
> Yes. I did not hear any proper insisting till now though.
>
>> Since I think we'd
>> like to introduce the timer_container_of() in the future it would be
>> nice to do this rename, as:
>>
>> 	$ git grep -w timer_container_of
>>
>> will have hrtimer related false positive hits in rust/ code, even
>> though the namespaces are obviously independent.
>
> Ok, I see. I'm not used to grepping like that, but I see how that can be
> annoying.
>
>>
>> The Rust method is arguably a minor misnomer as well: you have
>> work_container_of around struct work, but timer_container_of is around
>> struct hrtimer?
>
> Yes, you are right.
>
> Feel free to take this through tip. Otherwise maybe Miguel can pick it
> up in the rust PR for 6.15.

Sorry, I meant 6.16 of course.


Best regards,
Andreas Hindborg

[PATCH -v2 01/10] rust: Rename timer_container_of() to hrtimer_container_of()

[Ingo Molnar]

* Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "Ingo Molnar" <mingo@kernel.org> writes:
> 
> > * Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >
> >> "Ingo Molnar" <mingo@kernel.org> writes:
> >>
> >> > This primitive is dealing with 'struct hrtimer' objects, not
> >> > 'struct timer_list' objects - so clarify the name.
> >> >
> >> > We want to introduce the timer_container_of() symbol in the kernel
> >> > for timer_list, make sure there's no clash of namespaces, at least
> >> > on the conceptual plane.
> >>
> >> Is this a resend?
> >
> > I noted the changes in the boilerplate:
> >
> >   Changes in -v3:
> >
> >     - Picked up review tags
> >     - Rebased to v6.15-rc5
> >
> > This particular patch didn't change.
> 
> Thanks. I didn't get the cover letter, but I should have looked for it.

Sorry about that, I have added your Cc: to the main timer_container_of 
patch as well.

> >> [1] https://lore.kernel.org/all/877c3cbdq2.fsf@kernel.org
> >
> > Yeah, saw that, but you said you are fine with it if I insist, and I'd
> > like to have this to free up the timer_* namespace.
> 
> Yes. I did not hear any proper insisting till now though.

:) I always see these threads in their full context, and I didn't
immediately realize that you only saw part of it, with a limited, 
misleading context, and was slow at reacting to your concern.

> > Since I think we'd like to introduce the timer_container_of() in 
> > the future it would be nice to do this rename, as:
> >
> > 	$ git grep -w timer_container_of
> >
> > will have hrtimer related false positive hits in rust/ code, even
> > though the namespaces are obviously independent.
> 
> Ok, I see. I'm not used to grepping like that, but I see how that can be
> annoying.
> 
> >
> > The Rust method is arguably a minor misnomer as well: you have
> > work_container_of around struct work, but timer_container_of is around
> > struct hrtimer?
> 
> Yes, you are right.
> 
> Feel free to take this through tip. Otherwise maybe Miguel can pick it
> up in the rust PR for 6.15.
> 
> Acked-by: Andreas Hindborg <a.hindborg@kernel.org>

Thank you!!

The tentative merge plan is/was that, if everything goes smoothly, we'd 
send this and the general timer_container_of() patch to Linus a few 
days before -rc1, due to the substantial cross section of the changes:

  treewide, timers: Rename from_timer() => timer_container_of()

  693 files changed, 913 insertions(+), 913 deletions(-)

:-/

But with your Acked-by we can now send the Rust patch through the 
regular channels with the timer tree and only send the single treewide 
patch to Linus separately.

If things are too busy in the merge window for Thomas or Linus, the 
non-Rust patch can easily slip to the v6.17 merge window though.

Thanks,

	Ingo

====================>
From: Ingo Molnar <mingo@kernel.org>
Date: Mon, 14 Apr 2025 11:19:20 +0200
Subject: [PATCH] rust: Rename timer_container_of() to hrtimer_container_of()

This primitive is dealing with 'struct hrtimer' objects, not
'struct timer_list' objects - so clarify the name.

We want to introduce the timer_container_of() symbol in the kernel
for timer_list, make sure there's no clash of namespaces, at least
on the conceptual plane.

Tested-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Acked-by: Andreas Hindborg <a.hindborg@kernel.org>
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rust ML <rust-for-linux@vger.kernel.org>
---
 -v2: Add Andreas Hindborg's Acked-by

 rust/kernel/time/hrtimer.rs         | 4 ++--
 rust/kernel/time/hrtimer/arc.rs     | 2 +-
 rust/kernel/time/hrtimer/pin.rs     | 2 +-
 rust/kernel/time/hrtimer/pin_mut.rs | 2 +-
 rust/kernel/time/hrtimer/tbox.rs    | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index ce53f8579d18..f3fb7a0caf2f 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -338,7 +338,7 @@ pub unsafe trait HasHrTimer<T> {
     /// # Safety
     ///
     /// `ptr` must point to a [`HrTimer<T>`] field in a struct of type `Self`.
-    unsafe fn timer_container_of(ptr: *mut HrTimer<T>) -> *mut Self
+    unsafe fn hrtimer_container_of(ptr: *mut HrTimer<T>) -> *mut Self
     where
         Self: Sized;
 
@@ -498,7 +498,7 @@ unsafe fn raw_get_timer(
             }
 
             #[inline]
-            unsafe fn timer_container_of(
+            unsafe fn hrtimer_container_of(
                 ptr: *mut $crate::time::hrtimer::HrTimer<$timer_type>,
             ) -> *mut Self {
                 // SAFETY: As per the safety requirement of this function, `ptr`
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 4a984d85b4a1..5cfe6c27795f 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -80,7 +80,7 @@ impl<T> RawHrTimerCallback for Arc<T>
 
         // SAFETY: By C API contract `ptr` is the pointer we passed when
         // queuing the timer, so it is a `HrTimer<T>` embedded in a `T`.
-        let data_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let data_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - `data_ptr` is derived form the pointer to the `T` that was used to
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index f760db265c7b..d16a676b0639 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -83,7 +83,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a T>
 
         // SAFETY: By the safety requirement of this function, `timer_ptr`
         // points to a `HrTimer<T>` contained in an `T`.
-        let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let receiver_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - By the safety requirement of this function, `timer_ptr`
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 90c0351d62e4..17c68f8fbb37 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -87,7 +87,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
 
         // SAFETY: By the safety requirement of this function, `timer_ptr`
         // points to a `HrTimer<T>` contained in an `T`.
-        let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let receiver_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - By the safety requirement of this function, `timer_ptr`
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index 2071cae07234..9dace895ce58 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -103,7 +103,7 @@ impl<T, A> RawHrTimerCallback for Pin<Box<T, A>>
 
         // SAFETY: By C API contract `ptr` is the pointer we passed when
         // queuing the timer, so it is a `HrTimer<T>` embedded in a `T`.
-        let data_ptr = unsafe { T::timer_container_of(timer_ptr) };
+        let data_ptr = unsafe { T::hrtimer_container_of(timer_ptr) };
 
         // SAFETY:
         //  - As per the safety requirements of the trait `HrTimerHandle`, the