public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@kernel.org>
To: Tim Bird <tim.bird@sony.com>,
	pmladek@suse.com, rostedt@goodmis.org, john.ogness@linutronix.de,
	senozhatsky@chromium.org
Cc: francesco@valla.it, geert@linux-m68k.org,
	shashankbalaji02@gmail.com, linux-embedded@vger.kernel.org,
	linux-kernel@vger.kernel.org, Tim Bird <tim.bird@sony.com>
Subject: Re: [PATCH v4 1/1] printk: fix zero-valued printk timestamps in early boot
Date: Wed, 15 Apr 2026 00:38:29 +0200	[thread overview]
Message-ID: <87qzohdv6i.ffs@tglx> (raw)
In-Reply-To: <20260410203741.997410-2-tim.bird@sony.com>

On Fri, Apr 10 2026 at 14:37, Tim Bird wrote:
> +
> +#include <linux/timekeeping.h>
> +#ifdef CONFIG_ARM64
> +#include <asm/sysreg.h>
> +#endif
> +
> +#ifdef CONFIG_EARLY_CYCLES_KHZ
> +static inline u64 early_unsafe_cycles(void)
> +{
> +#if defined(CONFIG_X86_64)
> +	/*
> +	 * This rdtsc may happen before secure TSC is initialized, and
> +	 * it is unordered. So please don't use this value for cryptography
> +	 * or after SMP is initialized.
> +	 */
> +	return rdtsc();
> +#elif defined(CONFIG_ARM64)
> +	return read_sysreg(cntvct_el0);
> +#elif defined(CONFIG_RISCV_TIMER)
> +	u64 val;
> +
> +	asm volatile("rdtime %0" : "=r"(val));
> +	return val;
> +#else
> +	return 0;
> +#endif
> +}

No. Generic code and generic headers have no business to implement any
architecture specific code and there is zero justification for
architecture specific #ifdefs in generic code.

Ask your favourite AI assistant for an opinion.

> +#define NS_PER_KHZ	1000000UL

The existing nomenclature for nanosecond related constants is NSEC_* 

> +
> +/* returns a nanosecond value based on early cycles */
> +static inline u64 early_times_ns(void)
> +{
> +	if (CONFIG_EARLY_CYCLES_KHZ)
> +		/*
> +		 * Note: the multiply must precede the division to avoid
> +		 * truncation and loss of resolution
> +		 * Don't use fancier MULT/SHIFT math here.  Since this is
> +		 * static, the compiler can optimize the math operations.
> +		 */
> +		return (early_unsafe_cycles() * NS_PER_KHZ) / CONFIG_EARLY_CYCLES_KHZ;

This code will result in a division by zero warning from any reasonable
compiler because this is evaluated _before_ it is eliminated.

> @@ -2294,6 +2295,8 @@ int vprintk_store(int facility, int level,
>  	 * timestamp with respect to the caller.
>  	 */
>  	ts_nsec = local_clock();
> +	if (unlikely(!ts_nsec))
> +		ts_nsec = early_times_ns();

I explained to you how this wants to be implemented to be useful and you
declared that you are unwilling to put the effort in.

My NAK for this disgusting and tasteless hack still applies.

Either you are willing to work with the community and the relevant
maintainers or you can keep your hackery maintained out of tree for
those who care about it and are willing to ignore the fallout. 

Thanks,

        tglx

  parent reply	other threads:[~2026-04-14 22:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-10 20:37 [PATCH v4 0/1] printk: fix zero-valued printk timestamps Tim Bird
2026-04-10 20:37 ` [PATCH v4 1/1] printk: fix zero-valued printk timestamps in early boot Tim Bird
2026-04-12 10:10   ` kernel test robot
2026-04-13 17:58     ` Bird, Tim
2026-04-13 20:17       ` David Laight
2026-04-14 22:48       ` Thomas Gleixner
2026-04-12 11:12   ` kernel test robot
2026-04-12 12:55   ` kernel test robot
2026-04-14  7:03   ` Geert Uytterhoeven
2026-04-14  9:51     ` Roberto A. Foglietta
2026-04-14 22:38   ` Thomas Gleixner [this message]
2026-04-15  0:19     ` Roberto A. Foglietta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87qzohdv6i.ffs@tglx \
    --to=tglx@kernel.org \
    --cc=francesco@valla.it \
    --cc=geert@linux-m68k.org \
    --cc=john.ogness@linutronix.de \
    --cc=linux-embedded@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=shashankbalaji02@gmail.com \
    --cc=tim.bird@sony.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox