Re: [PATCH v9 01/13] rust: hrtimer: introduce hrtimer support

[Andreas Hindborg <a.hindborg@kernel.org>]

"Boqun Feng" <boqun.feng@gmail.com> writes:

> On Mon, Feb 24, 2025 at 07:58:04PM +0100, Andreas Hindborg wrote:
>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>>
>> > On Mon, Feb 24, 2025 at 05:45:03PM +0100, Miguel Ojeda wrote:
>> >> On Mon, Feb 24, 2025 at 5:31 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>> >> >
>> >> > On Mon, Feb 24, 2025 at 05:23:59PM +0100, Miguel Ojeda wrote:
>> >> > >
>> >> > > side -- Andreas and I discussed it the other day. The description of
>> >> > > the issue has some lines, but perhaps the commit message could
>> >> >
>> >> > Do you have a link to the issue?
>> >>
>> >> Sorry, I meant "description of the symbol", i.e. the description field
>> >> in the patch.
>> >>
>> >
>> > Oh, I see. Yes, the patch description should provide more information
>> > about what the kconfig means for hrtimer maintainers' development.
>>
>> Right, I neglected to update the commit message. I will do that if we
>> have another version.
>>
>> >
>> >> > I asked because hrtimer API is always available regardless of the
>> >> > configuration, and it's such a core API, so it should always be there
>> >> > (Rust or C).
>> >>
>> >> It may not make sense for something that is always built on the C
>> >> side, yeah. I think the intention here may be that one can easily
>> >> disable it while "developing" a change on the C side. I am not sure
>> >> what "developing" means here, though, and we need to be careful --
>> >> after all, Kconfig options are visible to users and they do not care
>> >> about that.
>> >>
>> >
>> > Personally, I don't think CONFIG_RUST_HRTIMER is necessarily because as
>> > you mentioned below, people can disable Rust entirely during
>> > "developing".
>> >
>> > And if I understand the intention correctly, the CONFIG_RUST_HRTIMER
>> > config provides hrtimer maintainers a way that they could disable Rust
>> > hrtimer abstraction (while enabling other Rust component) when they're
>> > developing a change on the C side, right? If so, it's hrtimer
>> > maintainers' call, and this patch should provide more information on
>> > this.
>> >
>> > Back to my personal opinion, I don't think this is necessary ;-)
>> > Particularly because I can fix if something breaks Rust side, and I'm
>> > confident and happy to do so for hrtimer ;-)
>>
>> As Miguel said, the idea for this came up in the past week in one of the
>> mega threads discussing rust in general. We had a lot of "what happens
>> if I change something in my subsystem and that breaks rust" kind of
>> discussions.
>>
>
> So far we haven't heard such a question from hrtimer maintainers, I
> would only add such a kconfig if explicitly requested.

It gives flexibility and has no negative side effects. Of course, if it
is unwanted, we can just remove it. But I would like to understand the
deeper rationale.


>
>> For subsystems where the people maintaining the C subsystem is not the
>> same people maintaining the Rust abstractions, this switch might be
>> valuable. It would allow making breaking changes to the C code of a
>> subsystem without refactoring the Rust code in the same sitting. Rather
>
> That's why I asked Frederic to be a reviewer of Rust hrtimer API. In
> longer-term, more and more people will get more or less Rust knowledge,
> and I'd argue that's the direction we should head to. So my vision is a
> significant amount of core kernel developers would be able to make C and
> Rust changes at the same time. It's of course not mandatory, but it's
> better collaboration.

Having this switch does not prevent longer term plans or change
directions of anything. It's simply a convenience feature made
available. I also expect the future you envision. But it is an
envisioned _future_. It is not the present reality.

>
>> than having to disable rust entirely - or going and commenting out lines
>> in the kernel crate - I think it is better to provide an option to just
>> disable building these particular bindings.
>>
>> This has nothing to do with general policies related to breakage between
>> Rust and C code, and how to fix such breakage in a timely manner. It is
>> simply a useful switch for disabling part of the build so that people
>> can move on with their business, while someone else scrambles to fix
>> whatever needs fixing on the Rust side.
>>
>
> It's of course up to hrtimer maintainers. But I personally nack this
> kconfig, because it's not necessary, and hrtimer API has been stable for
> a while.

Having the switch is fine for me, removing it is fine as well. It's just
an added convenience that might come in handy. But having this kconfig
very close to zero overhead, so I do not really understand your
objection. I would like to better understand your reasoning.


Best regards,
Andreas Hindborg