RE: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within a die
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Friday, March 19, 2021 11:02 PM > To: Jonathan Cameron > Cc: Song Bao Hua (Barry Song) ; > tim.c.c...@linux.intel.com; catalin.mari...@arm.com; w...@kernel.org; > r...@rjwysocki.net; vincent.guit...@linaro.org; b...@alien8.de; > t...@linutronix.de; mi...@redhat.com; l...@kernel.org; pet...@infradead.org; > dietmar.eggem...@arm.com; rost...@goodmis.org; bseg...@google.com; > mgor...@suse.de; msys.miz...@gmail.com; valentin.schnei...@arm.com; > juri.le...@redhat.com; mark.rutl...@arm.com; sudeep.ho...@arm.com; > aubrey...@linux.intel.com; linux-arm-ker...@lists.infradead.org; > linux-kernel@vger.kernel.org; linux-a...@vger.kernel.org; x...@kernel.org; > xuwei (O) ; Zengtao (B) ; > guodong...@linaro.org; yangyicong ; Liguozhu (Kenneth) > ; linux...@openeuler.org; h...@zytor.com > Subject: Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within > a die > > On Fri, Mar 19, 2021 at 09:36:16AM +, Jonathan Cameron wrote: > > On Fri, 19 Mar 2021 06:57:08 + > > "Song Bao Hua (Barry Song)" wrote: > > > > > > -Original Message- > > > > From: Greg KH [mailto:gre...@linuxfoundation.org] > > > > Sent: Friday, March 19, 2021 7:35 PM > > > > To: Song Bao Hua (Barry Song) > > > > Cc: tim.c.c...@linux.intel.com; catalin.mari...@arm.com; > w...@kernel.org; > > > > r...@rjwysocki.net; vincent.guit...@linaro.org; b...@alien8.de; > > > > t...@linutronix.de; mi...@redhat.com; l...@kernel.org; > pet...@infradead.org; > > > > dietmar.eggem...@arm.com; rost...@goodmis.org; bseg...@google.com; > > > > mgor...@suse.de; msys.miz...@gmail.com; valentin.schnei...@arm.com; > Jonathan > > > > Cameron ; juri.le...@redhat.com; > > > > mark.rutl...@arm.com; sudeep.ho...@arm.com; aubrey...@linux.intel.com; > > > > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > > > > linux-a...@vger.kernel.org; x...@kernel.org; xuwei (O) > ; > > > > Zengtao (B) ; guodong...@linaro.org; > yangyicong > > > > ; Liguozhu (Kenneth) ; > > > > linux...@openeuler.org; h...@zytor.com > > > > Subject: Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs > > > > within > > > > a die > > > > > > > > On Fri, Mar 19, 2021 at 05:16:15PM +1300, Barry Song wrote: > > > > > diff --git a/Documentation/admin-guide/cputopology.rst > > > > b/Documentation/admin-guide/cputopology.rst > > > > > index b90dafc..f9d3745 100644 > > > > > --- a/Documentation/admin-guide/cputopology.rst > > > > > +++ b/Documentation/admin-guide/cputopology.rst > > > > > @@ -24,6 +24,12 @@ core_id: > > > > > identifier (rather than the kernel's). The actual value is > > > > > architecture and platform dependent. > > > > > > > > > > +cluster_id: > > > > > + > > > > > + the Cluster ID of cpuX. Typically it is the hardware platform's > > > > > + identifier (rather than the kernel's). The actual value is > > > > > + architecture and platform dependent. > > > > > + > > > > > book_id: > > > > > > > > > > the book ID of cpuX. Typically it is the hardware platform's > > > > > @@ -56,6 +62,14 @@ package_cpus_list: > > > > > human-readable list of CPUs sharing the same > > > > > physical_package_id. > > > > > (deprecated name: "core_siblings_list") > > > > > > > > > > +cluster_cpus: > > > > > + > > > > > + internal kernel map of CPUs within the same cluster. > > > > > + > > > > > +cluster_cpus_list: > > > > > + > > > > > + human-readable list of CPUs within the same cluster. > > > > > + > > > > > die_cpus: > > > > > > > > > > internal kernel map of CPUs within the same die. > > > > > > > > Why are these sysfs files in this file, and not in a Documentation/ABI/ > > > > file which can be correctly parsed and shown to userspace? > > > > > > Well. Those ABIs have been there for much a long time. It is like: > > > > > > [root@ceph1 topology]# ls > > > core_id core_siblings core_siblings_list physical_package_id > thread_siblings thread_siblings_list > > > [r
Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within a die
On Fri, Mar 19, 2021 at 09:36:16AM +, Jonathan Cameron wrote: > On Fri, 19 Mar 2021 06:57:08 + > "Song Bao Hua (Barry Song)" wrote: > > > > -Original Message- > > > From: Greg KH [mailto:gre...@linuxfoundation.org] > > > Sent: Friday, March 19, 2021 7:35 PM > > > To: Song Bao Hua (Barry Song) > > > Cc: tim.c.c...@linux.intel.com; catalin.mari...@arm.com; w...@kernel.org; > > > r...@rjwysocki.net; vincent.guit...@linaro.org; b...@alien8.de; > > > t...@linutronix.de; mi...@redhat.com; l...@kernel.org; > > > pet...@infradead.org; > > > dietmar.eggem...@arm.com; rost...@goodmis.org; bseg...@google.com; > > > mgor...@suse.de; msys.miz...@gmail.com; valentin.schnei...@arm.com; > > > Jonathan > > > Cameron ; juri.le...@redhat.com; > > > mark.rutl...@arm.com; sudeep.ho...@arm.com; aubrey...@linux.intel.com; > > > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > > > linux-a...@vger.kernel.org; x...@kernel.org; xuwei (O) > > > ; > > > Zengtao (B) ; guodong...@linaro.org; yangyicong > > > ; Liguozhu (Kenneth) ; > > > linux...@openeuler.org; h...@zytor.com > > > Subject: Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs > > > within > > > a die > > > > > > On Fri, Mar 19, 2021 at 05:16:15PM +1300, Barry Song wrote: > > > > diff --git a/Documentation/admin-guide/cputopology.rst > > > b/Documentation/admin-guide/cputopology.rst > > > > index b90dafc..f9d3745 100644 > > > > --- a/Documentation/admin-guide/cputopology.rst > > > > +++ b/Documentation/admin-guide/cputopology.rst > > > > @@ -24,6 +24,12 @@ core_id: > > > > identifier (rather than the kernel's). The actual value is > > > > architecture and platform dependent. > > > > > > > > +cluster_id: > > > > + > > > > + the Cluster ID of cpuX. Typically it is the hardware platform's > > > > + identifier (rather than the kernel's). The actual value is > > > > + architecture and platform dependent. > > > > + > > > > book_id: > > > > > > > > the book ID of cpuX. Typically it is the hardware platform's > > > > @@ -56,6 +62,14 @@ package_cpus_list: > > > > human-readable list of CPUs sharing the same > > > > physical_package_id. > > > > (deprecated name: "core_siblings_list") > > > > > > > > +cluster_cpus: > > > > + > > > > + internal kernel map of CPUs within the same cluster. > > > > + > > > > +cluster_cpus_list: > > > > + > > > > + human-readable list of CPUs within the same cluster. > > > > + > > > > die_cpus: > > > > > > > > internal kernel map of CPUs within the same die. > > > > > > Why are these sysfs files in this file, and not in a Documentation/ABI/ > > > file which can be correctly parsed and shown to userspace? > > > > Well. Those ABIs have been there for much a long time. It is like: > > > > [root@ceph1 topology]# ls > > core_id core_siblings core_siblings_list physical_package_id > > thread_siblings thread_siblings_list > > [root@ceph1 topology]# pwd > > /sys/devices/system/cpu/cpu100/topology > > [root@ceph1 topology]# cat core_siblings_list > > 64-127 > > [root@ceph1 topology]# > > > > > > > > Any chance you can fix that up here as well? > > > > Yes. we will send a separate patch to address this, which won't > > be in this patchset. This patchset will base on that one. > > > > > > > > Also note that "list" is not something that goes in sysfs, sysfs is "one > > > value per file", and a list is not "one value". How do you prevent > > > overflowing the buffer of the sysfs file if you have a "list"? > > > > > > > At a glance, the list is using "-" rather than a real list > > [root@ceph1 topology]# cat core_siblings_list > > 64-127 > > > > Anyway, I will take a look if it has any chance to overflow. > > It could in theory be alternate CPUs as comma separated list. > So it's would get interesting around 500-1000 cpus (guessing). > > Hopefully no one has that crazy a cpu numbering scheme but it's possible > (note that cluster is fine for this, but I guess it might eventually > happen for core-siblings list (cpus within a package). > > Shouldn't crash or anything like that but might terminate early. We have a broken sysfs api already for listing LED numbers that has had to be worked around in the past, please do not create a new one with that same problem, we should learn from them :) thanks, greg k-h
Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within a die
On Fri, 19 Mar 2021 06:57:08 + "Song Bao Hua (Barry Song)" wrote: > > -Original Message- > > From: Greg KH [mailto:gre...@linuxfoundation.org] > > Sent: Friday, March 19, 2021 7:35 PM > > To: Song Bao Hua (Barry Song) > > Cc: tim.c.c...@linux.intel.com; catalin.mari...@arm.com; w...@kernel.org; > > r...@rjwysocki.net; vincent.guit...@linaro.org; b...@alien8.de; > > t...@linutronix.de; mi...@redhat.com; l...@kernel.org; pet...@infradead.org; > > dietmar.eggem...@arm.com; rost...@goodmis.org; bseg...@google.com; > > mgor...@suse.de; msys.miz...@gmail.com; valentin.schnei...@arm.com; Jonathan > > Cameron ; juri.le...@redhat.com; > > mark.rutl...@arm.com; sudeep.ho...@arm.com; aubrey...@linux.intel.com; > > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > > linux-a...@vger.kernel.org; x...@kernel.org; xuwei (O) ; > > Zengtao (B) ; guodong...@linaro.org; yangyicong > > ; Liguozhu (Kenneth) ; > > linux...@openeuler.org; h...@zytor.com > > Subject: Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within > > a die > > > > On Fri, Mar 19, 2021 at 05:16:15PM +1300, Barry Song wrote: > > > diff --git a/Documentation/admin-guide/cputopology.rst > > b/Documentation/admin-guide/cputopology.rst > > > index b90dafc..f9d3745 100644 > > > --- a/Documentation/admin-guide/cputopology.rst > > > +++ b/Documentation/admin-guide/cputopology.rst > > > @@ -24,6 +24,12 @@ core_id: > > > identifier (rather than the kernel's). The actual value is > > > architecture and platform dependent. > > > > > > +cluster_id: > > > + > > > + the Cluster ID of cpuX. Typically it is the hardware platform's > > > + identifier (rather than the kernel's). The actual value is > > > + architecture and platform dependent. > > > + > > > book_id: > > > > > > the book ID of cpuX. Typically it is the hardware platform's > > > @@ -56,6 +62,14 @@ package_cpus_list: > > > human-readable list of CPUs sharing the same physical_package_id. > > > (deprecated name: "core_siblings_list") > > > > > > +cluster_cpus: > > > + > > > + internal kernel map of CPUs within the same cluster. > > > + > > > +cluster_cpus_list: > > > + > > > + human-readable list of CPUs within the same cluster. > > > + > > > die_cpus: > > > > > > internal kernel map of CPUs within the same die. > > > > Why are these sysfs files in this file, and not in a Documentation/ABI/ > > file which can be correctly parsed and shown to userspace? > > Well. Those ABIs have been there for much a long time. It is like: > > [root@ceph1 topology]# ls > core_id core_siblings core_siblings_list physical_package_id > thread_siblings thread_siblings_list > [root@ceph1 topology]# pwd > /sys/devices/system/cpu/cpu100/topology > [root@ceph1 topology]# cat core_siblings_list > 64-127 > [root@ceph1 topology]# > > > > > Any chance you can fix that up here as well? > > Yes. we will send a separate patch to address this, which won't > be in this patchset. This patchset will base on that one. > > > > > Also note that "list" is not something that goes in sysfs, sysfs is "one > > value per file", and a list is not "one value". How do you prevent > > overflowing the buffer of the sysfs file if you have a "list"? > > > > At a glance, the list is using "-" rather than a real list > [root@ceph1 topology]# cat core_siblings_list > 64-127 > > Anyway, I will take a look if it has any chance to overflow. It could in theory be alternate CPUs as comma separated list. So it's would get interesting around 500-1000 cpus (guessing). Hopefully no one has that crazy a cpu numbering scheme but it's possible (note that cluster is fine for this, but I guess it might eventually happen for core-siblings list (cpus within a package). Shouldn't crash or anything like that but might terminate early. On sysfs file conversion, that got mentioned earlier but I forgot to remind Barry about it when he took this patch into his series. Sorry about that! Jonathan > > > thanks, > > > > greg k-h > > Thanks > Barry >
RE: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within a die
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Friday, March 19, 2021 7:35 PM > To: Song Bao Hua (Barry Song) > Cc: tim.c.c...@linux.intel.com; catalin.mari...@arm.com; w...@kernel.org; > r...@rjwysocki.net; vincent.guit...@linaro.org; b...@alien8.de; > t...@linutronix.de; mi...@redhat.com; l...@kernel.org; pet...@infradead.org; > dietmar.eggem...@arm.com; rost...@goodmis.org; bseg...@google.com; > mgor...@suse.de; msys.miz...@gmail.com; valentin.schnei...@arm.com; Jonathan > Cameron ; juri.le...@redhat.com; > mark.rutl...@arm.com; sudeep.ho...@arm.com; aubrey...@linux.intel.com; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > linux-a...@vger.kernel.org; x...@kernel.org; xuwei (O) ; > Zengtao (B) ; guodong...@linaro.org; yangyicong > ; Liguozhu (Kenneth) ; > linux...@openeuler.org; h...@zytor.com > Subject: Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within > a die > > On Fri, Mar 19, 2021 at 05:16:15PM +1300, Barry Song wrote: > > diff --git a/Documentation/admin-guide/cputopology.rst > b/Documentation/admin-guide/cputopology.rst > > index b90dafc..f9d3745 100644 > > --- a/Documentation/admin-guide/cputopology.rst > > +++ b/Documentation/admin-guide/cputopology.rst > > @@ -24,6 +24,12 @@ core_id: > > identifier (rather than the kernel's). The actual value is > > architecture and platform dependent. > > > > +cluster_id: > > + > > + the Cluster ID of cpuX. Typically it is the hardware platform's > > + identifier (rather than the kernel's). The actual value is > > + architecture and platform dependent. > > + > > book_id: > > > > the book ID of cpuX. Typically it is the hardware platform's > > @@ -56,6 +62,14 @@ package_cpus_list: > > human-readable list of CPUs sharing the same physical_package_id. > > (deprecated name: "core_siblings_list") > > > > +cluster_cpus: > > + > > + internal kernel map of CPUs within the same cluster. > > + > > +cluster_cpus_list: > > + > > + human-readable list of CPUs within the same cluster. > > + > > die_cpus: > > > > internal kernel map of CPUs within the same die. > > Why are these sysfs files in this file, and not in a Documentation/ABI/ > file which can be correctly parsed and shown to userspace? Well. Those ABIs have been there for much a long time. It is like: [root@ceph1 topology]# ls core_id core_siblings core_siblings_list physical_package_id thread_siblings thread_siblings_list [root@ceph1 topology]# pwd /sys/devices/system/cpu/cpu100/topology [root@ceph1 topology]# cat core_siblings_list 64-127 [root@ceph1 topology]# > > Any chance you can fix that up here as well? Yes. we will send a separate patch to address this, which won't be in this patchset. This patchset will base on that one. > > Also note that "list" is not something that goes in sysfs, sysfs is "one > value per file", and a list is not "one value". How do you prevent > overflowing the buffer of the sysfs file if you have a "list"? > At a glance, the list is using "-" rather than a real list [root@ceph1 topology]# cat core_siblings_list 64-127 Anyway, I will take a look if it has any chance to overflow. > thanks, > > greg k-h Thanks Barry
Re: [RFC PATCH v5 1/4] topology: Represent clusters of CPUs within a die
On Fri, Mar 19, 2021 at 05:16:15PM +1300, Barry Song wrote: > diff --git a/Documentation/admin-guide/cputopology.rst > b/Documentation/admin-guide/cputopology.rst > index b90dafc..f9d3745 100644 > --- a/Documentation/admin-guide/cputopology.rst > +++ b/Documentation/admin-guide/cputopology.rst > @@ -24,6 +24,12 @@ core_id: > identifier (rather than the kernel's). The actual value is > architecture and platform dependent. > > +cluster_id: > + > + the Cluster ID of cpuX. Typically it is the hardware platform's > + identifier (rather than the kernel's). The actual value is > + architecture and platform dependent. > + > book_id: > > the book ID of cpuX. Typically it is the hardware platform's > @@ -56,6 +62,14 @@ package_cpus_list: > human-readable list of CPUs sharing the same physical_package_id. > (deprecated name: "core_siblings_list") > > +cluster_cpus: > + > + internal kernel map of CPUs within the same cluster. > + > +cluster_cpus_list: > + > + human-readable list of CPUs within the same cluster. > + > die_cpus: > > internal kernel map of CPUs within the same die. Why are these sysfs files in this file, and not in a Documentation/ABI/ file which can be correctly parsed and shown to userspace? Any chance you can fix that up here as well? Also note that "list" is not something that goes in sysfs, sysfs is "one value per file", and a list is not "one value". How do you prevent overflowing the buffer of the sysfs file if you have a "list"? thanks, greg k-h
[RFC PATCH v5 1/4] topology: Represent clusters of CPUs within a die
From: Jonathan Cameron Both ACPI and DT provide the ability to describe additional layers of topology between that of individual cores and higher level constructs such as the level at which the last level cache is shared. In ACPI this can be represented in PPTT as a Processor Hierarchy Node Structure [1] that is the parent of the CPU cores and in turn has a parent Processor Hierarchy Nodes Structure representing a higher level of topology. For example Kunpeng 920 has 6 or 8 clusters in each NUMA node, and each cluster has 4 cpus. All clusters share L3 cache data, but each cluster has local L3 tag. On the other hand, each clusters will share some internal system bus. +---+ +-+ | +--++--++---+ | | | CPU0 || cpu1 | |+---+ | | | +--++--+ || | | | | ++L3 | | | | +--++--+ cluster ||tag| | | | | CPU2 || CPU3 | || | | | | +--++--+ |+---+ | | | | | | +---+ | | +---+ | | | +--++--+ +--+ | | | || | |+---+ | | | +--++--+ || | | | | ||L3 | | | | +--++--+ ++tag| | | | | || | || | | | | +--++--+ |+---+ | | | | | | +---+ | L3| | data | +---+ | | | +--++--+ |+---+ | | | | || | || | | | | +--++--+ ++L3 | | | | ||tag| | | | +--++--+ || | | | | | || |+++---+ | | | +--++--+|---+ | +---| | | +---| | | | +--++--++---+ | | | || | |+---+ | | | +--++--+ || | | | | ++L3 | | | | +--++--+ ||tag| | | | | || | || | | | | +--++--+ |+---+ | | | | | | +---+ | | +---+ | | | +--++--+ +--+ | | | || | | +---+ | | | +--++--+ | | | | | | | |L3 | | | | +--++--+ +---+tag| | | | | || | | | | | | | +--++--+ | +---+ | | | | | | +---+ | | +---+ ++ | | +--++--+ +--+ | | | || | | +---+ | | | +--++--+ | | | | | | | |L3 | | | | +--++--+ +--+tag| | | | | || | | | | | | | +--++--+ |
