The unsynchronized_tsc() eventually checks num_possible_cpus(), and
if the system is non-Intel and the number of possible CPUs is greater
than one, assumes that TSCs are unsynchronized. This despite the
comment saying "assume multi socket systems are not synchronized",
that is, socket rather than CPU. This behavior was preserved by
commit 8fbbc4b45ce3 ("x86: merge tsc_init and clocksource code") and
by the previous relevant commit 7e69f2b1ead2 ("clocksource: Remove the
update callback").
The clocksource drivers were added by commit 5d0cf410e94b ("Time: i386
Clocksource Drivers") back in 2006, and the comment still said "socket"
rather than "CPU".
Therefore, bravely (and perhaps foolishly) make the code match the
comment.
Note that it is possible to bypass both code and comment by booting
with tsc=reliable, but this also disables the clocksource watchdog,
which is undesirable when trust in the TSC is strictly limited.
Changes since v1:
o Forward-port to v6.9-rc1.
Reported-by: Zhengxu Chen <[email protected]>
Reported-by: Danielle Costantino <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Feng Tang <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Neeraj Upadhyay <[email protected]>
Cc: <[email protected]>
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 5a69a49acc963..e938b990bea19 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1289,7 +1289,7 @@ int unsynchronized_tsc(void)
*/
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
/* assume multi socket systems are not synchronized: */
- if (num_possible_cpus() > 1)
+ if (nr_online_nodes > 1)
return 1;
}
On Wed, Apr 03, 2024 at 12:10:41PM -0700, Paul E. McKenney wrote:
> The unsynchronized_tsc() eventually checks num_possible_cpus(), and
> if the system is non-Intel and the number of possible CPUs is greater
> than one, assumes that TSCs are unsynchronized. This despite the
> comment saying "assume multi socket systems are not synchronized",
> that is, socket rather than CPU. This behavior was preserved by
> commit 8fbbc4b45ce3 ("x86: merge tsc_init and clocksource code") and
> by the previous relevant commit 7e69f2b1ead2 ("clocksource: Remove the
> update callback").
>
> The clocksource drivers were added by commit 5d0cf410e94b ("Time: i386
> Clocksource Drivers") back in 2006, and the comment still said "socket"
> rather than "CPU".
>
> Therefore, bravely (and perhaps foolishly) make the code match the
> comment.
>
> Note that it is possible to bypass both code and comment by booting
> with tsc=reliable, but this also disables the clocksource watchdog,
> which is undesirable when trust in the TSC is strictly limited.
>
> Changes since v1:
>
> o Forward-port to v6.9-rc1.
>
> Reported-by: Zhengxu Chen <[email protected]>
> Reported-by: Danielle Costantino <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Feng Tang <[email protected]>
> Cc: Waiman Long <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Neeraj Upadhyay <[email protected]>
> Cc: <[email protected]>
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 5a69a49acc963..e938b990bea19 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1289,7 +1289,7 @@ int unsynchronized_tsc(void)
> */
> if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
> /* assume multi socket systems are not synchronized: */
> - if (num_possible_cpus() > 1)
> + if (nr_online_nodes > 1)
Regarding package/socket number, Thomas' topology refactoring patchset
(merged in 6.9-rc1) makes topology_max_packages() more accurate than
nr_online_nodes(), more details in https://lore.kernel.org/lkml/[email protected]/
Thanks,
Feng
On Thu, Apr 04, 2024 at 01:08:56PM +0800, Feng Tang wrote:
> On Wed, Apr 03, 2024 at 12:10:41PM -0700, Paul E. McKenney wrote:
> > The unsynchronized_tsc() eventually checks num_possible_cpus(), and
> > if the system is non-Intel and the number of possible CPUs is greater
> > than one, assumes that TSCs are unsynchronized. This despite the
> > comment saying "assume multi socket systems are not synchronized",
> > that is, socket rather than CPU. This behavior was preserved by
> > commit 8fbbc4b45ce3 ("x86: merge tsc_init and clocksource code") and
> > by the previous relevant commit 7e69f2b1ead2 ("clocksource: Remove the
> > update callback").
> >
> > The clocksource drivers were added by commit 5d0cf410e94b ("Time: i386
> > Clocksource Drivers") back in 2006, and the comment still said "socket"
> > rather than "CPU".
> >
> > Therefore, bravely (and perhaps foolishly) make the code match the
> > comment.
> >
> > Note that it is possible to bypass both code and comment by booting
> > with tsc=reliable, but this also disables the clocksource watchdog,
> > which is undesirable when trust in the TSC is strictly limited.
> >
> > Changes since v1:
> >
> > o Forward-port to v6.9-rc1.
> >
> > Reported-by: Zhengxu Chen <[email protected]>
> > Reported-by: Danielle Costantino <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: Dave Hansen <[email protected]>
> > Cc: "H. Peter Anvin" <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Feng Tang <[email protected]>
> > Cc: Waiman Long <[email protected]>
> > Cc: John Stultz <[email protected]>
> > Cc: Neeraj Upadhyay <[email protected]>
> > Cc: <[email protected]>
> >
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index 5a69a49acc963..e938b990bea19 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
> > @@ -1289,7 +1289,7 @@ int unsynchronized_tsc(void)
> > */
> > if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
> > /* assume multi socket systems are not synchronized: */
> > - if (num_possible_cpus() > 1)
> > + if (nr_online_nodes > 1)
>
> Regarding package/socket number, Thomas' topology refactoring patchset
> (merged in 6.9-rc1) makes topology_max_packages() more accurate than
> nr_online_nodes(), more details in https://lore.kernel.org/lkml/[email protected]/
Thank you, and updated with attribution.
Thanx, Paul
The unsynchronized_tsc() eventually checks num_possible_cpus(), and
if the system is non-Intel and the number of possible CPUs is greater
than one, assumes that TSCs are unsynchronized. This despite the
comment saying "assume multi socket systems are not synchronized",
that is, socket rather than CPU. This behavior was preserved by
commit 8fbbc4b45ce3 ("x86: merge tsc_init and clocksource code") and
by the previous relevant commit 7e69f2b1ead2 ("clocksource: Remove the
update callback").
The clocksource drivers were added by commit 5d0cf410e94b ("Time: i386
Clocksource Drivers") back in 2006, and the comment still said "socket"
rather than "CPU".
Therefore, bravely (and perhaps foolishly) make the code match the
comment.
Note that it is possible to bypass both code and comment by booting
with tsc=reliable, but this also disables the clocksource watchdog,
which is undesirable when trust in the TSC is strictly limited.
[ paulmck: Switch from nr_online_nodes to topology_max_packages() per Feng Tang feedback. ]
Reported-by: Zhengxu Chen <[email protected]>
Reported-by: Danielle Costantino <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Feng Tang <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Neeraj Upadhyay <[email protected]>
Cc: <[email protected]>
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 5a69a49acc963..0e7f44cc168e2 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1289,7 +1289,7 @@ int unsynchronized_tsc(void)
*/
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
/* assume multi socket systems are not synchronized: */
- if (num_possible_cpus() > 1)
+ if (topology_max_packages() > 1)
return 1;
}
The unsynchronized_tsc() eventually checks num_possible_cpus(), and
if the system is non-Intel and the number of possible CPUs is greater
than one, assumes that TSCs are unsynchronized. This despite the
comment saying "assume multi socket systems are not synchronized",
that is, socket rather than CPU. This behavior was preserved by
commit 8fbbc4b45ce3 ("x86: merge tsc_init and clocksource code") and
by the previous relevant commit 7e69f2b1ead2 ("clocksource: Remove the
update callback").
The clocksource drivers were added by commit 5d0cf410e94b ("Time: i386
Clocksource Drivers") back in 2006, and the comment still said "socket"
rather than "CPU".
Therefore, bravely (and perhaps foolishly) make the code match the
comment.
Note that it is possible to bypass both code and comment by booting
with tsc=reliable, but this also disables the clocksource watchdog,
which is undesirable when trust in the TSC is strictly limited.
[ paulmck: Switch from nr_online_nodes to topology_max_packages() per Feng Tang feedback. ]
Reported-by: Zhengxu Chen <[email protected]>
Reported-by: Danielle Costantino <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Feng Tang <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Neeraj Upadhyay <[email protected]>
Cc: <[email protected]>
---
The only change since v3 is forward-porting to v6.10-rc1.
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 06b170759e5bf..3640bf119f127 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1289,7 +1289,7 @@ int unsynchronized_tsc(void)
*/
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
/* assume multi socket systems are not synchronized: */
- if (num_possible_cpus() > 1)
+ if (topology_max_packages() > 1)
return 1;
}