Re: [PATCH 05/11] x86 topology: export die_siblings

["Liang, Kan" <kan.liang@linux.intel.com>]

On 2/19/2019 1:43 PM, Brown, Len wrote:
> Thanks for the comments, Kan,
> 
>>> 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.
>>> +
>>>    book_siblings:
>>>    
>>>    	internal kernel map of cpuX's hardware threads within the same diff
> 
>> Could you please update the document regarding to topology_die_cpumask and topology_core_cpumask in Documentation/x86/topology.txt
>   
> I agree that the top part of this file, as updated above, should document the external sysfs interface...
> 
> I'm less excited about the center of the file trying to document the internal implementation -- as the source code
> is actually more clear than the document, but here is an update, consistent with the existing file.
> Let me know if it does not fully address your comment.
>

Besides the generic document, I think we should update x86 document as 
well, which is in Documentation/x86/topology.txt.

The definition of topology_core_cpumask has to be changed to per die, right?

diff --git a/Documentation/x86/topology.txt b/Documentation/x86/topology.txt
index 8107b6c..8698a41 100644
--- a/Documentation/x86/topology.txt
+++ b/Documentation/x86/topology.txt
@@ -105,11 +105,16 @@ The topology of a system is described in the units of:

    Thread-related topology information in the kernel:

-  - topology_core_cpumask():
+  - topology_die_cpumask():

      The cpumask contains all online threads in the package to which a 
thread
      belongs.

+  - topology_core_cpumask():
+
+    The cpumask contains all online threads in the die to which a thread
+    belongs.
+
      The number of online threads is also printed in /proc/cpuinfo 
"siblings."

    - topology_sibling_cpumask():


Thanks,
Kan

> Thanks,
> -Len
> ---
> 
> diff --git a/Documentation/cputopology.txt b/Documentation/cputopology.txt
> index 7dd2ae3..f39c759 100644
> --- a/Documentation/cputopology.txt
> +++ b/Documentation/cputopology.txt
> @@ -102,6 +102,7 @@ these macros in include/asm-XXX/topology.h::
>          #define topology_drawer_id(cpu)
>          #define topology_sibling_cpumask(cpu)
>          #define topology_core_cpumask(cpu)
> +       #define topology_die_cpumask(cpu)
>          #define topology_book_cpumask(cpu)
>          #define topology_drawer_cpumask(cpu)
> 
> @@ -114,10 +115,11 @@ To be consistent on all architectures, include/linux/topology.h
>   provides default definitions for any of the above macros that are
>   not defined by include/asm-XXX/topology.h:
> 
> -1) physical_package_id: -1
> -2) core_id: 0
> -3) sibling_cpumask: just the given CPU
> -4) core_cpumask: just the given CPU
> +1) topology_physical_package_id: -1
> +2) topology_core_id: 0
> +3) topology_sibling_cpumask: just the given CPU
> +4) topology_core_cpumask: just the given CPU
> +5) topology_die_cpumask: just the given CPU
> 
>   For architectures that don't support books (CONFIG_SCHED_BOOK) there are no
>   default definitions for topology_book_id() and topology_book_cpumask().
> 
>