lkml.org 
[lkml]   [2018]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v13 13/18] x86/tsc: calibrate tsc only once
From
Date


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.

Thanks,
dou

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index da1dbd99cb6e..74cb16d89e25 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1197,12 +1197,12 @@ void __init setup_arch(char **cmdline_p)

memblock_find_dma_reserve();

+ x86_init.paging.pagetable_init();
+
tsc_early_delay_calibrate();
if (!early_xdbc_setup_hardware())
early_xdbc_register_console();

- x86_init.paging.pagetable_init();
-
kasan_init();

/*
> Thank you,
> Pavel
>
>>
>> Thanks,
>> dou
>> ------------------------8<-----------------------------------
>>
>> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
>> index 74392d9d51e0..e54fa1037d45 100644
>> --- a/arch/x86/kernel/tsc.c
>> +++ b/arch/x86/kernel/tsc.c
>> @@ -1370,8 +1370,10 @@ void __init tsc_init(void)
>> return;
>> }
>>
>> - cpu_khz = x86_platform.calibrate_cpu();
>> - tsc_khz = x86_platform.calibrate_tsc();
>> + if (!tsc_khz) {
>> + cpu_khz = x86_platform.calibrate_cpu();
>> + tsc_khz = x86_platform.calibrate_tsc();
>> + }
>>
>> /*
>> * Trust non-zero tsc_khz as authorative,
>
>
>


\
 
 \ /
  Last update: 2022-09-17 16:04    [W:0.504 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site