Re: [PATCH] Revert "x86/tsc: Consolidate init code"

From: Thomas Gleixner <tglx@linutronix.de>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Dou Liyang <douly.fnst@cn.fujitsu.com>,
	Pasha Tatashin <Pavel.Tatashin@microsoft.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH] Revert "x86/tsc: Consolidate init code"
Date: Tue, 11 Sep 2018 22:56:05 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1809112227520.1427@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20180911121548.GW5565@intel.com>

[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]

On Tue, 11 Sep 2018, Ville Syrjälä wrote:

> On Mon, Sep 10, 2018 at 06:53:54PM +0200, Thomas Gleixner wrote:
> > On Mon, 10 Sep 2018, Ville Syrjälä wrote:
> > 
> > Good: 1718674.70 BogoMIPS (lpj=2863311530)
> > Bad:   859455.59 BogoMIPS (lpj=1431852151)
> > 
> > while both kernels agree on the CPU frequency of 996MHz. This pretty much
> > smells like the 32bit LPJ conversion bug which got fixed in rc3. Does the
> > problem persist with rc3?
> 
> Indeed looks to be fixed by commit 17f6bac22493 ("x86/tsc:
> Prevent result truncation on 32bit").

Not a surprise. That was pretty clear when I looked at dmesg because
bogomips were very bogus for both variants.

So can you now understand why I prefer a proper bug/regression report with
as much information as possible over a revert patch which lacks a proper
explanation and does not even fix the underlying issue at all?

Reverting that patch as you can see from bogus mips solves exactly
nothing. It's pure chance that it booted.

You could have spared my and your time by

    1) checking whether the problem persist in the latest upstream -rc
       first

    2) Providing useful information upfront

I don't care much about your time and how you think what's the best way to
get a discussion started, but I care very much about my time being wasted
for pointless discsussions which can be avoided more or less completely.

> I suppose we just got very lucky with older kernels.

The problem got initialy introduced with

  commit dd759d93f4dd ("x86/timers: Add simple udelay calibration")

but this was not fatal because it only affected the very early boot and was
fixed up by the correct LPJ calculation in tsc_init() before any serious
user was affected. I'll whip up a fix for the affected stable kernels
nevertheless.

Thanks

	tglx