linux-kernel - Re: [PATCH v5 02/11] timekeeping: Add function to convert realtime to base clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCpP6Nd_iYtdgW+RyH1g7c-eyHR+j-LV4gv8rKWu9QkzhQ@mail.gmail.com>
Date: Tue, 19 Mar 2024 15:29:51 -0700
From: John Stultz <jstultz@...gle.com>
To: lakshmi.sowjanya.d@...el.com
Cc: tglx@...utronix.de, giometti@...eenne.com, corbet@....net, 
	linux-kernel@...r.kernel.org, x86@...nel.org, netdev@...r.kernel.org, 
	linux-doc@...r.kernel.org, intel-wired-lan@...ts.osuosl.org, 
	andriy.shevchenko@...ux.intel.com, eddie.dong@...el.com, 
	christopher.s.hall@...el.com, jesse.brandeburg@...el.com, davem@...emloft.net, 
	alexandre.torgue@...s.st.com, joabreu@...opsys.com, mcoquelin.stm32@...il.com, 
	perex@...ex.cz, linux-sound@...r.kernel.org, anthony.l.nguyen@...el.com, 
	peter.hilber@...nsynergy.com, pandith.n@...el.com, 
	mallikarjunappa.sangannavar@...el.com, subramanian.mohan@...el.com, 
	basavaraj.goudar@...el.com, thejesh.reddy.t.r@...el.com
Subject: Re: [PATCH v5 02/11] timekeeping: Add function to convert realtime to
 base clock

On Tue, Mar 19, 2024 at 6:06 AM <lakshmi.sowjanya.d@...el.com> wrote:
>
> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
>
> PPS(Pulse Per Second) generates signals in realtime, but Timed IO
> hardware understands time in base clock reference. Add an interface,
> ktime_real_to_base_clock() to convert realtime to base clock.
>
> Convert the base clock to the system clock using convert_base_to_cs() in
> get_device_system_crosststamp().
>
> Add the helper function timekeeping_clocksource_has_base(), to check
> whether the current clocksource has the same base clock. This will be
> used by Timed IO device to check if the base clock is X86_ART(Always
> Running Timer).
>
> Co-developed-by: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Co-developed-by: Christopher S. Hall <christopher.s.hall@...el.com>
> Signed-off-by: Christopher S. Hall <christopher.s.hall@...el.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
> ---
>  include/linux/timekeeping.h |   6 +++
>  kernel/time/timekeeping.c   | 105 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 109 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
> index 7e50cbd97f86..1b2a4a37bf93 100644
> --- a/include/linux/timekeeping.h
> +++ b/include/linux/timekeeping.h
> @@ -275,12 +275,18 @@ struct system_device_crosststamp {
>   *             timekeeping code to verify comparability of two cycle values.
>   *             The default ID, CSID_GENERIC, does not identify a specific
>   *             clocksource.
> + * @nsecs:     @cycles is in nanoseconds.
>   */
>  struct system_counterval_t {
>         u64                     cycles;
>         enum clocksource_ids    cs_id;
> +       bool                    nsecs;

Apologies, this is a bit of an annoying bikeshed request, but maybe
use_nsecs here?
There are plenty of places where nsecs fields hold actual nanoseconds,
so what you have might be easy to misread in the future.

Also, at least in this series, I'm not sure I see where this nsecs
value gets set? Maybe something to split out and add in a separate
patch, where its use is more clear?

> +bool timekeeping_clocksource_has_base(enum clocksource_ids id)
> +{
> +       unsigned int seq;
> +       bool ret;
> +
> +       do {
> +               seq = read_seqcount_begin(&tk_core.seq);
> +               ret = tk_core.timekeeper.tkr_mono.clock->base ?
> +               tk_core.timekeeper.tkr_mono.clock->base->id == id : false;

Again, bikeshed nit: I know folks like ternaries for conciseness, but
once you've crossed a single line, I'd often prefer to read an if
statement.

thanks
-john