usertools: enhance CPU layout

  The cores in a single CPU may be not all
the same.
The user tool is updated to show the
difference of the cores.

This patch addes below informantion,
1, Group the cores based on the die.
2, A core is either a performance core or an
   efficency core.
   A performance core is shown as 'Core-P'.
   An efficency core is shown as 'Core-E'.
3, All the E-cores which share the same L2-cache
   are grouped to one module.

The known limitation.
1, To tell a core is P-core or E-core is based on if
   this core shares L2 cache with others.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 usertools/cpu_layout.py | 77 +++++++++++++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 14 deletions(-)
  

On Tue, 18 Apr 2023 13:25:41 +0800
Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:

>      fd = open("{}/cpu{}/topology/physical_package_id".format(base_path, cpu))
>      socket = int(fd.read())
>      fd.close()
> +    fd = open("{}/cpu{}/topology/die_id".format(base_path, cpu))
> +    die = int(fd.read())
> +    fd.close()
> +    fd = open("{}/cpu{}/topology/thread_siblings_list".format(base_path, cpu))
> +    threads_share = str(fd.read())
> +    fd.close()
> +    fd = open("{}/cpu{}/cache/index2/shared_cpu_list".format(base_path, cpu))
> +    l2_cache_share = str(fd.read())
> +    fd.close()

This code would read easier and follow more pythonish convention by
using "with" and us f-strings.

Are all these API's available on oldest supported kernel version
for DPDK? 4.14

And there is a lot of duplicated code which indicates that
a helper function would help.

Not sure that the naming with -P and -E are generic enough,
looks like something Intel specific??

Also, your code causes more python lint warnings about things like:
usertools/cpu_layout.py:49:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)


Something like this?

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa19..3423a735ccf6 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -1,38 +1,82 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: BSD-3-Clause
-# Copyright(c) 2010-2014 Intel Corporation
+# Copyright(c) 2010-2023 Intel Corporation
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
 sockets = []
+dies = []
 cores = []
+module_id = []
 core_map = {}
+core_p_e = {}
+title_len = 47
+die_len = 8
+module_no = 0
+meaningful_module = []
 base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
+
+
+def sysfs_cpu(attribute):
+    '''read string from sysfs cpu info'''
+    with open(f"{base_path}/{attribute}") as sysfs_fd:
+        return sysfs_fd.read()
+
+
+max_cpus = int(sysfs_cpu("kernel_max"))
+
 for cpu in range(max_cpus + 1):
+    topology = f"cpu{cpu}/topology"
     try:
-        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
+        fd = open(f"{base_path}/{topology}/core_id")
+        core = int(fd.read())
+        fd.close()
     except IOError:
         continue
-    core = int(fd.read())
-    fd.close()
-    fd = open("{}/cpu{}/topology/physical_package_id".format(base_path, cpu))
-    socket = int(fd.read())
-    fd.close()
+
     if core not in cores:
         cores.append(core)
+    socket = int(sysfs_cpu(f"{topology}/physical_package_id"))
     if socket not in sockets:
         sockets.append(socket)
-    key = (socket, core)
+    die = int(sysfs_cpu(f"{topology}/die_id"))
+    if die not in dies:
+        dies.append(die)
+    key = (socket, die, core)
     if key not in core_map:
         core_map[key] = []
+
+    threads_share = sysfs_cpu(f"{topology}/thread_siblings_list")
+    l2_cache_share = sysfs_cpu(f"cpu{cpu}/cache/index2/shared_cpu_list")
+    if threads_share == l2_cache_share:
+        p_e = '-P'
+        module_id.append(-1)
+    else:
+        module_tmp = []
+        p_e = '-E'
+        for i in l2_cache_share:
+            if not i.isdigit():
+                break
+            module_tmp.append(i)
+        if cpu == int("".join(module_tmp)):
+            module_id.append(module_no)
+            module_no += 1
+        else:
+            module_id.append(-1)
+    key_p_e = (die, core)
+    if key_p_e not in core_p_e:
+        core_p_e[key_p_e] = p_e
     core_map[key].append(cpu)
 
-print(format("=" * (47 + len(base_path))))
+print(format("=" * (title_len + len(base_path))))
 print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path))))
+print("{}\n".format("=" * (title_len + len(base_path))))
 print("cores = ", cores)
+
+for i in module_id:
+    if i != -1:
+        meaningful_module.append(i)
+print("modules = ", meaningful_module)
+print("dies = ", dies)
 print("sockets = ", sockets)
 print("")
 
@@ -43,22 +87,30 @@
                       + len('[]') + len('Socket ')
 max_core_id_len = len(str(max(cores)))
 
-output = " ".ljust(max_core_id_len + len('Core '))
+socket_space_len = max_core_id_len + len('Core ') + die_len + len('-P')
+output = " ".ljust(socket_space_len)
 for s in sockets:
     output += " Socket %s" % str(s).ljust(max_core_map_len - len('Socket '))
 print(output)
 
-output = " ".ljust(max_core_id_len + len('Core '))
+output = " ".ljust(socket_space_len)
 for s in sockets:
     output += " --------".ljust(max_core_map_len)
     output += " "
 print(output)
 
-for c in cores:
-    output = "Core %s" % str(c).ljust(max_core_id_len)
-    for s in sockets:
-        if (s, c) in core_map:
-            output += " " + str(core_map[(s, c)]).ljust(max_core_map_len)
-        else:
-            output += " " * (max_core_map_len + 1)
-    print(output)
+for d in dies:
+    print("Die", die)
+    for c in cores:
+        if module_id[core_map[(sockets[0], d, c)][0]] != -1:
+            print("    Module", module_id[core_map[(sockets[0], d, c)][0]])
+        output = " ".ljust(die_len)
+        output += "Core"
+        output += core_p_e[(d, c)]
+        output += " %s" % str(c).ljust(max_core_id_len)
+        for s in sockets:
+            if (s, d, c) in core_map:
+                output += " " + str(core_map[(s, d, c)]).ljust(max_core_map_len)
+            else:
+                output += " " * (max_core_map_len + 1)
+        print(output)
  

On Tue, 18 Apr 2023 13:25:41 +0800
Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:

> The cores in a single CPU may be not all
> the same.
> The user tool is updated to show the
> difference of the cores.
> 
> This patch addes below informantion,
> 1, Group the cores based on the die.
> 2, A core is either a performance core or an
>    efficency core.
>    A performance core is shown as 'Core-P'.
>    An efficency core is shown as 'Core-E'.
> 3, All the E-cores which share the same L2-cache
>    are grouped to one module.
> 
> The known limitation.
> 1, To tell a core is P-core or E-core is based on if
>    this core shares L2 cache with others.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Side note:
This tool only exists because of lack of simple tool at the time.
Looking around, found that there is a tool 'lstopo' under the hwloc
package that gives output in many formats including graphical and
seems to do a better job than the DPDK python script.

Not sure how much farther DPDK should go in this area?
Really should be a distro tool.
  

Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, April 19, 2023 12:47 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] usertools: enhance CPU layout
> 
> On Tue, 18 Apr 2023 13:25:41 +0800
> Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:
> 
> > The cores in a single CPU may be not all the same.
> > The user tool is updated to show the
> > difference of the cores.
> >
> > This patch addes below informantion,
> > 1, Group the cores based on the die.
> > 2, A core is either a performance core or an
> >    efficency core.
> >    A performance core is shown as 'Core-P'.
> >    An efficency core is shown as 'Core-E'.
> > 3, All the E-cores which share the same L2-cache
> >    are grouped to one module.
> >
> > The known limitation.
> > 1, To tell a core is P-core or E-core is based on if
> >    this core shares L2 cache with others.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
> Side note:
> This tool only exists because of lack of simple tool at the time.
> Looking around, found that there is a tool 'lstopo' under the hwloc package
> that gives output in many formats including graphical and seems to do a better
> job than the DPDK python script.
> 
> Not sure how much farther DPDK should go in this area?
> Really should be a distro tool.
Many thanks for your review and comments.
Have to say I'm a green hand in this field. Just imitate the existing code to write mine. So, still trying to understand and handle the comments :)

Better to understand more about our opinion of this script before send a v2 patch.
I've used 'lstopo'. It's a great tool.
To my opinion, considering there're Linux tools to show all kinds of information, the reason that DPDK has its own tool is to summarize and emphasize the information that is important to DPDK. Here it's that some cores are more powerful than others. When the users use a testpmd-like APP, they can choose the appropriate cores after DPDK reminds them about the difference between cores.
Add Thomas for more suggestions. Thanks.
  

21/04/2023 03:47, Lu, Wenzhuo:
> Hi Stephen,
> 
> From: Stephen Hemminger <stephen@networkplumber.org>
> > On Tue, 18 Apr 2023 13:25:41 +0800
> > Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:
> > 
> > > The cores in a single CPU may be not all the same.
> > > The user tool is updated to show the
> > > difference of the cores.
> > >
> > > This patch addes below informantion,
> > > 1, Group the cores based on the die.
> > > 2, A core is either a performance core or an
> > >    efficency core.
> > >    A performance core is shown as 'Core-P'.
> > >    An efficency core is shown as 'Core-E'.
> > > 3, All the E-cores which share the same L2-cache
> > >    are grouped to one module.
> > >
> > > The known limitation.
> > > 1, To tell a core is P-core or E-core is based on if
> > >    this core shares L2 cache with others.
> > >
> > > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > 
> > Side note:
> > This tool only exists because of lack of simple tool at the time.
> > Looking around, found that there is a tool 'lstopo' under the hwloc package
> > that gives output in many formats including graphical and seems to do a better
> > job than the DPDK python script.

lstopo existed already when introducing this python script.
See its home page: https://www.open-mpi.org/projects/hwloc/

We talked about dropping the DPDK script in the past:
http://inbox.dpdk.org/dev/20151127161008.GA27472@bricha3-MOBL3/

I recommend using "lstopo-no-graphics --merge"

> > Not sure how much farther DPDK should go in this area?
> > Really should be a distro tool.
> 
> Many thanks for your review and comments.
> Have to say I'm a green hand in this field. Just imitate the existing code to write mine. So, still trying to understand and handle the comments :)
> 
> Better to understand more about our opinion of this script before send a v2 patch.
> I've used 'lstopo'. It's a great tool.
> To my opinion, considering there're Linux tools to show all kinds of information, the reason that DPDK has its own tool is to summarize and emphasize the information that is important to DPDK. Here it's that some cores are more powerful than others. When the users use a testpmd-like APP, they can choose the appropriate cores after DPDK reminds them about the difference between cores.
> Add Thomas for more suggestions. Thanks.

Adding Brice, hwloc maintainer.

I think it would be better to contribute to the hwloc project.
If we need a different set of info, we can probably tune it with options.
  

On Fri, 21 Apr 2023 10:28:31 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> lstopo existed already when introducing this python script.
> See its home page: https://www.open-mpi.org/projects/hwloc/
> 
> We talked about dropping the DPDK script in the past:
> http://inbox.dpdk.org/dev/20151127161008.GA27472@bricha3-MOBL3/
> 
> I recommend using "lstopo-no-graphics --merge"
> 
> > > Not sure how much farther DPDK should go in this area?
> > > Really should be a distro tool.  
> > 
> > Many thanks for your review and comments.
> > Have to say I'm a green hand in this field. Just imitate the existing code to write mine. So, still trying to understand and handle the comments :)

My refinements were more to reduce the amount of repeated code. The overall idea of reporting
more is a good one.

> > 
> > Better to understand more about our opinion of this script before send a v2 patch.
> > I've used 'lstopo'. It's a great tool.
> > To my opinion, considering there're Linux tools to show all kinds of information, the reason that DPDK has its own tool is to summarize and emphasize the information that is important to DPDK. Here it's that some cores are more powerful than others. When the users use a testpmd-like APP, they can choose the appropriate cores after DPDK reminds them about the difference between cores.
> > Add Thomas for more suggestions. Thanks.  
> 
> Adding Brice, hwloc maintainer.
> 
> I think it would be better to contribute to the hwloc project.
> If we need a different set of info, we can probably tune it with options.

The script had a purpose which was back when DPDK was first started.
But as systems get more complex, it becomes something that has to deal with lots
of corner cases; and if some other tool can it then that is better.
  

Le 21/04/2023 à 17:15, Stephen Hemminger a écrit :
>
>>> Better to understand more about our opinion of this script before send a v2 patch.
>>> I've used 'lstopo'. It's a great tool.
>>> To my opinion, considering there're Linux tools to show all kinds of information, the reason that DPDK has its own tool is to summarize and emphasize the information that is important to DPDK. Here it's that some cores are more powerful than others. When the users use a testpmd-like APP, they can choose the appropriate cores after DPDK reminds them about the difference between cores.
>>> Add Thomas for more suggestions. Thanks.
>> Adding Brice, hwloc maintainer.
>>
>> I think it would be better to contribute to the hwloc project.
>> If we need a different set of info, we can probably tune it with options.
> The script had a purpose which was back when DPDK was first started.
> But as systems get more complex, it becomes something that has to deal with lots
> of corner cases; and if some other tool can it then that is better.


Hello

Indeed, hwloc/lstopo should be able to do something similar to that 
script. I didn't see anything network-related in the script, does it 
only show CPU info?

Regarding the original patch, we already support all levels or caches, 
dies, clusters, etc. Hybrid CPUs are also detected but they are only 
nicely shown in the graphical output [1]. The textual output only says 
at the very end that there are two kinds and the bitmask of CPUs for 
each. I am open to improving this.

Brice

[1] https://twitter.com/bgoglin/status/1542117836008706049/photo/1
  

Hi Stephen, Thomas, Brice,

> -----Original Message-----
> From: Brice Goglin <Brice.Goglin@inria.fr>
> Sent: Tuesday, April 25, 2023 1:06 AM
> To: Stephen Hemminger <stephen@networkplumber.org>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org;
> david.marchand@redhat.com
> Subject: Re: [PATCH] usertools: enhance CPU layout
> 
> Le 21/04/2023 à 17:15, Stephen Hemminger a écrit :
> >
> >>> Better to understand more about our opinion of this script before send a
> v2 patch.
> >>> I've used 'lstopo'. It's a great tool.
> >>> To my opinion, considering there're Linux tools to show all kinds of
> information, the reason that DPDK has its own tool is to summarize and
> emphasize the information that is important to DPDK. Here it's that some cores
> are more powerful than others. When the users use a testpmd-like APP, they
> can choose the appropriate cores after DPDK reminds them about the
> difference between cores.
> >>> Add Thomas for more suggestions. Thanks.
> >> Adding Brice, hwloc maintainer.
> >>
> >> I think it would be better to contribute to the hwloc project.
> >> If we need a different set of info, we can probably tune it with options.
> > The script had a purpose which was back when DPDK was first started.
> > But as systems get more complex, it becomes something that has to deal
> > with lots of corner cases; and if some other tool can it then that is better.
> 
> 
> Hello
> 
> Indeed, hwloc/lstopo should be able to do something similar to that script. I
> didn't see anything network-related in the script, does it only show CPU info?
Yes, this script shows only CPU info.
Agree that 'lstopo' already shows everything that this script can show.

> 
> Regarding the original patch, we already support all levels or caches, dies,
> clusters, etc. Hybrid CPUs are also detected but they are only nicely shown in
> the graphical output [1]. The textual output only says at the very end that there
> are two kinds and the bitmask of CPUs for each. I am open to improving this.
> 
> Brice
> 
> [1] https://twitter.com/bgoglin/status/1542117836008706049/photo/1

Brice, Thomas, Stephen, many thanks for your reply and comments.
As cpu_layout.py has been planned to be removed, and the work can been done by 'lstopo', I'll withdraw this patch.