linux-kernel - Re: [PATCH 05/11] x86 topology: export die

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ceb7549e-6ccb-c6c7-241c-36c6e5c30770@inria.fr>
Date:   Wed, 20 Feb 2019 22:52:40 +0100
From:   Brice Goglin <Brice.Goglin@...ia.fr>
To:     Len Brown <lenb@...nel.org>, x86@...nel.org
Cc:     linux-kernel@...r.kernel.org, Len Brown <len.brown@...el.com>,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH 05/11] x86 topology: export die_siblings

Le 19/02/2019 à 04:40, Len Brown a écrit :
> From: Len Brown <len.brown@...el.com>
>
> like core_siblings, except it shows which die are in the same package.
>
> This is needed for lscpu(1) to correctly display die topology.
>
> Signed-off-by: Len Brown <len.brown@...el.com>
> Cc: linux-doc@...r.kernel.org
> Signed-off-by: Len Brown <len.brown@...el.com>
> ---
>  Documentation/cputopology.txt   | 10 ++++++++++
>  arch/x86/include/asm/smp.h      |  1 +
>  arch/x86/include/asm/topology.h |  1 +
>  arch/x86/kernel/smpboot.c       | 20 ++++++++++++++++++++
>  arch/x86/xen/smp_pv.c           |  1 +
>  drivers/base/topology.c         |  6 ++++++
>  include/linux/topology.h        |  3 +++
>  7 files changed, 42 insertions(+)
>
> diff --git a/Documentation/cputopology.txt b/Documentation/cputopology.txt
> index 287213b4517b..7dd2ae3df233 100644
> --- a/Documentation/cputopology.txt
> +++ b/Documentation/cputopology.txt
> @@ -56,6 +56,16 @@ core_siblings_list:
>  	human-readable list of cpuX's hardware threads within the same
>  	die_id.
>  
> +die_siblings:
> +
> +	internal kernel map of cpuX's hardware threads within the same
> +	physical_package_id.
> +
> +die_siblings_list:
> +
> +	human-readable list of cpuX's hardware threads within the same
> +	physical_package_id.
> +

Hello Len,

Patches #4 and #5 are changing the meaning the core_siblings (in the
past, it always returned all threads in the entire package). All
existing user-space tools will see each die as a separate package until
they are updated to read die_siblings too. It only matters for multi-die
CPUs when running a recent kernel with an old userspace tool, but it may
still be consider as a sysfs ABI change.

Worse, things will break again if you ever add tile_siblings for
CPUID.1f "Tiles". User-space will suddenly see 2 dies of 2 cores instead
1 die of 2 tiles of 2 cores.

I understand that this isn't easy to fix. But I want to make sure people
are aware of the meaning of this change.

The proper way to avoid this is to stop having file foo_siblings refer
to "the container of foo" instead of "foo itself" (because that
container changes when you add intermediate levels). Rename sysfs files
like below, and you don't get any breakage anymore when adding
intermediate levels:

thread_siblings -> core_threads (can we do sysfs alias or symlink to
keep the old name?)

core_siblings -> die_threads

die_siblings -> package_threads (needs an alias too)

The documentation would also be much easier to read since "die_threads"
is obviously "human-readable list of cpuX's hardware threads within the
same die_id". And no need to modify the doc anymore when adding levels :)

Brice