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, 25 Oct 2022 15:57:12 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Dave Hansen <dave.hansen@intel.com>,
        Zhang Rui <rui.zhang@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        "Peter Zijlstra" <peterz@infradead.org>, <x86@kernel.org>
CC:     <linux-kernel@vger.kernel.org>, <tim.c.chen@intel.com>,
        Xiongfeng Wang <wangxiongfeng2@huawei.com>,
        <liaoyu15@huawei.com>, <oliver.sang@intel.com>,
        <ying.huang@intel.com>
Subject: Re: [PATCH v1 1/2] x86/tsc: use logical_package as a better
 estimation of socket numbers
Message-ID: <Y1eW2L5y7jx69v0g@feng-clx>
References: <20221021062131.1826810-1-feng.tang@intel.com>
 <f27e4b3f858890c657df9a7d6f34dc2d60b89757.camel@intel.com>
 <63dca468-c94d-844a-5b19-09c03cf84911@intel.com>
 <Y1ZArguS/rVEjXOb@feng-clx>
 <397f513f-9273-76d1-a0ba-9d1d403020c5@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <397f513f-9273-76d1-a0ba-9d1d403020c5@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?+e5+EkxFdrHvktFMHjw2CcLFrhT5QdpC39f9dFog04BnbCbgPsxwKucOeYsO?=
 =?us-ascii?Q?8nDSqG9Vf9nMes6uxcaMxSOCH7Y5Hnu42F8fcMbGAd3gc/miEm2XrSzYFkIZ?=
 =?us-ascii?Q?/Z/ftcebaK0AJSFPn3uoVYT+BfZ860b5ohW8gEi4wVGLMEeJCkfMQC5fLMjb?=
 =?us-ascii?Q?ms85jVYAAHM1KiSNZy5RjM0UdbXYU5ICtvxqRbENZESiYdDlnyXgPkl8qnGV?=
 =?us-ascii?Q?xQ+IDHXJU1eQt6vGq7rmZ1NS5KfaLRfHCOIGHG/EGhSQZW5zX6ay5OTbuXDa?=
 =?us-ascii?Q?k6gPvHLEATyyk90CIy76S3FR4aKRLbTCX4OFe2o/JSWOzH2IhEYjgZhCAYwu?=
 =?us-ascii?Q?2tvaAWF6BzBxD+2rFBax1lJfQgXmARI14ni6ZA0FLQM/N2tk9mnomn2C0kNS?=
 =?us-ascii?Q?ZphLr+AeTRsYpYAY2CovwX4n/haJPm9R/jLw+ssmTtTsXuWYxf2aI1mxXSGm?=
 =?us-ascii?Q?fX3EbvESM0o/g81RkqYhIKEh7lmuad0BQAqE5xr82D0K4eoh8qhU8aHj7XJX?=
 =?us-ascii?Q?MF/oZJ5ukmobm0kjFIx8oUl2hlAQNbvUnhJm2IC8DXddb3y3oXqQsk4oxc1s?=
 =?us-ascii?Q?E0DymMcCPOdv4hrFWJvNYm97K/qhzTsJ/a9SHx0miaoAbrGevoQzbiTCneo9?=
 =?us-ascii?Q?9uVnsvQn8i/AfLt9L/982Rem3zeOHI94NFx6uIG3dtCoDbGiQzGawUobdmnJ?=
 =?us-ascii?Q?pJBxeoXoLJoa1mEdhg+Mh5RD+Uh69+ezMHT6F9b2SmIvOCLUU4KZTZQ/pA+6?=
 =?us-ascii?Q?n5/Xy3RjHSOzSHlfB/+w1HB+VklEIcABqTKi6lSz7kx/ib8JSAnPzsAXsPZC?=
 =?us-ascii?Q?+/xjpB33w1e1WTz3TBR/JzijfMEYyDSdxtE4ePpkuzU7TMdF+/O7BCR8HGjm?=
 =?us-ascii?Q?mE339uSUMezWpAa8C9fqn9L4FXeSknyjgsVW2OccW2O6UcqtSM2h6Xx7IeVK?=
 =?us-ascii?Q?E3vabBSvsdR4UzAgRYP/rfba/Es+XOfqS1f0ifxkUMS1lLwBtkD+jM492AsA?=
 =?us-ascii?Q?g8ERTECKJvONaApE4Xy8jWvj4wO0Dt7rfAG4YccPyfa8NSfTPcb7gZRyU1Fa?=
 =?us-ascii?Q?Wiutere+asXyT+r3yGwPO+MdzaUxdzWPmHBJB4Fnr/O4fowsA6JdDGyzRoEZ?=
 =?us-ascii?Q?8dc7R6oUHkXMgfxbN466+c+kTt5AxEtDDjeqk84kHzkMB1j9brD6A9rWdOw0?=
 =?us-ascii?Q?00hHZZcxcUdm38ZRCyRbsEbXG28WvaThXgK23GFG9bQZX2MWz6/f/I3bBE/5?=
 =?us-ascii?Q?GpLOxbIsz9vyykFXx7ciQabcsMmbWrOcqVrJttBhQTxBcjeo7EExAhCg5uvF?=
 =?us-ascii?Q?f9O8SNSZwKIuNbbWFEd0OKoSGt+93QncghkMUZPHHZMydfpjcibhBUn92tdW?=
 =?us-ascii?Q?yTzl58KqIrU1KEOM2KxfGcRnKfNI77oQcwZ04vqjUR2F22Fz1RCdsCnvXm1V?=
 =?us-ascii?Q?6S15KMH6rZLmEuZH6EVb6XalbbzSthqrAyRaJy3lljJ1MzoZ5fPHVmqFRwPy?=
 =?us-ascii?Q?m6bHFoPNPWk/Egoj7NlpuYZD5TQ3gPtdI8dUKOZwCpv89P3oyTXXBJIlJqMO?=
 =?us-ascii?Q?uzuUQANBWjL3A59Kur08YreB3YY89xgAMPIeJUCz?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 03c66762-edff-41e2-7960-08dab65e8d22
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2022 07:57:23.8623
 (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: G1MD1zQcDPF6QPgdmEoUf5OHeNvzOEeuUY3lTwFwW1o6Q4WTWtdwp7xbOIfz/mIugr7IXDxtsrAlDa89E1kIHg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4873
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        URIBL_BLOCKED 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 Mon, Oct 24, 2022 at 08:43:33AM -0700, Dave Hansen wrote:
> On 10/24/22 00:37, Feng Tang wrote:
> >> For instance, I can live with the implementation being a bit goofy when
> >> kernel commandlines are in play.  We can pr_info() about those cases.
> > Something like adding
> > 
> > pr_info("Watchdog for TSC is disabled for this platform while estimating
> > 	the socket number is %d, if the real socket number is bigger than
> > 	4 (may due to some tricks like 'maxcpus=' cmdline parameter, please
> > 	add 'tsc=watchdog' to cmdline as well\n", logical_packages);
> 
> That's too wishy-washy.  Also, I *KNOW* Intel has built systems with
> wonky, opaque numbers of "sockets".  Cascade Lake was a single physical
> "socket", but in all other respects (including enumeration to software)
> it acted like two logical sockets.
> 
> So, what was the "real" socket number for Cascade Lake?  If you looked
> in a chassis, you'd see one socket.  But, there were two dies in that
> socket talking to each other over UPI, so it had a system topology which
> was indistinguishable from a 2-socket system.
 
Good to know and thanks for the info.

I have to admit I haven't checked a server's internals before, and
thanks to Oliver for helping checking some Cascade Lake boxes of 0Day.

In one box where 'lscpu' shows 4 sockets (96 cores in total), it does
only have 2 physical processors in the chassis, just like you
mentioned, it has 2 dies for each processor. And in another box,
'lscpu' shows 2 sockets (44 cores in total), it also has 2 physical
processors but with much smaller size.

And fortunately the 'logical_packages' for these 2 boxes are both
the correct value: 2.

> Let's just state the facts:
> 
> 	pr_info("Disabling TSC watchdog on %d-package system.", ...)
> 
> Then, we can have a flag elsewhere to say how reliable that number is.
> A taint flag or CPU bug is probably going to far, but something like this:
> 
> bool logical_package_count_unreliable = false;
> 
> void mark_bad_package_count(char *reason)
> {
> 	if (logical_package_count_unreliable)
> 		return true;
> 
> 	pr_warn("processor package count is unreliable");
> }
> 
> Might be OK.  Then you can call mark_bad_package_count() from multiple
> sites, like the maxcpus= code.

This should work! we can just add one more check:

	boot_cpu_has(X86_FEATURE_TSC_ADJUST) &&
	!logical_package_count_unreliable &&
	logical_packages <= 2 

And when some new case leading to a imprecise 'logical_packages' is
found in future, we could just apply to this to it.

> But, like I said in the other thread, let's make sure we're agreed on
> the precise problem that we're solving before we go down this road.

Sure.

Thanks,
Feng


> > and adding a new 'tsc=watchdog' option to force watchdog on (might be
> > over-complexed?)
> 
> Agreed, I don't think that's quite warranted yet.