Message-ID: <98de1e48-085b-6edb-f1cc-2bf85180196f@asahilina.net>
Date:   Wed, 22 Feb 2023 13:56:19 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH] rust: time: New module for timekeeping functions
Content-Language: en-US
To:     Thomas Gleixner <tglx@linutronix.de>,
        Boqun Feng <boqun.feng@gmail.com>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Gary Guo <gary@garyguo.net>,
        =?UTF-8?Q?Bj=c3=b6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
        John Stultz <jstultz@google.com>,
        Stephen Boyd <sboyd@kernel.org>, linux-kernel@vger.kernel.org,
        rust-for-linux@vger.kernel.org, asahi@lists.linux.dev
References: <20230221-gpu-up-time-v1-1-bf8fe74b7f55@asahilina.net>
 <87v8jvnqq4.ffs@tglx> <Y/TP6as7qqwfcI42@boqun-archlinux>
 <6aa15295-219b-225c-607d-e87e3d51d048@asahilina.net> <87bklmonbv.ffs@tglx>
From:   Asahi Lina <lina@asahilina.net>
In-Reply-To: <87bklmonbv.ffs@tglx>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 22/02/2023 04.00, Thomas Gleixner wrote:
>>>>> +}
>>>>> +
>>>>> +/// Returns the kernel time elapsed since boot, including time spent sleeping, as a [`Duration`].
>>>>> +pub fn ktime_get_boottime() -> Duration {
>>>>> +    Duration::from_nanos(
>>>>> +        // SAFETY: Function has no side effects and no variable inputs.
>>>>> +        unsafe { bindings::ktime_get_with_offset(bindings::tk_offsets_TK_OFFS_BOOT) }
>>>>
>>>> No. Please use ktime_get_boottime() and not the timekeeping internal function.
>>
>> `ktime_get_boottime()` is static inline, so it will need a manual helper
>> wrapper to be callable from Rust (at least until bindgen implements
>> automatic helper generation, which I hear is coming soon). I was trying
>> to avoid introducing even more helpers, since helpers.c is kind of
>> already getting out of hand in my branch with all the stuff that's
>> getting added... but I can add it if you don't want me to use
>> ktime_get_with_offset(). It'll also be slower this way though (since the
>> helper introduces another layer of function calling), and I figured of
>> all things we probably want time functions to be fast, since they tend
>> to get called a lot...
> 
> I can understand that, but please add proper comments and an explanation
> to the changelog then. That would have avoided tripping my taste sensor.
> 
> Be aware that my Rust foo is not even rusty it's close to non-existant.
> That's probaly true for many maintainers you need to interact with.

I'll add a comment ^^

Please do feel free to reach out and ask any questions about all this
crazy Rust stuff stuff! We're here to help and I know this is all new to
a lot of maintainers. I want people to be comfortable that we aren't
just creating more maintenance burden for everyone else.

That's also another conversation that we probably need to have, how do
we handle maintainership of Rust abstractions? I think Miguel mentioned
that ideally existing subsystem maintainers take over their bits of the
Rust side too over time, but of course a lot of people aren't going to
be comfortable with that if they don't have a lot of Rust experience
yet... personally I'm happy to sign up as co-maintainer or supporter of
the abstractions I contribute, or maybe we can just pool resources and
have people interested in Rust agree to help support this stuff for
every subsystem?

~~ Lina