Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180712000419.5165-1-pasha.tatashin@oracle.com>
 <20180712000419.5165-14-pasha.tatashin@oracle.com> <928b8490-89d2-46a3-8596-16751bcd12db@cn.fujitsu.com>
 <CAGM2reb-MwiT8qoiSkXCjAAM46JOYL9BP+U3q7PMTcxp9EiK7w@mail.gmail.com> <01812492-b88b-4436-570b-ef776eaafc6a@cn.fujitsu.com>
In-Reply-To: <01812492-b88b-4436-570b-ef776eaafc6a@cn.fujitsu.com>
From:   Pavel Tatashin <pasha.tatashin@oracle.com>
Date:   Mon, 16 Jul 2018 09:35:40 -0400
Message-ID: <CAGM2reaU1GouLvUfW_dbs7gFiYCMX=X+Vag2RnuofmmgS4_TdA@mail.gmail.com>
Subject: Re: [PATCH v13 13/18] x86/tsc: calibrate tsc only once
To:     douly.fnst@cn.fujitsu.com
Cc:     Steven Sistare <steven.sistare@oracle.com>,
        Daniel Jordan <daniel.m.jordan@oracle.com>,
        linux@armlinux.org.uk, schwidefsky@de.ibm.com,
        Heiko Carstens <heiko.carstens@de.ibm.com>,
        John Stultz <john.stultz@linaro.org>, sboyd@codeaurora.org,
        x86@kernel.org, LKML <linux-kernel@vger.kernel.org>,
        mingo@redhat.com, tglx@linutronix.de, hpa@zytor.com,
        peterz@infradead.org, prarit@redhat.com, feng.tang@intel.com,
        Petr Mladek <pmladek@suse.com>, gnomes@lxorguk.ukuu.org.uk,
        linux-s390@vger.kernel.org, boris.ostrovsky@oracle.com,
        jgross@suse.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 16, 2018 at 5:33 AM Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>
>
>
> At 07/13/2018 07:30 PM, Pavel Tatashin wrote:
> > On Fri, Jul 13, 2018 at 3:24 AM Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >>
> >>
> >> At 07/12/2018 08:04 AM, Pavel Tatashin wrote:
> >>> During boot tsc is calibrated twice: once in tsc_early_delay_calibrate(),
> >>> and the second time in tsc_init().
> >>>
> >>> Rename tsc_early_delay_calibrate() to tsc_early_init(), and rework it so
> >>> the calibration is done only early, and make tsc_init() to use the values
> >>> already determined in tsc_early_init().
> >>>
> >>> Sometimes it is not possible to determine tsc early, as the subsystem that
> >>> is required is not yet initialized, in such case try again later in
> >>> tsc_init().
> >>>
> >>> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> >>> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
> >>
> >> Hi Pavel,
> >>
> >> Aha, a complex solution for a simple problem! ;-) And I did find any
> >> benefits of doing that. did I miss something?
> >
> > Hi Dou,
> >
> > I had this in previous version:  init early, and unconditionally
> > re-init later (which required to resync sched clocks for continuity,
> > and check for some other corner cases). Thomas did not like the idea,
> > as it is less deterministic: it leads for code to work by accident,
> > where we might get one tsc frequency early and another later, and so
> > on. The request was to initialize only once, and if that fails do it
> > again later. This way, if early initialization is broken, we will know
> > and fix it.
> >
>
> Hi Pavel,
>
> Yes, right, I have seen the purpose in v12.
>
> >>
> >> As the cpu_khz and tsc_khz are global variables and the tsc_khz may
> >> be reset to cpu_khz. How about the following patch.
> >
> > Could you please explain where you think this patch can be applied,
> > and what it fixes?
> >
>
> This patch is just an simple alternative to realize what you want in
> your patch. your patch is also good but complex, and need some scrub.
> eg:
>    - Is it suitable to using the WARN_ON()
>    - the name of determine_cpu_tsc_frequncies() function,
>     s/frequncies/frequencies/. BTW, How about tsc_calibrate_frequencies()
>    - ...
>
> BTW, before this patch, seems we need make sure xen should work well, I
> can investigate and try to test if we can also move the pagetable_init()
> to the front of tsc_early_init() for you.

v13 has xen patches, so xen timestamps work early as well now.

Thank you,
Pavel