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:   Tue, 11 Oct 2022 15:51:21 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Dave Hansen <dave.hansen@intel.com>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>, <x86@kernel.org>,
        <linux-kernel@vger.kernel.org>, <rui.zhang@intel.com>,
        <tim.c.chen@intel.com>, Xiongfeng Wang <wangxiongfeng2@huawei.com>,
        Yu Liao <liaoyu15@huawei.com>
Subject: Re: [PATCH] x86/tsc: Extend the watchdog check exemption to 4S/8S
 machine
Message-ID: <Y0UgeUIJSFNR4mQB@feng-clx>
References: <20221009051209.393377-1-feng.tang@intel.com>
 <Y0LGLGW7RSlklKyl@hirez.programming.kicks-ass.net>
 <Y0N0ENurfliW315D@feng-clx>
 <aff10f33-b379-6872-f180-b38f6a0a669a@intel.com>
 <Y0TCOKc7n38341eJ@feng-clx>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Y0TCOKc7n38341eJ@feng-clx>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MS25G3HnUPNwcFr/8BRulK+Ewc+J7QptA1rBxUP7qrEPWxm7DjZUGi6e+gAB?=
 =?us-ascii?Q?G/S9dhTpkiEbANv+XeDi3Nq45ZiqLObvO8cMMNqSm/OkL6pzJDEZQKYCHQs4?=
 =?us-ascii?Q?Dm95mNk57u7Lfc70OUturdF1pBjVk7FamHjigCyjjJR4V7raQPdnIQL00rSL?=
 =?us-ascii?Q?475iAFLtfMhZAyg7v31G/gWfK5mWl2ii8FT0wW5Jms2K+fKoxFCB8d3BCp9X?=
 =?us-ascii?Q?WQ5R+qfmqx7B7fX/EQZsBEV07X0Zy7oPcL/SHbO5jIo9Vf3uW//tjyVLB+9S?=
 =?us-ascii?Q?UO5rfLg1Z2QkrnTR7y/mnz6jpt66UuUQ+wSLhCRSvOMqkELkG7VtkIA6q32q?=
 =?us-ascii?Q?fXgCHzJR4JTiyggkMktgwvB7ee8VjarXd9gH7NImrMFsDjYB55NRl/q+4sBD?=
 =?us-ascii?Q?A/8prim8oB8q1xtT+cnPsP6bgmXAfsylswsldB9PVnnmcxMGodGA5t3PU6LX?=
 =?us-ascii?Q?j+/TaurYfAEYCqBLolxpmX9g/EN2PZ/QCyn3OYNAukrPqvvj4lr/BvftLrNC?=
 =?us-ascii?Q?qL+SLNoQLOcBu/TrruhlWF4xoTx8727nPDASw4t+KSglJCR1+4C8i7Bmjg2z?=
 =?us-ascii?Q?3LTSGPVoGipfE4/1mLfTeh2jeFn0X1doSPdPo/lAq0ZDc+MT/mBkFSE66UIh?=
 =?us-ascii?Q?BiemqupZ+j+PDgoUzkSOWjpikK+K92+QdLWfpHr9HUMc2oq1hk3861HBoZpk?=
 =?us-ascii?Q?6nSI/sDn5EZCtEefUI8tgOyw//lGL5ehH4vqj2FgWyWvx9xSgVKdhx2IeBRy?=
 =?us-ascii?Q?OePAfIN76P0pdj8rE2lCJ8Y0TFU9pw2vuIoJyYuZR80co8CUvTYgREdW6Gcu?=
 =?us-ascii?Q?+Z2qhIg0D0rwqs1Pj69B0Y6Lt5ZqxUwXXQSkZY4vz/zIXAJ2HMV7CQpcoJpc?=
 =?us-ascii?Q?O/q5D1eOqWWQLfoE2gbLy1JG6K3AUhYjNcNZZJItb/8xqqWTozIt1SPuxmmr?=
 =?us-ascii?Q?BVm5VyLXQfWPnrip1yqJ+Go1qxcKqq5/uKBgGHMDdkrY2LKCnDG3CPCLr3Jg?=
 =?us-ascii?Q?wOFef0hGaFymbRzUsplQVLzeWnA5OVNWD7ZVtlA5UsBGL49qxJP3381PLv+2?=
 =?us-ascii?Q?ukZCcak3UOs5D2AsA0CPd0nTYOwwbrpXyrd3X+NoKVq1k2SENMKE3IUWF9zO?=
 =?us-ascii?Q?g99AuxPKvcEs8CODu3FOWxGRVjrDZxCVQeQh9474/WgsiTQf5Ks2EPoMegJm?=
 =?us-ascii?Q?3DeH6iffcQQ5EoGsXFOGBFYcn5xuclh9P/qbC88v5T4PeuwmWNrupN4P2hUh?=
 =?us-ascii?Q?wBHlRTwNunXaTD2iNJBXVYsnlGn4PER8wFjYfMEA8lyPCZ/elrjvI+j1PbTX?=
 =?us-ascii?Q?MzRpiqeAC4w0V0NvMm7N0Vs2/YE984Ki/ZaaN+8tKEPZolM6sy3GdezZQYvz?=
 =?us-ascii?Q?b/P3NSe8JAVHX9nqOf/I+3WtZxACfog8z3VH52WG1ymj4KfD9bIwKbNlQtKi?=
 =?us-ascii?Q?LUg8GsqEzHAV55QMNyrZ2/x76Cnwtq6k/l/3AKTTiRtdNi21q+RsPAxOu7Tm?=
 =?us-ascii?Q?h8XaeGBeEpSYCGIbi0dg3qg5BtdeeMu6AAotlhmx91egSEuF0fXcKXUoeDsn?=
 =?us-ascii?Q?XvV+08v41SzoJN7nLhA3tJUPNawum73/JULA18ot?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 70fd3f19-53a7-4790-e340-08daab5d6f90
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Oct 2022 07:51:42.2482
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 1Y0t6H3bnQl97OkbgXyX9ssWEjA9easNT9T5nB27/Z+QChinP81qdWCD0i2YfNajbHpuANpWsZfeYtlmsrjFwg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4542
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Oct 11, 2022 at 09:09:12AM +0800, Feng Tang wrote:
> On Mon, Oct 10, 2022 at 07:23:10AM -0700, Dave Hansen wrote:
> > On 10/9/22 18:23, Feng Tang wrote:
> > >>> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > >>> index cafacb2e58cc..b4ea79cb1d1a 100644
> > >>> --- a/arch/x86/kernel/tsc.c
> > >>> +++ b/arch/x86/kernel/tsc.c
> > >>> @@ -1217,7 +1217,7 @@ static void __init check_system_tsc_reliable(void)
> > >>>  	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> > >>>  	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
> > >>>  	    boot_cpu_has(X86_FEATURE_TSC_ADJUST) &&
> > >>> -	    nr_online_nodes <= 2)
> > >>> +	    nr_online_nodes <= 8)
> > >> So you're saying all 8 socket systems since Broadwell (?) are TSC
> > >> sync'ed ?
> > > No, I didn't mean that. I haven't got chance to any 8 sockets
> > > machine, and I got a report last month that on one 8S machine,
> > > the TSC was judged 'unstable' by HPET as watchdog.
> > 
> > That's not a great check.  Think about numa=fake=4U, for instance.  Or a
> > single-socket system with persistent memory and high bandwidth memory.
> > 
> > Basically 'nr_online_nodes' is a software construct.  It's going to be
> > really hard to infer anything from it about what the _hardware_ is.
> 
> You are right! How to get the socket number was indeed a trouble when
> I worked on commit b50db7095fe0, the problem is related to the
> initialization order. This tsc check needs to be done in tsc_init(),
> while the node_stats[] get initialized in later's call of smp_init().
> 
> For the case you mentioned above, I dug out some old logs which showed
> its init order:
> 
>   numa=fake=4 on a SKL desktop
>   ================
>   [    0.000066] [tsc_early_init()]: nr_online_nodes = 1
>   [    0.000068] [tsc_early_init()]: nr_cpu_nodes = 0
>   [    0.000070] [tsc_early_init()]: nr_mem_nodes = 0
>   [    0.104015] [tsc_init()]: nr_online_nodes = 4
>   [    0.104019] [tsc_init()]: nr_cpu_nodes = 0
>   [    0.104022] [tsc_init()]: nr_mem_nodes = 4
>   [    0.124778] smp: Brought up 4 nodes, 4 CPUs
>   [    0.760915] [init_tsc_clocksource()]: nr_online_nodes = 4
>   [    0.760919] [init_tsc_clocksource()]: nr_cpu_nodes = 4
>   [    0.760922] [init_tsc_clocksource()]: nr_mem_nodes = 4
>   
>   QEMU with 2 CPU-DRAM nodes + 2 Persistent memory nodes 
>   ========================================================
>   [    0.066651] [tsc_early_init()]: nr_online_nodes = 1
>   [    0.067494] [tsc_early_init()]: nr_cpu_nodes = 0
>   [    0.068288] [tsc_early_init()]: nr_mem_nodes = 0
>   [    0.677694] [tsc_init()]: nr_online_nodes = 4
>   [    0.678862] [tsc_init()]: nr_cpu_nodes = 0
>   [    0.679962] [tsc_init()]: nr_mem_nodes = 4
>   [    1.139240] [init_tsc_clocksource()]: nr_online_nodes = 4
>   [    1.140576] [init_tsc_clocksource()]: nr_cpu_nodes = 2
>   [    1.141823] [init_tsc_clocksource()]: nr_mem_nodes = 4
>   [    1.660100] [kernel_init()]: nr_online_nodes = 4
>   [    1.661234] [kernel_init()]: nr_cpu_nodes = 2
>   [    1.662300] [kernel_init()]: nr_mem_nodes = 4
> 
> The 'nr_online_nodes' was chosed in the hope of that, in worse case
> the patch is just a nop and won't wrongly lift the check.
> 
> One possible solution for this problem is to leverage the SRAT table
> early init which is called before tsc_init(), and can provide CPU
> nodes info. Will try this way.

Th simple patch below is to have a dedicate CPU nodemask and set it in
early SRAT CPU parsing, still it has problem when sub-numa is enabled
in BIOS where there are more NUMA nodes in SRAT table. (also I'm
not sure the change to amdtopology.c is right)

Thanks,
Feng

diff --git a/arch/x86/include/asm/numa.h b/arch/x86/include/asm/numa.h
index e3bae2b60a0d..e745053a5f9a 100644
--- a/arch/x86/include/asm/numa.h
+++ b/arch/x86/include/asm/numa.h
@@ -31,6 +31,7 @@ extern int numa_off;
  */
 extern s16 __apicid_to_node[MAX_LOCAL_APIC];
 extern nodemask_t numa_nodes_parsed __initdata;
+extern nodemask_t numa_cpu_nodes __initdata;
 
 extern int __init numa_add_memblk(int nodeid, u64 start, u64 end);
 extern void __init numa_set_distance(int from, int to, int distance);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 179e0b1ba5cc..a2a7fc5aa15c 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -29,6 +29,7 @@
 #include <asm/intel-family.h>
 #include <asm/i8259.h>
 #include <asm/uv/uv.h>
+#include <asm/numa.h>
 
 unsigned int __read_mostly cpu_khz;	/* TSC clocks / usec, not used here */
 EXPORT_SYMBOL(cpu_khz);
@@ -1218,7 +1219,7 @@ first_dump();
 	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
 	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
 	    boot_cpu_has(X86_FEATURE_TSC_ADJUST) &&
-	    nr_online_nodes <= 2)
+	    nodes_weight(numa_cpu_nodes) <= 2)
 		tsc_disable_clocksource_watchdog();
 }
 
diff --git a/arch/x86/mm/amdtopology.c b/arch/x86/mm/amdtopology.c
index b3ca7d23e4b0..6b982a16cc38 100644
--- a/arch/x86/mm/amdtopology.c
+++ b/arch/x86/mm/amdtopology.c
@@ -152,6 +152,7 @@ int __init amd_numa_init(void)
 		prevbase = base;
 		numa_add_memblk(nodeid, base, limit);
 		node_set(nodeid, numa_nodes_parsed);
+		node_set(nodeid, numa_cpu_nodes);
 	}
 
 	if (nodes_empty(numa_nodes_parsed))
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 090125b3ee1f..82798fee97a2 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -21,6 +21,7 @@
 
 int numa_off;
 nodemask_t numa_nodes_parsed __initdata;
+nodemask_t numa_cpu_nodes __initdata;
 
 struct pglist_data *node_data[MAX_NUMNODES] __read_mostly;
 EXPORT_SYMBOL(node_data);
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 7688117ac2f4..11b08b317306 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -59,6 +59,7 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 	}
 	set_apicid_to_node(apic_id, node);
 	node_set(node, numa_nodes_parsed);
+	node_set(node, numa_cpu_nodes);
 
 	printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u\n",
@@ -106,6 +107,7 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
 
 	set_apicid_to_node(apic_id, node);
 	node_set(node, numa_nodes_parsed);
+	node_set(node, numa_cpu_nodes);
 
 	printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%02x -> Node %u\n",