From: Dou Liyang <douly.fnst@cn.fujitsu.com>
To: Pavel Tatashin <pasha.tatashin@oracle.com>,
<steven.sistare@oracle.com>, <daniel.m.jordan@oracle.com>,
<linux@armlinux.org.uk>, <schwidefsky@de.ibm.com>,
<heiko.carstens@de.ibm.com>, <john.stultz@linaro.org>,
<sboyd@codeaurora.org>, <x86@kernel.org>,
<linux-kernel@vger.kernel.org>, <mingo@redhat.com>,
<tglx@linutronix.de>, <hpa@zytor.com>, <peterz@infradead.org>,
<prarit@redhat.com>, <feng.tang@intel.com>, <pmladek@suse.com>,
<gnomes@lxorguk.ukuu.org.uk>, <linux-s390@vger.kernel.org>,
<boris.ostrovsky@oracle.com>, <jgross@suse.com>,
<pbonzini@redhat.com>
Subject: Re: [PATCH v14 20/25] x86/tsc: calibrate tsc only once
Date: Thu, 19 Jul 2018 13:33:27 +0800 [thread overview]
Message-ID: <4f0933a3-27b4-5468-83f5-af9b90842812@cn.fujitsu.com> (raw)
In-Reply-To: <20180718022211.6259-21-pasha.tatashin@oracle.com>
Hi, Pavel
I am sorry, I didn't point out typo clearly in the previous version.
Please see the concerns below. ;-)
At 07/18/2018 10:22 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>
> ---
> arch/x86/include/asm/tsc.h | 2 +-
> arch/x86/kernel/setup.c | 2 +-
> arch/x86/kernel/tsc.c | 86 ++++++++++++++++++++------------------
> 3 files changed, 48 insertions(+), 42 deletions(-)
>
> diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
> index 2701d221583a..c4368ff73652 100644
> --- a/arch/x86/include/asm/tsc.h
> +++ b/arch/x86/include/asm/tsc.h
> @@ -33,7 +33,7 @@ static inline cycles_t get_cycles(void)
> extern struct system_counterval_t convert_art_to_tsc(u64 art);
> extern struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns);
>
> -extern void tsc_early_delay_calibrate(void);
> +extern void tsc_early_init(void);
> extern void tsc_init(void);
> extern void mark_tsc_unstable(char *reason);
> extern int unsynchronized_tsc(void);
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 7490de925a81..5d32c55aeb8b 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1014,6 +1014,7 @@ void __init setup_arch(char **cmdline_p)
> */
> init_hypervisor_platform();
>
> + tsc_early_init();
> x86_init.resources.probe_roms();
>
> /* after parse_early_param, so could debug it */
> @@ -1199,7 +1200,6 @@ void __init setup_arch(char **cmdline_p)
>
> memblock_find_dma_reserve();
>
> - tsc_early_delay_calibrate();
> if (!early_xdbc_setup_hardware())
> early_xdbc_register_console();
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 186395041725..bc8eb82050a3 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -33,6 +33,8 @@ EXPORT_SYMBOL(cpu_khz);
> unsigned int __read_mostly tsc_khz;
> EXPORT_SYMBOL(tsc_khz);
>
> +#define KHZ 1000
> +
> /*
> * TSC can be unstable due to cpufreq or due to unsynced TSCs
> */
> @@ -1335,34 +1337,10 @@ static int __init init_tsc_clocksource(void)
> */
> device_initcall(init_tsc_clocksource);
>
> -void __init tsc_early_delay_calibrate(void)
> -{
> - unsigned long lpj;
> -
> - if (!boot_cpu_has(X86_FEATURE_TSC))
> - return;
> -
> - cpu_khz = x86_platform.calibrate_cpu();
> - tsc_khz = x86_platform.calibrate_tsc();
> -
> - tsc_khz = tsc_khz ? : cpu_khz;
> - if (!tsc_khz)
> - return;
> -
> - lpj = tsc_khz * 1000;
> - do_div(lpj, HZ);
> - loops_per_jiffy = lpj;
> -}
> -
> -void __init tsc_init(void)
> +static bool determine_cpu_tsc_frequncies(void)
this function need to be mark as __init.
And a typo here: frequency, s/frequncies/frequencies
> {
> - u64 lpj, cyc;
> - int cpu;
> -
> - if (!boot_cpu_has(X86_FEATURE_TSC)) {
> - setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
> - return;
> - }
> + /* Make sure that cpu and tsc are not already calibrated */
> + WARN_ON(cpu_khz || tsc_khz);
>
> cpu_khz = x86_platform.calibrate_cpu();
> tsc_khz = x86_platform.calibrate_tsc();
> @@ -1377,20 +1355,51 @@ void __init tsc_init(void)
> else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
> cpu_khz = tsc_khz;
>
> - if (!tsc_khz) {
> - mark_tsc_unstable("could not calculate TSC khz");
> - setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
> - return;
> - }
> + if (tsc_khz == 0)
> + return false;
>
> pr_info("Detected %lu.%03lu MHz processor\n",
> - (unsigned long)cpu_khz / 1000,
> - (unsigned long)cpu_khz % 1000);
> + (unsigned long)cpu_khz / KHZ,
> + (unsigned long)cpu_khz % KHZ);
>
> if (cpu_khz != tsc_khz) {
> pr_info("Detected %lu.%03lu MHz TSC",
> - (unsigned long)tsc_khz / 1000,
> - (unsigned long)tsc_khz % 1000);
> + (unsigned long)tsc_khz / KHZ,
> + (unsigned long)tsc_khz % KHZ);
> + }
this curly brackets can be removed
> + return true;
> +}
> +
> +static unsigned long get_loops_per_jiffy(void)
mark as __init as well.
> +{
> + unsigned long lpj = tsc_khz * KHZ;
> +
> + do_div(lpj, HZ);
> + return lpj;
> +}
> +
> +void __init tsc_early_init(void)
> +{
> + if (!boot_cpu_has(X86_FEATURE_TSC))
> + return;
> + if (!determine_cpu_tsc_frequncies())
> + return;
> + loops_per_jiffy = get_loops_per_jiffy();
> +}
> +
> +void __init tsc_init(void)
> +{
> + if (!boot_cpu_has(X86_FEATURE_TSC)) {
> + setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
> + return;
> + }
> +
> + if (!tsc_khz) {
> + /* We failed to determine frequencies earlier, try again */
> + if (!determine_cpu_tsc_frequncies()) {
Missing "setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER)" for local
APIC;
Thanks,
dou
> + mark_tsc_unstable("could not calculate TSC khz");
> + return;
> + }
> }
>
> /* Sanitize TSC ADJUST before cyc2ns gets initialized */
> @@ -1413,10 +1422,7 @@ void __init tsc_init(void)
> if (!no_sched_irq_time)
> enable_sched_clock_irqtime();
>
> - lpj = ((u64)tsc_khz * 1000);
> - do_div(lpj, HZ);
> - lpj_fine = lpj;
> -
> + lpj_fine = get_loops_per_jiffy();
> use_tsc_delay();
>
> check_system_tsc_reliable();
>
next prev parent reply other threads:[~2018-07-19 5:33 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-18 2:21 [PATCH v14 00/25] Early boot time stamps Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 01/25] x86/kvmclock: Remove memblock dependency Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 02/25] x86/kvmclock: Remove page size requirement from wall_clock Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 03/25] x86/kvmclock: Decrapify kvm_register_clock() Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 04/25] x86/kvmclock: Cleanup the code Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 05/25] x86/kvmclock: Mark variables __initdata and __ro_after_init Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 06/25] x86/kvmclock: Move kvmclock vsyscall param and init to kvmclock Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 07/25] x86/kvmclock: Switch kvmclock data to a PER_CPU variable Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 08/25] x86/kvmclock: Avoid TSC recalibration Pavel Tatashin
2018-07-18 11:14 ` Paolo Bonzini
2018-07-18 13:33 ` Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 09/25] x86: text_poke() may access uninitialized struct pages Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 10/25] x86: initialize static branching early Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 11/25] x86/CPU: Call detect_nopl() only on the BSP Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 12/25] x86/tsc: redefine notsc to behave as tsc=unstable Pavel Tatashin
2018-07-18 2:21 ` [PATCH v14 13/25] x86/xen/time: initialize pv xen time in init_hypervisor_platform Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 14/25] x86/xen/time: output xen sched_clock time from 0 Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 15/25] s390/time: add read_persistent_wall_and_boot_offset() Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 16/25] time: replace read_boot_clock64() with read_persistent_wall_and_boot_offset() Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 17/25] time: default boot time offset to local_clock() Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 18/25] s390/time: remove read_boot_clock64() Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 19/25] ARM/time: " Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 20/25] x86/tsc: calibrate tsc only once Pavel Tatashin
2018-07-19 5:33 ` Dou Liyang [this message]
2018-07-19 6:25 ` Thomas Gleixner
2018-07-19 6:48 ` Dou Liyang
2018-07-19 10:33 ` Peter Zijlstra
2018-07-19 11:01 ` Thomas Gleixner
2018-07-19 15:58 ` Pavel Tatashin
2018-07-19 16:19 ` Thomas Gleixner
2018-07-19 16:49 ` Pavel Tatashin
2018-07-19 18:38 ` Pavel Tatashin
2018-07-19 20:44 ` Thomas Gleixner
2018-07-19 20:46 ` Pavel Tatashin
2018-07-23 9:28 ` Alan Cox
2018-07-18 2:22 ` [PATCH v14 21/25] x86/tsc: initialize cyc2ns when tsc freq. is determined Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 22/25] x86/tsc: use tsc early Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 23/25] sched: move sched clock initialization and merge with generic clock Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 24/25] sched: early boot clock Pavel Tatashin
2018-07-19 10:39 ` Peter Zijlstra
2018-07-19 14:16 ` Pavel Tatashin
2018-07-18 2:22 ` [PATCH v14 25/25] sched: use static key for sched_clock_running Pavel Tatashin
2018-07-19 10:48 ` Peter Zijlstra
2018-07-19 14:24 ` Pavel Tatashin
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=4f0933a3-27b4-5468-83f5-af9b90842812@cn.fujitsu.com \
--to=douly.fnst@cn.fujitsu.com \
--cc=boris.ostrovsky@oracle.com \
--cc=daniel.m.jordan@oracle.com \
--cc=feng.tang@intel.com \
--cc=gnomes@lxorguk.ukuu.org.uk \
--cc=heiko.carstens@de.ibm.com \
--cc=hpa@zytor.com \
--cc=jgross@suse.com \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mingo@redhat.com \
--cc=pasha.tatashin@oracle.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=prarit@redhat.com \
--cc=sboyd@codeaurora.org \
--cc=schwidefsky@de.ibm.com \
--cc=steven.sistare@oracle.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).