| Message ID | 20240129235646.3171983-6-boqun.feng@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 19a51d54b3a48c9ef01b378c2007c2015341fa69 |
| Headers | show |
| Series | Misc RCU update for v6.9 | expand |
On Mon, Jan 29, 2024 at 03:56:38PM -0800, Boqun Feng wrote: > From: "Paul E. McKenney" <paulmck@kernel.org> > > 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. > > Reported-by: Zhengxu Chen <zhxchen17@meta.com> > Reported-by: Danielle Costantino <dcostantino@meta.com> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Feng Tang <feng.tang@intel.com> > Cc: Waiman Long <longman@redhat.com> > Cc: John Stultz <jstultz@google.com> > Cc: <x86@kernel.org> Hello, Boqun, Please drop this one, as I never got an ack from the maintainers, and quite possibly for good reason. ;-) Thanx, Paul > --- > arch/x86/kernel/tsc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > index 15f97c0abc9d..d45084c6a15e 100644 > --- a/arch/x86/kernel/tsc.c > +++ b/arch/x86/kernel/tsc.c > @@ -1287,7 +1287,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; > } > > -- > 2.43.0 >
On Mon, Feb 05, 2024 at 12:03:33PM -0800, Paul E. McKenney wrote: > On Mon, Jan 29, 2024 at 03:56:38PM -0800, Boqun Feng wrote: > > From: "Paul E. McKenney" <paulmck@kernel.org> > > > > 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. > > > > Reported-by: Zhengxu Chen <zhxchen17@meta.com> > > Reported-by: Danielle Costantino <dcostantino@meta.com> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Feng Tang <feng.tang@intel.com> > > Cc: Waiman Long <longman@redhat.com> > > Cc: John Stultz <jstultz@google.com> > > Cc: <x86@kernel.org> > > Hello, Boqun, > > Please drop this one, as I never got an ack from the maintainers, and > quite possibly for good reason. ;-) > Got it I will drop it in the PR, the topic branch has been updated: git://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git rcu-misc.2024.02.05a Regards, Boqun > Thanx, Paul > > > --- > > arch/x86/kernel/tsc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > > index 15f97c0abc9d..d45084c6a15e 100644 > > --- a/arch/x86/kernel/tsc.c > > +++ b/arch/x86/kernel/tsc.c > > @@ -1287,7 +1287,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; > > } > > > > -- > > 2.43.0 > >
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c index 15f97c0abc9d..d45084c6a15e 100644 --- a/arch/x86/kernel/tsc.c +++ b/arch/x86/kernel/tsc.c @@ -1287,7 +1287,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; }