[PATCH v8 00/14] hrtimer Rust API

Andreas Hindborg posted 14 patches 10 months ago
There is a newer version of this series
MAINTAINERS                         |  10 +
rust/kernel/alloc/kbox.rs           |   6 +
rust/kernel/sync/arc.rs             |  13 +-
rust/kernel/time.rs                 |  10 +
rust/kernel/time/hrtimer.rs         | 539 ++++++++++++++++++++++++++++++++++++
rust/kernel/time/hrtimer/arc.rs     |  87 ++++++
rust/kernel/time/hrtimer/pin.rs     |  95 +++++++
rust/kernel/time/hrtimer/pin_mut.rs |  97 +++++++
rust/kernel/time/hrtimer/tbox.rs    | 102 +++++++
9 files changed, 957 insertions(+), 2 deletions(-)
[PATCH v8 00/14] hrtimer Rust API
Posted by Andreas Hindborg 10 months ago
This series adds support for using the `hrtimer` subsystem from Rust code.

The series adds support for timer mode and clock source configuration during
timer initialization. Examples and functionality to execute closures at timer
expiration has been removed, as these depend on either atomics [3] or
`SpinLockIrq` [4], which are still being worked on.

This series is a dependency for unmerged features of the Rust null block driver
[1], and for rkvms [2].

Link: https://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull-v6.11-rc2 [1]
Link: https://gitlab.freedesktop.org/lyudess/linux/-/tree/rvkms-wip [2]
Link: https://lore.kernel.org/rust-for-linux/20240612223025.1158537-1-boqun.feng@gmail.com/ [3]
Link: https://lore.kernel.org/rust-for-linux/20240916213025.477225-1-lyude@redhat.com/ [4]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v8:
- Publicly expose timer handles.
- Link to v7: https://lore.kernel.org/r/20250203-hrtimer-v3-v6-12-rc2-v7-0-189144725399@kernel.org

Changes in v7:
- fix a typo in commit message for "rust: time: Add Ktime::from_ns()"
- fix a typo in safety comment in `HrTimer::new`
- fix a typo in `HrTimer::raw_cancel`
- fix a typo in the vocabulary
- fix a typo in `HrTimerCallback` docs
- refactor module documentation
- add an ascii state diagram to module documentation
- specify reason for adding `Arc::as_ptr`'
- change `boxed` to `this` in `Box::into_pin`
- change `from_ns` to `from_nanos` to align with std
- imporove safety comment for `impl Send for HrTimer`
- remove useless paragraph in docs for `HrTimerPointer`
- rephrase docs for `HrTimerPointer::TimerHandle`
- update docs for `HrTimerCallback::CallbackTarget`
- explain how users should use safe functions for cancelling a timer
- rename generics for consistency
- remove a note about storing mode in `HrTimer` - this is still required
- rebase on v6.14-rc1
- Link to v6: https://lore.kernel.org/r/20250110-hrtimer-v3-v6-12-rc2-v6-0-f71d50f16482@kernel.org

Changes in v6:
- prefix all hrtimer related type names with `Hr`
- add a few links for type names in the documentation
- Link to v5: https://lore.kernel.org/r/20241217-hrtimer-v3-v6-12-rc2-v5-0-b34c20ac2cb7@kernel.org

Changes in v5:
- Fix a typo in `impl_has_timer`
- Implement `Box::into_pin` in terms of `impl From<Box> for Pin<Box>`
- Link to v4: https://lore.kernel.org/r/20241206-hrtimer-v3-v6-12-rc2-v4-0-6cb8c3673682@kernel.org

Changes in v4:
- rebase on v6.13-rc1 and adapt to kernel `Box`
- add a missing safety comment to `hrtimer::start`
- use `hrtimer_setup`
- fix a build issue when `bindings::hrtimer_restart` is signed
- fix a memory leak where box was not destroyed
- fix a documentation typo
- remove `as` coercion at multiple locations
- use fully qualified syntax when invoking `deref`
- move `hrtimer` into `time` module
- Link to v3: https://lore.kernel.org/r/20241017-hrtimer-v3-v6-12-rc2-v3-0-59a75cbb44da@kernel.org

Changes in v3:
- support timer mode selection
- support clock source selection
- eliminate `Arc::clone_from_raw` in favor of using `ArcBorrow`
- make `Arc::as_ptr` an associated method
- update safety requirement for `ArcBorrow::from_raw`
- remove examples (pending `SpinLockIrq` and `CondVar` patches)
- remove `start_function` (v2 `schedule_function`, pending `SpinLockIrq` and `CondVar` patches)
- change function naming from schedule/armed to start/running
- add vocabulary to documentation
- update safety comment in `Arc::as_ptr`
- Link to v2: https://lore.kernel.org/r/20240917222739.1298275-1-a.hindborg@kernel.org

Changes in v2:
- use a handle to own the timer callback target
- add ability to for callback to reschedule timer
- improve `impl_has_timer` to allow generics
- add support for stack allocated timers
- add support for scheduling closures
- use `Ktime` for setting expiration
- use `CondVar` instead of `AtomicBool` in examples
- rebase on 6.11
- improve documentation
- Link to v1: https://lore.kernel.org/r/20240425094634.262674-1-nmi@metaspace.dk

---
Andreas Hindborg (13):
      rust: hrtimer: introduce hrtimer support
      rust: sync: add `Arc::as_ptr`
      rust: hrtimer: implement `HrTimerPointer` for `Arc`
      rust: hrtimer: allow timer restart from timer handler
      rust: hrtimer: add `UnsafeHrTimerPointer`
      rust: hrtimer: add `hrtimer::ScopedHrTimerPointer`
      rust: hrtimer: implement `UnsafeHrTimerPointer` for `Pin<&T>`
      rust: hrtimer: implement `UnsafeHrTimerPointer` for `Pin<&mut T>`
      rust: alloc: add `Box::into_pin`
      rust: hrtimer: implement `HrTimerPointer` for `Pin<Box<T>>`
      rust: hrtimer: add `HrTimerMode`
      rust: hrtimer: add clocksource selection through `ClockSource`
      rust: hrtimer: add maintainer entry

Lyude Paul (1):
      rust: time: Add Ktime::from_ns()

 MAINTAINERS                         |  10 +
 rust/kernel/alloc/kbox.rs           |   6 +
 rust/kernel/sync/arc.rs             |  13 +-
 rust/kernel/time.rs                 |  10 +
 rust/kernel/time/hrtimer.rs         | 539 ++++++++++++++++++++++++++++++++++++
 rust/kernel/time/hrtimer/arc.rs     |  87 ++++++
 rust/kernel/time/hrtimer/pin.rs     |  95 +++++++
 rust/kernel/time/hrtimer/pin_mut.rs |  97 +++++++
 rust/kernel/time/hrtimer/tbox.rs    | 102 +++++++
 9 files changed, 957 insertions(+), 2 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20241017-hrtimer-v3-v6-12-rc2-215dc6b169bf

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Frederic Weisbecker 10 months ago
Le Tue, Feb 18, 2025 at 02:27:05PM +0100, Andreas Hindborg a écrit :
> This series adds support for using the `hrtimer` subsystem from Rust code.
> 
> The series adds support for timer mode and clock source configuration during
> timer initialization. Examples and functionality to execute closures at timer
> expiration has been removed, as these depend on either atomics [3] or
> `SpinLockIrq` [4], which are still being worked on.
> 
> This series is a dependency for unmerged features of the Rust null block driver
> [1], and for rkvms [2].
> 
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull-v6.11-rc2 [1]
> Link: https://gitlab.freedesktop.org/lyudess/linux/-/tree/rvkms-wip [2]
> Link: https://lore.kernel.org/rust-for-linux/20240612223025.1158537-1-boqun.feng@gmail.com/ [3]
> Link: https://lore.kernel.org/rust-for-linux/20240916213025.477225-1-lyude@redhat.com/ [4]
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Let me repeat again that I have extremely limited understanding of this.
But let's say I haven't spotted a major issue on the interface...

	    Acked-by: Frederic Weisbecker <frederic@kernel.org>

Thanks!
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Andreas Hindborg 10 months ago
"Andreas Hindborg" <a.hindborg@kernel.org> writes:

> This series adds support for using the `hrtimer` subsystem from Rust code.
>
> The series adds support for timer mode and clock source configuration during
> timer initialization. Examples and functionality to execute closures at timer
> expiration has been removed, as these depend on either atomics [3] or
> `SpinLockIrq` [4], which are still being worked on.
>
> This series is a dependency for unmerged features of the Rust null block driver
> [1], and for rkvms [2].
>

@ timer subsystem maintainers: did you discuss how you want to set up
maintenance for this yet? As mentioned, I'm happy stepping up to
maintain this, but if you want to handle it with existing resources that
is perfectly fine as well.

I was hoping we could merge the patches in the near future. The patches
have been on list for quite a while now, and I am happy with the shape
of them. They are in my critical path for merging dependent code in the
rust null block driver.

Let me know if there is anything I can do to help move the process
forward.


Best regards,
Andreas Hindborg
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Frederic Weisbecker 10 months ago
Le Wed, Feb 19, 2025 at 12:02:50PM +0100, Andreas Hindborg a écrit :
> "Andreas Hindborg" <a.hindborg@kernel.org> writes:
> 
> > This series adds support for using the `hrtimer` subsystem from Rust code.
> >
> > The series adds support for timer mode and clock source configuration during
> > timer initialization. Examples and functionality to execute closures at timer
> > expiration has been removed, as these depend on either atomics [3] or
> > `SpinLockIrq` [4], which are still being worked on.
> >
> > This series is a dependency for unmerged features of the Rust null block driver
> > [1], and for rkvms [2].
> >
> 
> @ timer subsystem maintainers: did you discuss how you want to set up
> maintenance for this yet? As mentioned, I'm happy stepping up to
> maintain this, but if you want to handle it with existing resources that
> is perfectly fine as well.

You're the best candidate to maintain this code since you wrote it :-)

Also I personally have near zero skills in Rust as of today so all I can do
is to vaguely keep an eye on the binding's interface and keep in touch
with the changes.

So I suggest you to add a new entry with you as a maintainer (I suggested
something similar to Fujita for some other timer related things) but please
keep us Cc'ed for future changes.

> I was hoping we could merge the patches in the near future. The patches
> have been on list for quite a while now, and I am happy with the shape
> of them. They are in my critical path for merging dependent code in the
> rust null block driver.
> 
> Let me know if there is anything I can do to help move the process
> forward.

Let me have a last look...

Thanks.


> 
> 
> Best regards,
> Andreas Hindborg
> 
> 
> 
> 
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Andreas Hindborg 10 months ago
"Frederic Weisbecker" <frederic@kernel.org> writes:

> Le Wed, Feb 19, 2025 at 12:02:50PM +0100, Andreas Hindborg a écrit :
>> "Andreas Hindborg" <a.hindborg@kernel.org> writes:
>>
>> > This series adds support for using the `hrtimer` subsystem from Rust code.
>> >
>> > The series adds support for timer mode and clock source configuration during
>> > timer initialization. Examples and functionality to execute closures at timer
>> > expiration has been removed, as these depend on either atomics [3] or
>> > `SpinLockIrq` [4], which are still being worked on.
>> >
>> > This series is a dependency for unmerged features of the Rust null block driver
>> > [1], and for rkvms [2].
>> >
>>
>> @ timer subsystem maintainers: did you discuss how you want to set up
>> maintenance for this yet? As mentioned, I'm happy stepping up to
>> maintain this, but if you want to handle it with existing resources that
>> is perfectly fine as well.
>
> You're the best candidate to maintain this code since you wrote it :-)
>
> Also I personally have near zero skills in Rust as of today so all I can do
> is to vaguely keep an eye on the binding's interface and keep in touch
> with the changes.
>
> So I suggest you to add a new entry with you as a maintainer (I suggested
> something similar to Fujita for some other timer related things) but please
> keep us Cc'ed for future changes.

Alright, lets do that.

Do you want to pick future changes to this directly form list or would
you prefer that I send you a PR with changes?

We are probably going to have a new iteration anyway, as discussion
picked up again.


Best regards,
Andreas Hindborg
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Frederic Weisbecker 10 months ago
Le Fri, Feb 21, 2025 at 09:40:41AM +0100, Andreas Hindborg a écrit :
> "Frederic Weisbecker" <frederic@kernel.org> writes:
> 
> > Le Wed, Feb 19, 2025 at 12:02:50PM +0100, Andreas Hindborg a écrit :
> >> "Andreas Hindborg" <a.hindborg@kernel.org> writes:
> >>
> >> > This series adds support for using the `hrtimer` subsystem from Rust code.
> >> >
> >> > The series adds support for timer mode and clock source configuration during
> >> > timer initialization. Examples and functionality to execute closures at timer
> >> > expiration has been removed, as these depend on either atomics [3] or
> >> > `SpinLockIrq` [4], which are still being worked on.
> >> >
> >> > This series is a dependency for unmerged features of the Rust null block driver
> >> > [1], and for rkvms [2].
> >> >
> >>
> >> @ timer subsystem maintainers: did you discuss how you want to set up
> >> maintenance for this yet? As mentioned, I'm happy stepping up to
> >> maintain this, but if you want to handle it with existing resources that
> >> is perfectly fine as well.
> >
> > You're the best candidate to maintain this code since you wrote it :-)
> >
> > Also I personally have near zero skills in Rust as of today so all I can do
> > is to vaguely keep an eye on the binding's interface and keep in touch
> > with the changes.
> >
> > So I suggest you to add a new entry with you as a maintainer (I suggested
> > something similar to Fujita for some other timer related things) but please
> > keep us Cc'ed for future changes.
> 
> Alright, lets do that.
> 
> Do you want to pick future changes to this directly form list or would
> you prefer that I send you a PR with changes?

I was thinking the patchset would be better routed towards the Rust tree?

How do you guys proceed usually with bindings tree maintainance?

I read yesterday Linus saying that Rust bindings are users of existing
kernel infrastructure just like any other driver. I personally think it's
more than that, and probably he does to, but anyway he has a point in that
they are not changing the core infrastructures. Functionally they are
eventually new API users.

Adding to that, Rust bindings require quite some specific knowledge that
is mostly to be found among the Rust tree developers for now (and not much
shared elsewhere I suspect), I think it's a better idea that you guys handle
this hrtimer binding within the Rust tree. You'll be more flexible and people
applying the related patches will know what they are doing.

What do you think?

> 
> We are probably going to have a new iteration anyway, as discussion
> picked up again.

Ok.

Thanks.

> 
> 
> Best regards,
> Andreas Hindborg
> 
> 
> 
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Miguel Ojeda 9 months, 4 weeks ago
On Fri, Feb 21, 2025 at 12:20 PM Frederic Weisbecker
<frederic@kernel.org> wrote:
>
> I was thinking the patchset would be better routed towards the Rust tree?
>
> How do you guys proceed usually with bindings tree maintainance?

So far, what we have been doing is ask maintainers, first, if they
would be willing take the patches themselves -- they are the experts
of the subsystem, know what changes are incoming, etc. Some subsystems
have done this (e.g. KUnit). That is ideal, because the goal is to
scale and allows maintainers to be in full control.

Of course, sometimes maintainers are not fully comfortable doing that,
since they may not have the bandwidth, or the setup, or the Rust
knowledge. In those cases, we typically ask if they would be willing
to have a co-maintainer (i.e. in their entry, e.g. like locking did),
or a sub-maintainer (i.e. in a new entry, e.g. like block did), that
would take care of the bulk of the work from them.

I think that is a nice middle-ground -- the advantage of doing it like
that is that you get the benefits of knowing best what is going on
without too much work (hopefully), and it may allow you to get more
and more involved over time and confident on what is going on with the
Rust callers, typical issues that appear, etc. Plus the sub-maintainer
gets to learn more about the subsystem, its timelines, procedures,
etc., which you may welcome (if you are looking for new people to get
involved).

I think that would be a nice middle-ground. As far as I understand,
Andreas would be happy to commit to maintain the Rust side as a
sub-maintainer (for instance). He would also need to make sure the
tree builds properly with Rust enabled and so on. He already does
something similar for Jens. Would that work for you?

You could take the patches directly with his RoBs or Acked-bys, for
instance. Or perhaps it makes more sense to take PRs from him (on the
Rust code only, of course), to save you more work. Andreas does not
send PRs to anyone yet, but I think it would be a good time for him to
start learning how to apply patches himself etc.

If not, then the last fallback would be putting it in the Rust tree as
a sub-entry or similar.

I hope that clarifies (and thanks whatever you decide!).

Cheers,
Miguel
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Frederic Weisbecker 9 months, 2 weeks ago
Le Sat, Feb 22, 2025 at 02:04:16PM +0100, Miguel Ojeda a écrit :
> On Fri, Feb 21, 2025 at 12:20 PM Frederic Weisbecker
> <frederic@kernel.org> wrote:
> >
> > I was thinking the patchset would be better routed towards the Rust tree?
> >
> > How do you guys proceed usually with bindings tree maintainance?
> 
> So far, what we have been doing is ask maintainers, first, if they
> would be willing take the patches themselves -- they are the experts
> of the subsystem, know what changes are incoming, etc. Some subsystems
> have done this (e.g. KUnit). That is ideal, because the goal is to
> scale and allows maintainers to be in full control.
> 
> Of course, sometimes maintainers are not fully comfortable doing that,
> since they may not have the bandwidth, or the setup, or the Rust
> knowledge. In those cases, we typically ask if they would be willing
> to have a co-maintainer (i.e. in their entry, e.g. like locking did),
> or a sub-maintainer (i.e. in a new entry, e.g. like block did), that
> would take care of the bulk of the work from them.
> 
> I think that is a nice middle-ground -- the advantage of doing it like
> that is that you get the benefits of knowing best what is going on
> without too much work (hopefully), and it may allow you to get more
> and more involved over time and confident on what is going on with the
> Rust callers, typical issues that appear, etc. Plus the sub-maintainer
> gets to learn more about the subsystem, its timelines, procedures,
> etc., which you may welcome (if you are looking for new people to get
> involved).
> 
> I think that would be a nice middle-ground. As far as I understand,
> Andreas would be happy to commit to maintain the Rust side as a
> sub-maintainer (for instance). He would also need to make sure the
> tree builds properly with Rust enabled and so on. He already does
> something similar for Jens. Would that work for you?
> 
> You could take the patches directly with his RoBs or Acked-bys, for
> instance. Or perhaps it makes more sense to take PRs from him (on the
> Rust code only, of course), to save you more work. Andreas does not
> send PRs to anyone yet, but I think it would be a good time for him to
> start learning how to apply patches himself etc.
> 
> If not, then the last fallback would be putting it in the Rust tree as
> a sub-entry or similar.
> 
> I hope that clarifies (and thanks whatever you decide!).

Yes this clarifies a lot, thanks for the detailed options.
I think it's preferrable that you guys maintain it because you
are the experts with these Rust bindings and you'll be much
more flexible committing and pushing to your own tree instead
of waiting on us lagging to comprehend the content of each
pull requests.

Just keep us in Cc so we stay in touch with what's going on.

Thanks a lot!

> 
> Cheers,
> Miguel
Re: [PATCH v8 00/14] hrtimer Rust API
Posted by Miguel Ojeda 9 months, 4 weeks ago
On Sat, Feb 22, 2025 at 2:04 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> send PRs to anyone yet, but I think it would be a good time for him to
> start learning how to apply patches himself etc.

By this, I meant taking care of the usual maintainer bits, not just
the actual "applying" (which he of course knows how to do!).

If you decide to go for PRs from him to you, I am happy to help
getting him up to speed if needed, too.

Cheers,
Miguel