Message-ID: <CAMuHMdWBb3y=2jYv8PzWact=KOhZGn4DFBP7XpxhdjAmDcDeBg@mail.gmail.com>
Date: Tue, 27 Jan 2026 09:10:31 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Bird, Tim" <Tim.Bird@...y.com>
Cc: "pmladek@...e.com" <pmladek@...e.com>, "rostedt@...dmis.org" <rostedt@...dmis.org>, 
	"john.ogness@...uxtronix.de" <john.ogness@...uxtronix.de>, 
	"senozhatsky@...omium.org" <senozhatsky@...omium.org>, "francesco@...la.it" <francesco@...la.it>, 
	"linux-embedded@...r.kernel.org" <linux-embedded@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] printk: fix zero-valued printk timestamps in early boot

Hi Tim,

On Mon, 26 Jan 2026 at 18:11, Bird, Tim <Tim.Bird@...y.com> wrote:
> > From: Geert Uytterhoeven <geert@...ux-m68k.org>
> > On Sat, 24 Jan 2026 at 20:41, Tim Bird <tim.bird@...y.com> wrote:
> > > During early boot, printk timestamps are reported as zero before
> > > kernel timekeeping starts (e.g. before time_init()).  This
> > > hinders boot-time optimization efforts.  This period is about 400
> > > milliseconds for many current desktop and embedded machines
> > > running Linux.
> > >
> > > Add support to save cycles during early boot, and output correct
> > > timestamp values after timekeeping is initialized.  get_cycles()
> > > is operational on arm64 and x86_64 from kernel start.  Add code
> > > and variables to save calibration values used to later convert
> > > cycle counts to time values in the early printks.  Add a config
> > > to control the feature.
> > >
> > > This yields non-zero timestamps for printks from the very start
> > > of kernel execution.  The timestamps are relative to the start of
> > > the architecture-specified counter used in get_cycles
> > > (e.g. the TSC on x86_64 and cntvct_el0 on arm64).
> > >
> > > All timestamps reflect time from power-on instead of time from
> > > the kernel's timekeeping initialization.
> > >
> > > Signed-off-by: Tim Bird <tim.bird@...y.com>
> > > ---
> > > V1 -> V2
> > >   Remove calibration CONFIG vars
> > >   Add 'depends on' to restrict arches (to handle ppc bug)
> > >   Add early_ts_offset to avoid discontinuity
> > >   Save cycles in ts_nsec, and convert on output
> > >   Move conditional code to include file (early_times.h)
> >
> > Thanks for the update!
> >
> > > --- /dev/null
> > > +++ b/include/linux/early_times.h
> > > @@ -0,0 +1,48 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +#ifndef _KERNEL_PRINTK_EARLY_TIMES_H
> > > +#define _KERNEL_PRINTK_EARLY_TIMES_H
> > > +
> > > +#include <linux/timex.h>
> > > +
> > > +#if defined(CONFIG_EARLY_PRINTK_TIMES)
> > > +extern u32 early_mult, early_shift;
> > > +extern u64 early_ts_offset;
> > > +
> > > +static inline u64 early_cycles(void)
> > > +{
> > > +       return ((u64)get_cycles() | (1ULL << 63));
> >
> > No need to cast to u64, as the second operand of the OR is u64 anyway.
> > BIT_ULL(63)
> >
> > I think it would be good to have a #define for this at the top.
>
> I'll look at this.  Is BIT_ULL(63) preferred over (1ULL << 63)?

When you refer to the bit value, yes: BIT() for unsigned long, BIT_ULL()
for unsigned long long; recently we got BIT_U{8,16,32,64}(), too).

> Do you think something like "HIGH_BIT63" would be good enough?

I'd name it for what it means, not what it does, e.g. EARLY_TS_FLAG?

> > > +}
> > > +
> > > +static inline u64 adjust_early_ts(u64 ts)
> > > +{
> > > +       /* High bit means ts is a cycle count */
> > > +       if (unlikely(ts & (1ULL << 63)))
> > > +               /*
> > > +                * mask high bit and convert to ns
> > > +                * Note that early_mult may be 0, but that's OK because
> > > +                * we'll just multiply by 0 and return 0. This will
> > > +                * only occur if we're outputting a printk message
> > > +                * before the calibration of the early timestamp.
> > > +                * Any output after user space start (eg. from dmesg or
> > > +                * journalctl) will show correct values.
> > > +                */
> > > +               return (((ts & ~(1ULL << 63)) * early_mult) >> early_shift);
> >
> > Please use the mul_u64_u32_shr() helper.
> OK.  I did not know about that.
>
> I can check, but do you know offhand if timestamps from local_clock() on 32-bit systems are
> always 64-bit nanoseconds?  I assume so looking at the printk code and
> making some assumptions.  (But that's dangerous.)

I am not 100% sure, but I think they are.  Note that on systems
without high-resolution timers, they may increment in large steps,
e.g. by 10000000 if HZ=100.

> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -777,6 +777,18 @@ config IKHEADERS
> > >           or similar programs.  If you build the headers as a module, a module called
> > >           kheaders.ko is built which can be loaded on-demand to get access to headers.
> > >
> > > +config EARLY_PRINTK_TIMES
> > > +       bool "Show non-zero printk timestamps early in boot"
> > > +       default y
> > > +       depends on PRINTK
> > > +       depends on ARM64 || X86_64
> >
> > So for now this is limited to (a few) 64-bit platforms...
>
> Yes, but it really shouldn't be.  I got spooked when 0-day told me that the code
> wouldn't link on powerpc, so I restricted it to just machines I was actually testing.
> But this should work on some ARM32 and most x8632 platforms.  Actually, it should
> work on anything that has a cycle-counter from kernel start (some RISC machine
> might qualify as well).  However, I've seen a few cases where some platforms require
> kernel initialization of their cycle-counter, and I wanted to play it safe.
>
> If such platforms are well-behaved and return 0 before they are initialized, it should
> still be safe to turn this on - it just won't have any effect.

At least on m68k, get_cycles() always returns zero ;-)

> I plan to do some more investigation of the powerpc error.  It was
> powerpc-linux-ld: init/main.o: in function `kernel_init':
> main.c:(.ref.text+0x144): undefined reference to `__udivdi3'
>
> from the definition of clocks_calc_mult_shift(), which seems to indicate a bug
> in the powerpc code.  In the long run, I should try to track down that bug rather than
> exclude a bunch of other (likely-working) arches.  But I was playing it conservative for now.

An undefined reference to __udivdi3 means you are using a 64-by-32
division, for which you should always use the helpers from
<linux/math64.h>.  Even on 64-bit, the helpers may generate better code.

> > > --- a/init/main.c
> > > +++ b/init/main.c

> > > +
> > > +       /* set calibration data for early_printk_times */
> > > +       end_cycles = get_cycles();
> > > +       end_ns = local_clock();
> > > +       clocks_calc_mult_shift(&early_mult, &early_shift,
> > > +               ((end_cycles - start_cycles) * NSEC_PER_SEC)/(end_ns - start_ns),
> > > +               NSEC_PER_SEC, 50);
> >
> > mul_u64_u64_div_u64() is probably the best helper that is available
> > (there is no mul_ulong_u32_div_u64()).
>
> What would the mul_u64_u64_div_u64 do if cycles_t is u32?
> Should it sill all work, just not optimized?

It should still work.
arch/x86/include/asm/div64.h uses divq, so it is up to Intel or AMD.
Everything else uses lib/math/div64.c.
If need, you can add an optimized version for e.g. arm64, too.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists