Re: [PATCH] usertools: enhance CPU layout

[Stephen Hemminger <stephen@networkplumber.org>]

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