linux-kernel - Re: [PATCH v1 1/2] x86/tsc: use logical_package as a better estimation of socket numbers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Oct 2022 15:37:18 +0800
From:   Feng Tang <feng.tang@...el.com>
To:     Dave Hansen <dave.hansen@...el.com>,
        Zhang Rui <rui.zhang@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Peter Zijlstra" <peterz@...radead.org>, <x86@...nel.org>
CC:     <linux-kernel@...r.kernel.org>, <tim.c.chen@...el.com>,
        Xiongfeng Wang <wangxiongfeng2@...wei.com>,
        <liaoyu15@...wei.com>
Subject: Re: [PATCH v1 1/2] x86/tsc: use logical_package as a better
 estimation of socket numbers

On Fri, Oct 21, 2022 at 09:21:02AM -0700, Dave Hansen wrote:
> On 10/21/22 08:00, Zhang Rui wrote:
> > I checked the history of '__max_logical_packages', and realized that
> > 
> > 1. for topology_max_packages()/'__max_logical_packages', the divisor
> >    'ncpus' uses cpu_data(0).booted_cores, which is based on the
> >    *online* CPUs. So when using kernel cmdlines like maxcpus=/nr_cpus=,
> >    '__max_logical_packages' can get over-estimated.
> > 
> > 2. for 'logical_packages', it equals the number of different physical
> >    Package IDs for all *online* CPUs. So with kernel cmdlines like
> >    nr_cpus=/maxcpus=, it can gets under-estimated.

Thanks for raising this new case! For 'maxcpus=' cmdline parameter, I
run this on 4-Sockets and 2-Sockets platform, and found the
'logical_packages' is still correct if the 'maxcpus >= (total_cpus/2 + 1)',
but gets wrong for smaller numbers.

SRAT parsing solution [1]. can solve 'maxcpus' case, but it will fail
for other user cases like sub-numa cluster or 'numa=off' case

IIUC, 'maxcpus' are likely for debug purpose, I think 'logical_packages'
is the better option for socket number estimation in the several
existing solutions.

> 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);

and adding a new 'tsc=watchdog' option to force watchdog on (might be
over-complexed?)

[1]. https://lore.kernel.org/lkml/Y0UgeUIJSFNR4mQB@feng-clx/

Thanks,
Feng