Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 15 Jun 2023 11:20:21 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Feng Tang <feng.tang@intel.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H . Peter Anvin" <hpa@zytor.com>,
        David Woodhouse <dwmw@amazon.co.uk>,
        "Paul E . McKenney" <paulmck@kernel.org>, x86@kernel.org,
        linux-kernel@vger.kernel.org, rui.zhang@intel.com,
        tim.c.chen@intel.com
Subject: Re: [Patch v2 2/2] x86/tsc: use logical_packages as a better
 estimation of socket numbers
Message-ID: <20230615092021.GE1683497@hirez.programming.kicks-ass.net>
References: <20230613052523.1106821-1-feng.tang@intel.com>
 <20230613052523.1106821-2-feng.tang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230613052523.1106821-2-feng.tang@intel.com>
Precedence: bulk

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.

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.

I realize there might not be a perfect solution, but I'm also struggling
to see the value of the whole package_count_unreliable thing.