Re: [GIT pull] printk updates for 4.15

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [GIT pull] printk updates for 4.15
Date: Mon, 13 Nov 2017 17:18:33 -0800	[thread overview]
Message-ID: <CA+55aFwUfA__6MgMgjENnx+_RYY2ZOOLiSx2ea1AvYhSZN+78A@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1711131023170.1851@nanos>

On Mon, Nov 13, 2017 at 1:36 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> Linus,
>
> please pull the latest core-printk-for-linus git tree from:
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-printk-for-linus
>
> This update adds the mechanisms to emit printk timestamps based on
> different clocks:
>
>   - scheduler clock (default)
>   - monotonic time
>   - boot time
>   - wall clock time
>
> This helps to correlate dmesg with user space log information, tracing,
> etc. This can be local correlation or in case of wall clock time correlated
> across machines, assumed that all machines are synchronized via
> NTP/PTP.

Honestly, this just seems bogus to me, particularly since it's a single choice.

The *sane* model would be to

 (a) continue to use the existing time that we always have
(local_clock()) in the printk timestamps, and don't confuse people
with the semantics of that field changing.

 (b) just emit a "synchronization printk" every once in a while, which
is obviously also using the same standard time source, but the line
actually _says_ what the other time sources are.

Then it's easy to see what the printk time source is, in relation to
any _number_ of other timesources. And if that synchronization printk
is nicely formatted, it will even be something that people appreciate
seeing in dmesg _irrespective_ of any actual synchronization issues.

And something that reads the journal could trivially pick up on the
synchronization printk line, and then correct the local timesource to
whatever internal journal timesource it wants to. And the important
thing is that because you just give *all* timesources in the
synchronization line, that choice isn't fixed by some random kernel
configuration or setting.

Instead, this seems to have a completely broken "pick one time source
model at random" approach, so now different machines will have
different models, and it will likely _break_ existing code that picks
the timesource from the kernel dmesg, unless you just pick the local
one.

That seems like bad design, and really stupid.

Am I missing something? Because as-is, this just seems like a horribly
bad feature to me. I'm not pulling it without some very good arguments
for this all.

                Linus