Re: [Patch v2 2/2] x86/tsc: use logical_packages as a better estimation of socket numbers

[Zhang, Rui]

On Thu, 2023-06-15 at 11:20 +0200, Peter Zijlstra wrote:
> On Tue, Jun 13, 2023 at 01:25:23PM +0800, Feng Tang wrote:
> > Commit b50db7095fe0 ("x86/tsc: Disable clocksource watchdog for TSC
> > on qualified platorms") was introduced to solve problem that
> > sometimes TSC clocksource is wrongly judged as unstable by watchdog
> > like 'jiffies', HPET, etc.
> > 
> > In it, the hardware socket number is a key factor for judging
> > whether
> > to disable the watchdog for TSC, and 'nr_online_nodes' was chosen
> > as
> > an estimation due to it is needed in early boot phase before
> > registering 'tsc-early' clocksource, where all none-boot CPUs are
> > not
> > brought up yet.
> > 
> > In recent patch review, Dave and Rui pointed out there are many
> > cases
> > in which 'nr_online_nodes' is not accurate, like:
> > 
> > * numa emulation (numa=fake=4 etc.)
> > * numa=off
> > * platforms with CPU-less HBM nodes, CPU-less Optane memory nodes.
> > * SNC (sub-numa cluster) mode enabled
> > * 'nr_cpus=' and 'maxcpus=' kernel cmdline parameter setup
> > 
> > Peter Zijlstra suggested 'logical_packages', and it seems to be the
> > best option we have as it covers all the cases above except the
> > last one. But it is only usable after smp_init() and all CPUs are
> > brought up, while 'tsc-early' is initialized before that.
> > 
> > One solution is to skip the watchdog for 'tsc-early' clocksource,
> > and move the check after smp_init(), while before 'tsc' clocksource
> > is registered, where 'logical_packages' could be used.
> > 
> > For 'nr_cpus' and 'maxcpus' cmdline case, the global flag
> > 'package_count_unreliable' will be set to true and the watchdog
> > will be kept as is.
> 
> So I have at least two machines where I boot with 'possible_cpus=#'
> because the BIOS MADT is reporting a stupid number of CPUs that
> aren't
> actually there.

Does the MADT report those CPUs as disabled but online capable?
can you send me a copy of the acpidmp?

I had a patch to parse MADT and count the number of physical packages
by decoding all the valid APICIDs in MADT.
I'm wondering if the patch still works on this machine.

> 
> So I think I'm lucky and side-stepped this nonsense, but if someone
> were
> to use nr_cpus= for this same purpose, they get screwed over and get
> the
> watchdog. Sad day for them I suppose.

what if using package_count_from_MADT?

thanks,
rui