[PATCH v1 1/2] usertools/cpu_layout: update coding style

[PATCH v1 1/2] usertools/cpu_layout: update coding style

[Anatoly Burakov]

Update coding style:

- make it PEP-484 compliant
- address all flake8, mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 usertools/cpu_layout.py | 162 ++++++++++++++++++++++++++--------------
 1 file changed, 104 insertions(+), 58 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..843b29a134 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,108 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-    try:
-        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-    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)
-    if socket not in sockets:
-        sockets.append(socket)
-    key = (socket, core)
-    if key not in core_map:
-        core_map[key] = []
-    core_map[key].append(cpu)
-
-print(format("=" * (47 + len(base_path))))
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path))))
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
-
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count)  \
-                      + len(", ") * (max_thread_count - 1) \
-                      + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
-
-output = " ".ljust(max_core_id_len + len('Core '))
-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 '))
-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)
+from typing import List, Set, Dict, Tuple
+
+
+def _range_expand(rstr: str) -> List[int]:
+    """Expand a range string into a list of integers."""
+    # 0,1-3 => [0, 1-3]
+    ranges = rstr.split(",")
+    valset: List[int] = []
+    for r in ranges:
+        # 1-3 => [1, 2, 3]
+        if "-" in r:
+            start, end = r.split("-")
+            valset.extend(range(int(start), int(end) + 1))
         else:
-            output += " " * (max_core_map_len + 1)
-    print(output)
+            valset.append(int(r))
+    return valset
+
+
+def _read_sysfs(path: str) -> str:
+    with open(path) as fd:
+        return fd.read().strip()
+
+
+def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
+    first, *rest = row
+    w_first, *w_rest = col_widths
+    first_end = " " * 4
+    rest_end = " " * 10
+
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
+
+
+def _print_section(heading: str) -> None:
+    sep = "=" * len(heading)
+    print(sep)
+    print(heading)
+    print(sep)
+    print()
+
+
+def _main() -> None:
+    sockets_s: Set[int] = set()
+    cores_s: Set[int] = set()
+    core_map: Dict[Tuple[int, int], List[int]] = {}
+    base_path = "/sys/devices/system/cpu"
+
+    cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
+
+    for cpu in cpus:
+        lcore_base = f"{base_path}/cpu{cpu}"
+        core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
+        socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+        cores_s.add(core)
+        sockets_s.add(socket)
+        key = (socket, core)
+        core_map.setdefault(key, [])
+        core_map[key].append(cpu)
+
+    cores = sorted(cores_s)
+    sockets = sorted(sockets_s)
+
+    _print_section("Core and Socket Information "
+                   f"(as reported by '{base_path}')")
+
+    print("cores = ", cores)
+    print("sockets = ", sockets)
+    print()
+
+    # Core, [Socket, Socket, ...]
+    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
+    rows: List[Tuple[str, ...]] = []
+
+    for c in cores:
+        # Core,
+        row: Tuple[str, ...] = (f"Core {c}",)
+
+        # [lcores, lcores, ...]
+        for s in sockets:
+            try:
+                lcores = core_map[(s, c)]
+                row += (f"{lcores}",)
+            except KeyError:
+                row += ("",)
+        rows += [row]
+
+    # find max widths for each column, including header and rows
+    max_widths = [
+        max([len(tup[col_idx]) for tup in rows + [heading_strs]])
+        for col_idx in range(len(heading_strs))
+    ]
+
+    # print out table taking row widths into account
+    _print_row(heading_strs, max_widths)
+    _print_row(sep_strs, max_widths)
+    for row in rows:
+        _print_row(row, max_widths)
+
+
+if __name__ == "__main__":
+    _main()
-- 
2.43.5

[PATCH v1 2/2] usertools/cpu_layout: print out NUMA nodes

[Anatoly Burakov]

In traditional NUMA case, NUMA nodes and physical sockets were used
interchangeably, but there are cases where there can be multiple NUMA
nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
cases of multiple sockets. Use sysfs to print out NUMA information.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 usertools/cpu_layout.py | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 843b29a134..be89909464 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -4,6 +4,7 @@
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
 from typing import List, Set, Dict, Tuple
+import glob
 
 
 def _range_expand(rstr: str) -> List[int]:
@@ -26,11 +27,19 @@ def _read_sysfs(path: str) -> str:
         return fd.read().strip()
 
 
+def _read_numa_node(base: str) -> int:
+    node_glob = f"{base}/node*"
+    node_dirs = glob.glob(node_glob)
+    if not node_dirs:
+        return 0  # default to node 0
+    return int(node_dirs[0].split("node")[1])
+
+
 def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
     first, *rest = row
     w_first, *w_rest = col_widths
     first_end = " " * 4
-    rest_end = " " * 10
+    rest_end = " " * 4
 
     print(first.ljust(w_first), end=first_end)
     for cell, width in zip(rest, w_rest):
@@ -50,6 +59,7 @@ def _main() -> None:
     sockets_s: Set[int] = set()
     cores_s: Set[int] = set()
     core_map: Dict[Tuple[int, int], List[int]] = {}
+    numa_map: Dict[int, int] = {}
     base_path = "/sys/devices/system/cpu"
 
     cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
@@ -58,12 +68,14 @@ def _main() -> None:
         lcore_base = f"{base_path}/cpu{cpu}"
         core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
         socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+        node = _read_numa_node(lcore_base)
 
         cores_s.add(core)
         sockets_s.add(socket)
         key = (socket, core)
         core_map.setdefault(key, [])
         core_map[key].append(cpu)
+        numa_map[cpu] = node
 
     cores = sorted(cores_s)
     sockets = sorted(sockets_s)
@@ -73,24 +85,37 @@ def _main() -> None:
 
     print("cores = ", cores)
     print("sockets = ", sockets)
+    print("numa = ", sorted(set(numa_map.values())))
     print()
 
-    # Core, [Socket, Socket, ...]
-    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    # Core, [NUMA, Socket, NUMA, Socket, ...]
+    heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
     sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
     rows: List[Tuple[str, ...]] = []
 
+    prev_numa = None
     for c in cores:
         # Core,
         row: Tuple[str, ...] = (f"Core {c}",)
 
-        # [lcores, lcores, ...]
+        # assume NUMA changes symmetrically
+        first_lcore = core_map[(0, c)][0]
+        cur_numa = numa_map[first_lcore]
+        numa_changed = prev_numa != cur_numa
+        prev_numa = cur_numa
+
+        # [NUMA, lcores, NUMA, lcores, ...]
         for s in sockets:
             try:
                 lcores = core_map[(s, c)]
+                numa = numa_map[lcores[0]]
+                if numa_changed:
+                    row += (f"NUMA {numa}",)
+                else:
+                    row += ("",)
                 row += (f"{lcores}",)
             except KeyError:
-                row += ("",)
+                row += ("", "")
         rows += [row]
 
     # find max widths for each column, including header and rows
-- 
2.43.5

[PATCH v2 1/4] usertools/cpu_layout: update coding style

[Anatoly Burakov]

Update coding style:

- make it PEP-484 compliant
- address all flake8, mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 usertools/cpu_layout.py | 162 ++++++++++++++++++++++++++--------------
 1 file changed, 104 insertions(+), 58 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..be86f06938 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,108 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-    try:
-        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-    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)
-    if socket not in sockets:
-        sockets.append(socket)
-    key = (socket, core)
-    if key not in core_map:
-        core_map[key] = []
-    core_map[key].append(cpu)
-
-print(format("=" * (47 + len(base_path))))
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path))))
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
-
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count)  \
-                      + len(", ") * (max_thread_count - 1) \
-                      + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
-
-output = " ".ljust(max_core_id_len + len('Core '))
-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 '))
-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)
+from typing import List, Set, Dict, Tuple
+
+
+def _range_expand(rstr: str) -> List[int]:
+    """Expand a range string into a list of integers."""
+    # 0,1-3 => [0, 1-3]
+    ranges = rstr.split(",")
+    valset: List[int] = []
+    for r in ranges:
+        # 1-3 => [1, 2, 3]
+        if "-" in r:
+            start, end = r.split("-")
+            valset.extend(range(int(start), int(end) + 1))
         else:
-            output += " " * (max_core_map_len + 1)
-    print(output)
+            valset.append(int(r))
+    return valset
+
+
+def _read_sysfs(path: str) -> str:
+    with open(path, encoding="utf-8") as fd:
+        return fd.read().strip()
+
+
+def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
+    first, *rest = row
+    w_first, *w_rest = col_widths
+    first_end = " " * 4
+    rest_end = " " * 10
+
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
+
+
+def _print_section(heading: str) -> None:
+    sep = "=" * len(heading)
+    print(sep)
+    print(heading)
+    print(sep)
+    print()
+
+
+def _main() -> None:
+    sockets_s: Set[int] = set()
+    cores_s: Set[int] = set()
+    core_map: Dict[Tuple[int, int], List[int]] = {}
+    base_path = "/sys/devices/system/cpu"
+
+    cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
+
+    for cpu in cpus:
+        lcore_base = f"{base_path}/cpu{cpu}"
+        core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
+        socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+        cores_s.add(core)
+        sockets_s.add(socket)
+        key = (socket, core)
+        core_map.setdefault(key, [])
+        core_map[key].append(cpu)
+
+    cores = sorted(cores_s)
+    sockets = sorted(sockets_s)
+
+    _print_section("Core and Socket Information "
+                   f"(as reported by '{base_path}')")
+
+    print("cores = ", cores)
+    print("sockets = ", sockets)
+    print()
+
+    # Core, [Socket, Socket, ...]
+    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
+    rows: List[Tuple[str, ...]] = []
+
+    for c in cores:
+        # Core,
+        row: Tuple[str, ...] = (f"Core {c}",)
+
+        # [lcores, lcores, ...]
+        for s in sockets:
+            try:
+                lcores = core_map[(s, c)]
+                row += (str(lcores),)
+            except KeyError:
+                row += ("",)
+        rows += [row]
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max([len(tup[col_idx]) for tup in rows + [heading_strs]])
+        for col_idx in range(len(heading_strs))
+    ]
+
+    # print out table taking row widths into account
+    _print_row(heading_strs, col_widths)
+    _print_row(sep_strs, col_widths)
+    for row in rows:
+        _print_row(row, col_widths)
+
+
+if __name__ == "__main__":
+    _main()
-- 
2.43.5

[PATCH v2 2/4] usertools/cpu_layout: print out NUMA nodes

[Anatoly Burakov]

In traditional NUMA case, NUMA nodes and physical sockets were used
interchangeably, but there are cases where there can be multiple NUMA
nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
cases of multiple sockets. Use sysfs to print out NUMA information.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 usertools/cpu_layout.py | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index be86f06938..e43bdbf343 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -4,6 +4,7 @@
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
 from typing import List, Set, Dict, Tuple
+import glob
 
 
 def _range_expand(rstr: str) -> List[int]:
@@ -26,11 +27,19 @@ def _read_sysfs(path: str) -> str:
         return fd.read().strip()
 
 
+def _read_numa_node(base: str) -> int:
+    node_glob = f"{base}/node*"
+    node_dirs = glob.glob(node_glob)
+    if not node_dirs:
+        return 0  # default to node 0
+    return int(node_dirs[0].split("node")[1])
+
+
 def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
     first, *rest = row
     w_first, *w_rest = col_widths
     first_end = " " * 4
-    rest_end = " " * 10
+    rest_end = " " * 4
 
     print(first.ljust(w_first), end=first_end)
     for cell, width in zip(rest, w_rest):
@@ -50,6 +59,7 @@ def _main() -> None:
     sockets_s: Set[int] = set()
     cores_s: Set[int] = set()
     core_map: Dict[Tuple[int, int], List[int]] = {}
+    numa_map: Dict[int, int] = {}
     base_path = "/sys/devices/system/cpu"
 
     cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
@@ -58,12 +68,14 @@ def _main() -> None:
         lcore_base = f"{base_path}/cpu{cpu}"
         core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
         socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+        node = _read_numa_node(lcore_base)
 
         cores_s.add(core)
         sockets_s.add(socket)
         key = (socket, core)
         core_map.setdefault(key, [])
         core_map[key].append(cpu)
+        numa_map[cpu] = node
 
     cores = sorted(cores_s)
     sockets = sorted(sockets_s)
@@ -73,24 +85,37 @@ def _main() -> None:
 
     print("cores = ", cores)
     print("sockets = ", sockets)
+    print("numa = ", sorted(set(numa_map.values())))
     print()
 
-    # Core, [Socket, Socket, ...]
-    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    # Core, [NUMA, Socket, NUMA, Socket, ...]
+    heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
     sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
     rows: List[Tuple[str, ...]] = []
 
+    prev_numa = None
     for c in cores:
         # Core,
         row: Tuple[str, ...] = (f"Core {c}",)
 
-        # [lcores, lcores, ...]
+        # assume NUMA changes symmetrically
+        first_lcore = core_map[(0, c)][0]
+        cur_numa = numa_map[first_lcore]
+        numa_changed = prev_numa != cur_numa
+        prev_numa = cur_numa
+
+        # [NUMA, lcores, NUMA, lcores, ...]
         for s in sockets:
             try:
                 lcores = core_map[(s, c)]
+                numa = numa_map[lcores[0]]
+                if numa_changed:
+                    row += (f"NUMA {numa}",)
+                else:
+                    row += ("",)
                 row += (str(lcores),)
             except KeyError:
-                row += ("",)
+                row += ("", "")
         rows += [row]
 
     # find max widths for each column, including header and rows
-- 
2.43.5

Re: [PATCH v2 2/4] usertools/cpu_layout: print out NUMA nodes

[Robin Jarry]

Anatoly Burakov, Aug 16, 2024 at 14:16:
> In traditional NUMA case, NUMA nodes and physical sockets were used
> interchangeably, but there are cases where there can be multiple NUMA
> nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
> cases of multiple sockets. Use sysfs to print out NUMA information.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  usertools/cpu_layout.py | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
> index be86f06938..e43bdbf343 100755
> --- a/usertools/cpu_layout.py
> +++ b/usertools/cpu_layout.py
> @@ -4,6 +4,7 @@
>  # Copyright(c) 2017 Cavium, Inc. All rights reserved.
>  
>  from typing import List, Set, Dict, Tuple
> +import glob

Can you keep the import sorted alphabetically?

>  
>  
>  def _range_expand(rstr: str) -> List[int]:
> @@ -26,11 +27,19 @@ def _read_sysfs(path: str) -> str:
>          return fd.read().strip()
>  
>  
> +def _read_numa_node(base: str) -> int:
> +    node_glob = f"{base}/node*"
> +    node_dirs = glob.glob(node_glob)
> +    if not node_dirs:
> +        return 0  # default to node 0
> +    return int(node_dirs[0].split("node")[1])

Maybe you could make this safer and more "python"-ic as follows:

def read_numa_node(base: str) -> int:
    for node in glob.iglob(f"{base}/node*"):
        match = re.match(r"^.*/node(\d+)$", node)
        if match:
            return int(match.group(1))
    return 0  # default to node 0

> +
> +
>  def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
>      first, *rest = row
>      w_first, *w_rest = col_widths
>      first_end = " " * 4
> -    rest_end = " " * 10
> +    rest_end = " " * 4
>  
>      print(first.ljust(w_first), end=first_end)
>      for cell, width in zip(rest, w_rest):
> @@ -50,6 +59,7 @@ def _main() -> None:
>      sockets_s: Set[int] = set()
>      cores_s: Set[int] = set()
>      core_map: Dict[Tuple[int, int], List[int]] = {}
> +    numa_map: Dict[int, int] = {}
>      base_path = "/sys/devices/system/cpu"
>  
>      cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
> @@ -58,12 +68,14 @@ def _main() -> None:
>          lcore_base = f"{base_path}/cpu{cpu}"
>          core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
>          socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
> +        node = _read_numa_node(lcore_base)
>  
>          cores_s.add(core)
>          sockets_s.add(socket)
>          key = (socket, core)
>          core_map.setdefault(key, [])
>          core_map[key].append(cpu)
> +        numa_map[cpu] = node
>  
>      cores = sorted(cores_s)
>      sockets = sorted(sockets_s)
> @@ -73,24 +85,37 @@ def _main() -> None:
>  
>      print("cores = ", cores)
>      print("sockets = ", sockets)
> +    print("numa = ", sorted(set(numa_map.values())))
>      print()
>  
> -    # Core, [Socket, Socket, ...]
> -    heading_strs = "", *[f"Socket {s}" for s in sockets]
> +    # Core, [NUMA, Socket, NUMA, Socket, ...]
> +    heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
>      sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
>      rows: List[Tuple[str, ...]] = []
>  
> +    prev_numa = None
>      for c in cores:
>          # Core,
>          row: Tuple[str, ...] = (f"Core {c}",)
>  
> -        # [lcores, lcores, ...]
> +        # assume NUMA changes symmetrically
> +        first_lcore = core_map[(0, c)][0]
> +        cur_numa = numa_map[first_lcore]
> +        numa_changed = prev_numa != cur_numa
> +        prev_numa = cur_numa
> +
> +        # [NUMA, lcores, NUMA, lcores, ...]
>          for s in sockets:
>              try:
>                  lcores = core_map[(s, c)]
> +                numa = numa_map[lcores[0]]
> +                if numa_changed:
> +                    row += (f"NUMA {numa}",)
> +                else:
> +                    row += ("",)
>                  row += (str(lcores),)
>              except KeyError:
> -                row += ("",)
> +                row += ("", "")
>          rows += [row]
>  
>      # find max widths for each column, including header and rows
> -- 
> 2.43.5

[PATCH v2 3/4] usertools/dpdk-hugepages.py: sort by NUMA node

[Anatoly Burakov]

Currently, the list of per-NUMA node hugepages is displayed in glob order,
which can be arbitrary. Fix it to sort the glob order.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 usertools/dpdk-hugepages.py | 40 ++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index bf2575ba36..54232ddf22 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -74,21 +74,37 @@ def set_hugepages(path, reqpages):
                  gotpages, reqpages, filename))
 
 
+def get_numa_pages_node(node):
+    '''Read list of hugepage reservations on specific NUMA node'''
+    hp_path = f'/sys/devices/system/node/node{node}/hugepages'
+    if not os.path.exists(hp_path):
+        return
+    res = []
+    for pg_sz_dir in os.listdir(hp_path):
+        pg_sz = int(pg_sz_dir[10:-2])
+        nr_pages = get_hugepages(f'{hp_path}/{pg_sz_dir}')
+        if nr_pages > 0:
+            pg_sz_str = fmt_memsize(pg_sz)
+            total_sz_str = fmt_memsize(nr_pages * pg_sz)
+            res += [(nr_pages, pg_sz_str, total_sz_str)]
+        else:
+            res += [(0, None, None)]
+    return res
+
+
 def show_numa_pages():
     '''Show huge page reservations on Numa system'''
+    # get list of NUMA nodes and sort them by integer order
     print('Node Pages Size Total')
-    for numa_path in glob.glob('/sys/devices/system/node/node*'):
-        node = numa_path[29:]  # slice after /sys/devices/system/node/node
-        path = numa_path + '/hugepages'
-        if not os.path.exists(path):
-            continue
-        for hdir in os.listdir(path):
-            pages = get_hugepages(path + '/' + hdir)
-            if pages > 0:
-                kb = int(hdir[10:-2])  # slice out of hugepages-NNNkB
-                print('{:<4} {:<5} {:<6} {}'.format(node, pages,
-                                                    fmt_memsize(kb),
-                                                    fmt_memsize(pages * kb)))
+    nodes = sorted(int(node[29:])
+                   for node in glob.glob('/sys/devices/system/node/node*'))
+    for node in nodes:
+        pg_sz_data = get_numa_pages_node(node)
+        for nr_pages, pg_sz, total_sz in pg_sz_data:
+            if not nr_pages:
+                continue
+            print('{:<4} {:<5} {:<6} {}'
+                  .format(node, nr_pages, pg_sz, total_sz))
 
 
 def show_non_numa_pages():
-- 
2.43.5

Re: [PATCH v2 3/4] usertools/dpdk-hugepages.py: sort by NUMA node

[Burakov, Anatoly]

On 8/16/2024 2:16 PM, Anatoly Burakov wrote:
> Currently, the list of per-NUMA node hugepages is displayed in glob order,
> which can be arbitrary. Fix it to sort the glob order.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>   usertools/dpdk-hugepages.py | 40 ++++++++++++++++++++++++++-----------
>   1 file changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
> index bf2575ba36..54232ddf22 100755
> --- a/usertools/dpdk-hugepages.py
> +++ b/usertools/dpdk-hugepages.py
> @@ -74,21 +74,37 @@ def set_hugepages(path, reqpages):
>                    gotpages, reqpages, filename))
>   
>   
> +def get_numa_pages_node(node):
> +    '''Read list of hugepage reservations on specific NUMA node'''
> +    hp_path = f'/sys/devices/system/node/node{node}/hugepages'
> +    if not os.path.exists(hp_path):
> +        return

Oops, should have been return []

-- 
Thanks,
Anatoly

Re: [PATCH v2 3/4] usertools/dpdk-hugepages.py: sort by NUMA node

[Robin Jarry]

Anatoly Burakov, Aug 16, 2024 at 14:16:
> Currently, the list of per-NUMA node hugepages is displayed in glob order,
> which can be arbitrary. Fix it to sort the glob order.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Hey Anatoly,

I mean no offense to anyone but dpdk-hugepages.py is really ugly :(

Is this script really needed? If it is, maybe it would be a good 
opportunity to rewrite it. Is this something you'd be willing to work 
on?

> ---
>  usertools/dpdk-hugepages.py | 40 ++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
> index bf2575ba36..54232ddf22 100755
> --- a/usertools/dpdk-hugepages.py
> +++ b/usertools/dpdk-hugepages.py
> @@ -74,21 +74,37 @@ def set_hugepages(path, reqpages):
>                   gotpages, reqpages, filename))
>  
>  
> +def get_numa_pages_node(node):
> +    '''Read list of hugepage reservations on specific NUMA node'''
> +    hp_path = f'/sys/devices/system/node/node{node}/hugepages'
> +    if not os.path.exists(hp_path):
> +        return
> +    res = []
> +    for pg_sz_dir in os.listdir(hp_path):
> +        pg_sz = int(pg_sz_dir[10:-2])
> +        nr_pages = get_hugepages(f'{hp_path}/{pg_sz_dir}')
> +        if nr_pages > 0:
> +            pg_sz_str = fmt_memsize(pg_sz)
> +            total_sz_str = fmt_memsize(nr_pages * pg_sz)
> +            res += [(nr_pages, pg_sz_str, total_sz_str)]
> +        else:
> +            res += [(0, None, None)]
> +    return res
> +
> +
>  def show_numa_pages():
>      '''Show huge page reservations on Numa system'''
> +    # get list of NUMA nodes and sort them by integer order
>      print('Node Pages Size Total')
> -    for numa_path in glob.glob('/sys/devices/system/node/node*'):
> -        node = numa_path[29:]  # slice after /sys/devices/system/node/node
> -        path = numa_path + '/hugepages'
> -        if not os.path.exists(path):
> -            continue
> -        for hdir in os.listdir(path):
> -            pages = get_hugepages(path + '/' + hdir)
> -            if pages > 0:
> -                kb = int(hdir[10:-2])  # slice out of hugepages-NNNkB
> -                print('{:<4} {:<5} {:<6} {}'.format(node, pages,
> -                                                    fmt_memsize(kb),
> -                                                    fmt_memsize(pages * kb)))
> +    nodes = sorted(int(node[29:])
> +                   for node in glob.glob('/sys/devices/system/node/node*'))
> +    for node in nodes:
> +        pg_sz_data = get_numa_pages_node(node)
> +        for nr_pages, pg_sz, total_sz in pg_sz_data:
> +            if not nr_pages:
> +                continue
> +            print('{:<4} {:<5} {:<6} {}'
> +                  .format(node, nr_pages, pg_sz, total_sz))
>  
>  
>  def show_non_numa_pages():
> -- 
> 2.43.5

Re: [PATCH v2 3/4] usertools/dpdk-hugepages.py: sort by NUMA node

[Burakov, Anatoly]

On 8/19/2024 1:32 PM, Robin Jarry wrote:
> Anatoly Burakov, Aug 16, 2024 at 14:16:
>> Currently, the list of per-NUMA node hugepages is displayed in glob 
>> order,
>> which can be arbitrary. Fix it to sort the glob order.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Hey Anatoly,
> 
> I mean no offense to anyone but dpdk-hugepages.py is really ugly :(
> 
> Is this script really needed? If it is, maybe it would be a good 
> opportunity to rewrite it. Is this something you'd be willing to work on?

I do agree it is, umm, let's say not up to Python standards, but I was 
attempting to minimize changes to ensure this change gets integrated 
quickly. I can try and rewrite it if that's something that community is 
willing to accept. I'm not a master Pythonist by any stretch, but I do 
like to think I'm at least capable :D

-- 
Thanks,
Anatoly

Re: [PATCH v2 3/4] usertools/dpdk-hugepages.py: sort by NUMA node

[Robin Jarry]

Burakov, Anatoly, Aug 20, 2024 at 11:04:
> I do agree it is, umm, let's say not up to Python standards, but I was 
> attempting to minimize changes to ensure this change gets integrated 
> quickly. I can try and rewrite it if that's something that community is 
> willing to accept. I'm not a master Pythonist by any stretch, but I do 
> like to think I'm at least capable :D

That would be awesome, thanks!

[PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Anatoly Burakov]

Currently, devbind does not print out any NUMA information, which makes
figuring out which NUMA node device belongs to not trivial. Add printouts
for NUMA information if NUMA support is enabled on the system.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 usertools/dpdk-devbind.py | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index b276e8efc8..c0611a501d 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -110,6 +110,11 @@
 args = []
 
 
+# check if this system has NUMA support
+def is_numa():
+    return os.path.exists('/sys/devices/system/node')
+
+
 # check if a specific kernel module is loaded
 def module_is_loaded(module):
     global loaded_modules
@@ -579,18 +584,24 @@ def show_device_status(devices_type, device_name, if_field=False):
 
     # print each category separately, so we can clearly see what's used by DPDK
     if dpdk_drv:
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
+        if is_numa():
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
         display_devices("%s devices using DPDK-compatible driver" % device_name,
-                        dpdk_drv, "drv=%(Driver_str)s unused=%(Module_str)s")
+                        dpdk_drv, extra_param)
     if kernel_drv:
-        if_text = ""
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
         if if_field:
-            if_text = "if=%(Interface)s "
-        display_devices("%s devices using kernel driver" % device_name, kernel_drv,
-                        if_text + "drv=%(Driver_str)s "
-                        "unused=%(Module_str)s %(Active)s")
+            extra_param = "if=%(Interface)s " + extra_param
+        if is_numa():
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("%s devices using kernel driver" % device_name,
+                        kernel_drv, extra_param)
     if no_drv:
-        display_devices("Other %s devices" % device_name, no_drv,
-                        "unused=%(Module_str)s")
+        extra_param = "unused=%(Module_str)s"
+        if is_numa():
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("Other %s devices" % device_name, no_drv, extra_param)
 
 
 def show_status():
-- 
2.43.5

Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Robin Jarry]

Anatoly Burakov, Aug 16, 2024 at 14:16:
> Currently, devbind does not print out any NUMA information, which makes
> figuring out which NUMA node device belongs to not trivial. Add printouts
> for NUMA information if NUMA support is enabled on the system.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Acked-by: Robin Jarry <rjarry@redhat.com>

NB: Although it is better than dpdk-hugepages.py, this script could also 
benefit from a major cleanup as you did for cpu_layout.py.

>  usertools/dpdk-devbind.py | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> index b276e8efc8..c0611a501d 100755
> --- a/usertools/dpdk-devbind.py
> +++ b/usertools/dpdk-devbind.py
> @@ -110,6 +110,11 @@
>  args = []
>  
>  
> +# check if this system has NUMA support
> +def is_numa():
> +    return os.path.exists('/sys/devices/system/node')
> +
> +
>  # check if a specific kernel module is loaded
>  def module_is_loaded(module):
>      global loaded_modules
> @@ -579,18 +584,24 @@ def show_device_status(devices_type, device_name, if_field=False):
>  
>      # print each category separately, so we can clearly see what's used by DPDK
>      if dpdk_drv:
> +        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
> +        if is_numa():
> +            extra_param = "numa_node=%(NUMANode)s " + extra_param
>          display_devices("%s devices using DPDK-compatible driver" % device_name,
> -                        dpdk_drv, "drv=%(Driver_str)s unused=%(Module_str)s")
> +                        dpdk_drv, extra_param)
>      if kernel_drv:
> -        if_text = ""
> +        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
>          if if_field:
> -            if_text = "if=%(Interface)s "
> -        display_devices("%s devices using kernel driver" % device_name, kernel_drv,
> -                        if_text + "drv=%(Driver_str)s "
> -                        "unused=%(Module_str)s %(Active)s")
> +            extra_param = "if=%(Interface)s " + extra_param
> +        if is_numa():
> +            extra_param = "numa_node=%(NUMANode)s " + extra_param
> +        display_devices("%s devices using kernel driver" % device_name,
> +                        kernel_drv, extra_param)
>      if no_drv:
> -        display_devices("Other %s devices" % device_name, no_drv,
> -                        "unused=%(Module_str)s")
> +        extra_param = "unused=%(Module_str)s"
> +        if is_numa():
> +            extra_param = "numa_node=%(NUMANode)s " + extra_param
> +        display_devices("Other %s devices" % device_name, no_drv, extra_param)
>  
>  
>  def show_status():
> -- 
> 2.43.5

Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Burakov, Anatoly]

On 8/19/2024 1:34 PM, Robin Jarry wrote:
> Anatoly Burakov, Aug 16, 2024 at 14:16:
>> Currently, devbind does not print out any NUMA information, which makes
>> figuring out which NUMA node device belongs to not trivial. Add printouts
>> for NUMA information if NUMA support is enabled on the system.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
> 
> Acked-by: Robin Jarry <rjarry@redhat.com>
> 
> NB: Although it is better than dpdk-hugepages.py, this script could also 
> benefit from a major cleanup as you did for cpu_layout.py.

Rewriting this one would take a bit more time because unlike working 
with hugepages or sysfs CPU layouts, I'm not familiar enough with all of 
the specifics this script does, so I'd have to learn it more. I can try 
though, if you think it's worth the effort?

-- 
Thanks,
Anatoly

Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Robin Jarry]

Burakov, Anatoly, Aug 20, 2024 at 11:08:
> Rewriting this one would take a bit more time because unlike working 
> with hugepages or sysfs CPU layouts, I'm not familiar enough with all of 
> the specifics this script does, so I'd have to learn it more. I can try 
> though, if you think it's worth the effort?

If you have better things to do. It is probably not worth the effort. 
This script is partially redundant with driverctl:

https://gitlab.com/driverctl/driverctl

I guess we cannot completely get rid of it but it is also OK not to 
invest too much into it :)

Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Burakov, Anatoly]

On 8/20/2024 11:28 AM, Robin Jarry wrote:
> Burakov, Anatoly, Aug 20, 2024 at 11:08:
>> Rewriting this one would take a bit more time because unlike working 
>> with hugepages or sysfs CPU layouts, I'm not familiar enough with all 
>> of the specifics this script does, so I'd have to learn it more. I can 
>> try though, if you think it's worth the effort?
> 
> If you have better things to do. It is probably not worth the effort. 
> This script is partially redundant with driverctl:
> 
> https://gitlab.com/driverctl/driverctl
> 
> I guess we cannot completely get rid of it but it is also OK not to 
> invest too much into it :)
> 

I have heard about driverctl for a long time in context of devbind, and 
I quickly tried it out just now, and IMO while the *functionality* is 
there, the usability of devbind is IMO far more friendly:

- it filters by device types that are of interest to us (driverctl just 
lists all pci devices)
- it prints out user-friendly names, not just PCI addresses
- it lists loaded drivers, but not alternatives etc. like devbind does
- I didn't test this, but I suspect it would allow overriding driver for 
an active NIC
- after my changes, devbind displays NUMA socket, driverctl doesn't :)

What we maybe can do for e.g. next release (currently I do have better 
things to do if I can help it), is to make devbind a wrapper around 
driverctl? This way, we can still keep the usability bells and whistles 
we have, but drop the driver overriding code and leave it to driverctl? 
It seems like that would be a good compromise, with the only downside 
being that we'll depend on driverctl being installed.

-- 
Thanks,
Anatoly

Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Robin Jarry]

Burakov, Anatoly, Aug 20, 2024 at 11:40:
> I have heard about driverctl for a long time in context of devbind, and 
> I quickly tried it out just now, and IMO while the *functionality* is 
> there, the usability of devbind is IMO far more friendly:
>
> - it filters by device types that are of interest to us (driverctl just 
> lists all pci devices)
> - it prints out user-friendly names, not just PCI addresses
> - it lists loaded drivers, but not alternatives etc. like devbind does
> - I didn't test this, but I suspect it would allow overriding driver for 
> an active NIC
> - after my changes, devbind displays NUMA socket, driverctl doesn't :)

Yes, that's fair. devbind is more a dev oriented tool. driverctl is 
really focused on reproducible and persistent driver override (it has 
a systemd service that will force a rebind of pci devices on reboot).

> What we maybe can do for e.g. next release (currently I do have better 
> things to do if I can help it), is to make devbind a wrapper around 
> driverctl? This way, we can still keep the usability bells and whistles 
> we have, but drop the driver overriding code and leave it to driverctl? 
> It seems like that would be a good compromise, with the only downside 
> being that we'll depend on driverctl being installed.

Honestly, I don't think the dependency would be a good idea. Also, 
driverctl does not have any option to make a driver override temporary. 
It will persist after reboot which is not what devbind does.

Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Burakov, Anatoly]

On 8/20/2024 11:49 AM, Robin Jarry wrote:
> Burakov, Anatoly, Aug 20, 2024 at 11:40:
>> I have heard about driverctl for a long time in context of devbind, 
>> and I quickly tried it out just now, and IMO while the *functionality* 
>> is there, the usability of devbind is IMO far more friendly:
>>
>> - it filters by device types that are of interest to us (driverctl 
>> just lists all pci devices)
>> - it prints out user-friendly names, not just PCI addresses
>> - it lists loaded drivers, but not alternatives etc. like devbind does
>> - I didn't test this, but I suspect it would allow overriding driver 
>> for an active NIC
>> - after my changes, devbind displays NUMA socket, driverctl doesn't :)
> 
> Yes, that's fair. devbind is more a dev oriented tool. driverctl is 
> really focused on reproducible and persistent driver override (it has a 
> systemd service that will force a rebind of pci devices on reboot).

Actually, I should correct myself (and yourself) somewhat.

This is not obvious from help message (I found it through reading the 
REAME) but driverctl *does* allow listing PCI devices by type, but only 
one type (e.g. list all network devices), but not multiple (e.g. list 
network and mempool devices), and as far as I can tell it does not 
support some of the device classes that we're interested in, so we will 
have to use the entire PCI device list and then filter it.

It does also print out device names (adding "verbose" option does it).

And it also *does* allow to bind/unbind drivers without persistence - 
--nosave option does that.

Still, all of the other points apply: devbind is much more useful for 
DPDK developers as it provides information relevant to us right out of 
the box.

> 
>> What we maybe can do for e.g. next release (currently I do have better 
>> things to do if I can help it), is to make devbind a wrapper around 
>> driverctl? This way, we can still keep the usability bells and 
>> whistles we have, but drop the driver overriding code and leave it to 
>> driverctl? It seems like that would be a good compromise, with the 
>> only downside being that we'll depend on driverctl being installed.
> 
> Honestly, I don't think the dependency would be a good idea. Also, 
> driverctl does not have any option to make a driver override temporary. 
> It will persist after reboot which is not what devbind does.
> 

See above, this is actually not true: driverctl can override a driver 
without persistence.

-- 
Thanks,
Anatoly

Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node

[Robin Jarry]

Burakov, Anatoly, Aug 20, 2024 at 11:56:
> Actually, I should correct myself (and yourself) somewhat.
>
> This is not obvious from help message (I found it through reading the 
> REAME) but driverctl *does* allow listing PCI devices by type, but only 
> one type (e.g. list all network devices), but not multiple (e.g. list 
> network and mempool devices), and as far as I can tell it does not 
> support some of the device classes that we're interested in, so we will 
> have to use the entire PCI device list and then filter it.
>
> It does also print out device names (adding "verbose" option does it).
>
> And it also *does* allow to bind/unbind drivers without persistence - 
> --nosave option does that.

Hehe, I had glanced over this option on the man page but didn't pay 
attention. Thanks!

> Still, all of the other points apply: devbind is much more useful for 
> DPDK developers as it provides information relevant to us right out of 
> the box.

Let's keep it as it is then. No need to impose another dependency. The 
driver override code isn't that complicated.

Re: [PATCH v2 1/4] usertools/cpu_layout: update coding style

[Robin Jarry]

Anatoly Burakov, Aug 16, 2024 at 14:16:
> Update coding style:
>
> - make it PEP-484 compliant
> - address all flake8, mypy etc. warnings
> - use f-strings in place of old-style string interpolation
> - refactor printing to make the code more readable
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  usertools/cpu_layout.py | 162 ++++++++++++++++++++++++++--------------
>  1 file changed, 104 insertions(+), 58 deletions(-)
>
> diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
> index 891b9238fa..be86f06938 100755
> --- a/usertools/cpu_layout.py
> +++ b/usertools/cpu_layout.py
> @@ -3,62 +3,108 @@
>  # Copyright(c) 2010-2014 Intel Corporation
>  # Copyright(c) 2017 Cavium, Inc. All rights reserved.
>  
> -sockets = []
> -cores = []
> -core_map = {}
> -base_path = "/sys/devices/system/cpu"
> -fd = open("{}/kernel_max".format(base_path))
> -max_cpus = int(fd.read())
> -fd.close()
> -for cpu in range(max_cpus + 1):
> -    try:
> -        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
> -    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)
> -    if socket not in sockets:
> -        sockets.append(socket)
> -    key = (socket, core)
> -    if key not in core_map:
> -        core_map[key] = []
> -    core_map[key].append(cpu)
> -
> -print(format("=" * (47 + len(base_path))))
> -print("Core and Socket Information (as reported by '{}')".format(base_path))
> -print("{}\n".format("=" * (47 + len(base_path))))
> -print("cores = ", cores)
> -print("sockets = ", sockets)
> -print("")
> -
> -max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
> -max_thread_count = len(list(core_map.values())[0])
> -max_core_map_len = (max_processor_len * max_thread_count)  \
> -                      + len(", ") * (max_thread_count - 1) \
> -                      + len('[]') + len('Socket ')
> -max_core_id_len = len(str(max(cores)))
> -
> -output = " ".ljust(max_core_id_len + len('Core '))
> -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 '))
> -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)
> +from typing import List, Set, Dict, Tuple

Minor nit pick: recently I started using `import typing as T` and then 
referencing T.List, T.Tuple, etc.

This avoids clobbering the patches when new symbols from the typing 
module are required and using a short alias keeps the code readable.

> +
> +
> +def _range_expand(rstr: str) -> List[int]:

def range_expand(rstr: str) -> T.List[int]:

> +    """Expand a range string into a list of integers."""
> +    # 0,1-3 => [0, 1-3]
> +    ranges = rstr.split(",")
> +    valset: List[int] = []
> +    for r in ranges:
> +        # 1-3 => [1, 2, 3]
> +        if "-" in r:
> +            start, end = r.split("-")
> +            valset.extend(range(int(start), int(end) + 1))
>          else:
> -            output += " " * (max_core_map_len + 1)
> -    print(output)
> +            valset.append(int(r))
> +    return valset
> +
> +
> +def _read_sysfs(path: str) -> str:
> +    with open(path, encoding="utf-8") as fd:
> +        return fd.read().strip()
> +
> +
> +def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:

def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:

> +    first, *rest = row
> +    w_first, *w_rest = col_widths
> +    first_end = " " * 4
> +    rest_end = " " * 10
> +
> +    print(first.ljust(w_first), end=first_end)
> +    for cell, width in zip(rest, w_rest):
> +        print(cell.rjust(width), end=rest_end)
> +    print()
> +
> +
> +def _print_section(heading: str) -> None:
> +    sep = "=" * len(heading)
> +    print(sep)
> +    print(heading)
> +    print(sep)
> +    print()
> +
> +
> +def _main() -> None:
> +    sockets_s: Set[int] = set()
> +    cores_s: Set[int] = set()
> +    core_map: Dict[Tuple[int, int], List[int]] = {}

    sockets_s: T.Set[int] = set()
    cores_s: T.Set[int] = set()
    core_map: T.Dict[Tuple[int, int], T.List[int]] = {}

> +    base_path = "/sys/devices/system/cpu"
> +
> +    cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
> +
> +    for cpu in cpus:
> +        lcore_base = f"{base_path}/cpu{cpu}"
> +        core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
> +        socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
> +
> +        cores_s.add(core)
> +        sockets_s.add(socket)
> +        key = (socket, core)
> +        core_map.setdefault(key, [])
> +        core_map[key].append(cpu)
> +
> +    cores = sorted(cores_s)
> +    sockets = sorted(sockets_s)
> +
> +    _print_section("Core and Socket Information "
> +                   f"(as reported by '{base_path}')")
> +
> +    print("cores = ", cores)
> +    print("sockets = ", sockets)
> +    print()
> +
> +    # Core, [Socket, Socket, ...]
> +    heading_strs = "", *[f"Socket {s}" for s in sockets]
> +    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
> +    rows: List[Tuple[str, ...]] = []

    rows: T.List[T.Tuple[str, ...]] = []

> +
> +    for c in cores:
> +        # Core,
> +        row: Tuple[str, ...] = (f"Core {c}",)

        row: T.Tuple[str, ...] = (f"Core {c}",)

> +
> +        # [lcores, lcores, ...]
> +        for s in sockets:
> +            try:
> +                lcores = core_map[(s, c)]
> +                row += (str(lcores),)
> +            except KeyError:
> +                row += ("",)
> +        rows += [row]
> +
> +    # find max widths for each column, including header and rows
> +    col_widths = [
> +        max([len(tup[col_idx]) for tup in rows + [heading_strs]])
> +        for col_idx in range(len(heading_strs))
> +    ]
> +
> +    # print out table taking row widths into account
> +    _print_row(heading_strs, col_widths)
> +    _print_row(sep_strs, col_widths)
> +    for row in rows:
> +        _print_row(row, col_widths)
> +
> +
> +if __name__ == "__main__":
> +    _main()
> -- 
> 2.43.5

FYI: if we change the minimum supported python version to 3.9, we could 
even get rid of this import completely as builtin types `set`, `list`, 
`tuple` and `dict` have become subscriptable.

    https://peps.python.org/pep-0585/

You can use them directly instead of the symbols defined in the typing 
module. E.g. the following becomes valid syntax:

    def foo(a: list[set[int]]) -> dict[str, tuple[str]]:

Instead of the more verbose:

    def foo(a: T.List[T.Set[int]]) -> T.Dict[str, T.Tuple[str]]:

But we can keep that for another day.

Re: [PATCH v2 1/4] usertools/cpu_layout: update coding style

[Burakov, Anatoly]

On 8/19/2024 1:11 PM, Robin Jarry wrote:
> Anatoly Burakov, Aug 16, 2024 at 14:16:
>> Update coding style:
>>
>> - make it PEP-484 compliant
>> - address all flake8, mypy etc. warnings
>> - use f-strings in place of old-style string interpolation
>> - refactor printing to make the code more readable
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---

<...>

> 
> FYI: if we change the minimum supported python version to 3.9, we could 
> even get rid of this import completely as builtin types `set`, `list`, 
> `tuple` and `dict` have become subscriptable.
> 
>     https://peps.python.org/pep-0585/
> 
> You can use them directly instead of the symbols defined in the typing 
> module. E.g. the following becomes valid syntax:
> 
>     def foo(a: list[set[int]]) -> dict[str, tuple[str]]:
> 
> Instead of the more verbose:
> 
>     def foo(a: T.List[T.Set[int]]) -> T.Dict[str, T.Tuple[str]]:
> 
> But we can keep that for another day.
> 

Incidentally, `dpdk-pmdinfo.py` script does use this syntax already. So 
either our minimum Python requirement is already met for this, or 
pmdinfo needs to be fixed to avoid it.

-- 
Thanks,
Anatoly

Re: [PATCH v2 1/4] usertools/cpu_layout: update coding style

[Robin Jarry]

Burakov, Anatoly, Aug 20, 2024 at 11:12:
> > FYI: if we change the minimum supported python version to 3.9, we could 
> > even get rid of this import completely as builtin types `set`, `list`, 
> > `tuple` and `dict` have become subscriptable.
> > 
> >     https://peps.python.org/pep-0585/
> > 
> > You can use them directly instead of the symbols defined in the typing 
> > module. E.g. the following becomes valid syntax:
> > 
> >     def foo(a: list[set[int]]) -> dict[str, tuple[str]]:
> > 
> > Instead of the more verbose:
> > 
> >     def foo(a: T.List[T.Set[int]]) -> T.Dict[str, T.Tuple[str]]:
> > 
> > But we can keep that for another day.
> > 
>
> Incidentally, `dpdk-pmdinfo.py` script does use this syntax already. So 
> either our minimum Python requirement is already met for this, or 
> pmdinfo needs to be fixed to avoid it.

I checked and unless I missed something, dpdk-pmdinfo.py uses the 
symbols from the typing module for annotations. Not builtin container 
types.

But I think that we should bump the minimum python version to 3.9. 3.6 
is really old...

Re: [PATCH v2 1/4] usertools/cpu_layout: update coding style

[Burakov, Anatoly]

On 8/20/2024 11:20 AM, Robin Jarry wrote:
> Burakov, Anatoly, Aug 20, 2024 at 11:12:
>> > FYI: if we change the minimum supported python version to 3.9, we 
>> could > even get rid of this import completely as builtin types `set`, 
>> `list`, > `tuple` and `dict` have become subscriptable.
>> > >     https://peps.python.org/pep-0585/
>> > > You can use them directly instead of the symbols defined in the 
>> typing > module. E.g. the following becomes valid syntax:
>> > >     def foo(a: list[set[int]]) -> dict[str, tuple[str]]:
>> > > Instead of the more verbose:
>> > >     def foo(a: T.List[T.Set[int]]) -> T.Dict[str, T.Tuple[str]]:
>> > > But we can keep that for another day.
>> >
>> Incidentally, `dpdk-pmdinfo.py` script does use this syntax already. 
>> So either our minimum Python requirement is already met for this, or 
>> pmdinfo needs to be fixed to avoid it.
> 
> I checked and unless I missed something, dpdk-pmdinfo.py uses the 
> symbols from the typing module for annotations. Not builtin container 
> types.

It does both. Check e.g. line 147:

def scrub_pci_ids(info: dict):

> 
> But I think that we should bump the minimum python version to 3.9. 3.6 
> is really old...
> 

Well, that's not really up to me to decide :)

-- 
Thanks,
Anatoly

Re: [PATCH v2 1/4] usertools/cpu_layout: update coding style

[Robin Jarry]

Burakov, Anatoly, Aug 20, 2024 at 11:31:
> > I checked and unless I missed something, dpdk-pmdinfo.py uses the 
> > symbols from the typing module for annotations. Not builtin container 
> > types.
>
> It does both. Check e.g. line 147:
>
> def scrub_pci_ids(info: dict):

Ah, that does not require python 3.9. The dict type is not subscripted 
with key and value types. This syntax works since python 3.5.

What would not work is something as follows:

def scrub_pci_ids(info: dict[str, list[int]]):

Re: [PATCH v2 1/4] usertools/cpu_layout: update coding style

[Burakov, Anatoly]

On 8/20/2024 11:45 AM, Robin Jarry wrote:
> Burakov, Anatoly, Aug 20, 2024 at 11:31:
>> > I checked and unless I missed something, dpdk-pmdinfo.py uses the > 
>> symbols from the typing module for annotations. Not builtin container 
>> > types.
>>
>> It does both. Check e.g. line 147:
>>
>> def scrub_pci_ids(info: dict):
> 
> Ah, that does not require python 3.9. The dict type is not subscripted 
> with key and value types. This syntax works since python 3.5.
> 
> What would not work is something as follows:
> 
> def scrub_pci_ids(info: dict[str, list[int]]):
> 

I stand corrected then!

-- 
Thanks,
Anatoly

Re: [PATCH v1 1/2] usertools/cpu_layout: update coding style

[Robin Jarry]

Anatoly Burakov, Aug 14, 2024 at 13:19:
> Update coding style:
>
> - make it PEP-484 compliant
> - address all flake8, mypy etc. warnings
> - use f-strings in place of old-style string interpolation
> - refactor printing to make the code more readable
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Hi Anatoly,

thanks for the patch. Did you format the code using black/ruff? I'd like 
to start keeping python code formatting consistent across user tools.

>  usertools/cpu_layout.py | 162 ++++++++++++++++++++++++++--------------
>  1 file changed, 104 insertions(+), 58 deletions(-)
>
> diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
> index 891b9238fa..843b29a134 100755
> --- a/usertools/cpu_layout.py
> +++ b/usertools/cpu_layout.py
> @@ -3,62 +3,108 @@
>  # Copyright(c) 2010-2014 Intel Corporation
>  # Copyright(c) 2017 Cavium, Inc. All rights reserved.
>  
> -sockets = []
> -cores = []
> -core_map = {}
> -base_path = "/sys/devices/system/cpu"
> -fd = open("{}/kernel_max".format(base_path))
> -max_cpus = int(fd.read())
> -fd.close()
> -for cpu in range(max_cpus + 1):
> -    try:
> -        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
> -    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)
> -    if socket not in sockets:
> -        sockets.append(socket)
> -    key = (socket, core)
> -    if key not in core_map:
> -        core_map[key] = []
> -    core_map[key].append(cpu)
> -
> -print(format("=" * (47 + len(base_path))))
> -print("Core and Socket Information (as reported by '{}')".format(base_path))
> -print("{}\n".format("=" * (47 + len(base_path))))
> -print("cores = ", cores)
> -print("sockets = ", sockets)
> -print("")
> -
> -max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
> -max_thread_count = len(list(core_map.values())[0])
> -max_core_map_len = (max_processor_len * max_thread_count)  \
> -                      + len(", ") * (max_thread_count - 1) \
> -                      + len('[]') + len('Socket ')
> -max_core_id_len = len(str(max(cores)))
> -
> -output = " ".ljust(max_core_id_len + len('Core '))
> -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 '))
> -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)
> +from typing import List, Set, Dict, Tuple
> +
> +
> +def _range_expand(rstr: str) -> List[int]:

I don't see any reason to prefix the symbols with an underscore in this 
script. It will not be used as an importable library.

> +    """Expand a range string into a list of integers."""
> +    # 0,1-3 => [0, 1-3]
> +    ranges = rstr.split(",")
> +    valset: List[int] = []
> +    for r in ranges:
> +        # 1-3 => [1, 2, 3]
> +        if "-" in r:
> +            start, end = r.split("-")
> +            valset.extend(range(int(start), int(end) + 1))
>          else:
> -            output += " " * (max_core_map_len + 1)
> -    print(output)
> +            valset.append(int(r))
> +    return valset
> +
> +
> +def _read_sysfs(path: str) -> str:
> +    with open(path) as fd:
> +        return fd.read().strip()
> +
> +
> +def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
> +    first, *rest = row
> +    w_first, *w_rest = col_widths
> +    first_end = " " * 4
> +    rest_end = " " * 10
> +
> +    print(first.ljust(w_first), end=first_end)
> +    for cell, width in zip(rest, w_rest):
> +        print(cell.rjust(width), end=rest_end)
> +    print()
> +
> +
> +def _print_section(heading: str) -> None:
> +    sep = "=" * len(heading)
> +    print(sep)
> +    print(heading)
> +    print(sep)
> +    print()
> +
> +
> +def _main() -> None:
> +    sockets_s: Set[int] = set()
> +    cores_s: Set[int] = set()
> +    core_map: Dict[Tuple[int, int], List[int]] = {}
> +    base_path = "/sys/devices/system/cpu"
> +
> +    cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
> +
> +    for cpu in cpus:
> +        lcore_base = f"{base_path}/cpu{cpu}"
> +        core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
> +        socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
> +
> +        cores_s.add(core)
> +        sockets_s.add(socket)
> +        key = (socket, core)
> +        core_map.setdefault(key, [])
> +        core_map[key].append(cpu)
> +
> +    cores = sorted(cores_s)
> +    sockets = sorted(sockets_s)
> +
> +    _print_section("Core and Socket Information "
> +                   f"(as reported by '{base_path}')")
> +
> +    print("cores = ", cores)
> +    print("sockets = ", sockets)
> +    print()
> +
> +    # Core, [Socket, Socket, ...]
> +    heading_strs = "", *[f"Socket {s}" for s in sockets]
> +    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
> +    rows: List[Tuple[str, ...]] = []
> +
> +    for c in cores:
> +        # Core,
> +        row: Tuple[str, ...] = (f"Core {c}",)
> +
> +        # [lcores, lcores, ...]
> +        for s in sockets:
> +            try:
> +                lcores = core_map[(s, c)]
> +                row += (f"{lcores}",)
> +            except KeyError:
> +                row += ("",)
> +        rows += [row]
> +
> +    # find max widths for each column, including header and rows
> +    max_widths = [
> +        max([len(tup[col_idx]) for tup in rows + [heading_strs]])
> +        for col_idx in range(len(heading_strs))
> +    ]
> +
> +    # print out table taking row widths into account
> +    _print_row(heading_strs, max_widths)
> +    _print_row(sep_strs, max_widths)
> +    for row in rows:
> +        _print_row(row, max_widths)
> +
> +
> +if __name__ == "__main__":
> +    _main()
> -- 
> 2.43.5

Re: [PATCH v1 1/2] usertools/cpu_layout: update coding style

[Burakov, Anatoly]

On 8/19/2024 11:26 AM, Robin Jarry wrote:
> Anatoly Burakov, Aug 14, 2024 at 13:19:
>> Update coding style:
>>
>> - make it PEP-484 compliant
>> - address all flake8, mypy etc. warnings
>> - use f-strings in place of old-style string interpolation
>> - refactor printing to make the code more readable
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
> 
> Hi Anatoly,
> 
> thanks for the patch. Did you format the code using black/ruff? I'd like 
> to start keeping python code formatting consistent across user tools.

I don't think I did any formatting with a specific tool. Rather, my IDE 
had flake8 set up to give me warnings if there are issues with 
formatting, and it also does formatting, so this would effectively be 
the same thing.

Judging by description of Ruff, it sounds like something I should try 
out, so thanks for the suggestion.

> 
>>  usertools/cpu_layout.py | 162 ++++++++++++++++++++++++++--------------
>>  1 file changed, 104 insertions(+), 58 deletions(-)
>>
>> diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
>> index 891b9238fa..843b29a134 100755
>> --- a/usertools/cpu_layout.py
>> +++ b/usertools/cpu_layout.py
>> @@ -3,62 +3,108 @@
>>  # Copyright(c) 2010-2014 Intel Corporation
>>  # Copyright(c) 2017 Cavium, Inc. All rights reserved.
>>
>> -output = " ".ljust(max_core_id_len + len('Core '))
>> -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)
>> +from typing import List, Set, Dict, Tuple
>> +
>> +
>> +def _range_expand(rstr: str) -> List[int]:
> 
> I don't see any reason to prefix the symbols with an underscore in this 
> script. It will not be used as an importable library.

In the general case I prefer not to make such assumptions, but in this 
particular case I think it's a safe assumption to make.

-- 
Thanks,
Anatoly

Re: [PATCH v1 1/2] usertools/cpu_layout: update coding style

[Stephen Hemminger]

On Mon, 19 Aug 2024 11:36:44 +0200
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 8/19/2024 11:26 AM, Robin Jarry wrote:
> > Anatoly Burakov, Aug 14, 2024 at 13:19:  
> >> Update coding style:
> >>
> >> - make it PEP-484 compliant
> >> - address all flake8, mypy etc. warnings
> >> - use f-strings in place of old-style string interpolation
> >> - refactor printing to make the code more readable
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> ---  
> > 
> > Hi Anatoly,
> > 
> > thanks for the patch. Did you format the code using black/ruff? I'd like 
> > to start keeping python code formatting consistent across user tools.  
> 
> I don't think I did any formatting with a specific tool. Rather, my IDE 
> had flake8 set up to give me warnings if there are issues with 
> formatting, and it also does formatting, so this would effectively be 
> the same thing.


I tend to use yapf3

[PATCH v3 1/4] usertools/cpu_layout: update coding style

[Anatoly Burakov]

Update coding style:

- make it PEP-484 compliant
- address all flake8, mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable
- read valid CPU ID's from "online" sysfs node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v1,v2 -> v3:
    - Import typing as T instead of individual types

 usertools/cpu_layout.py | 162 ++++++++++++++++++++++++++--------------
 1 file changed, 107 insertions(+), 55 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..1c255ff1a1 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,114 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-    try:
-        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-    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)
-    if socket not in sockets:
-        sockets.append(socket)
-    key = (socket, core)
-    if key not in core_map:
-        core_map[key] = []
-    core_map[key].append(cpu)
+"""Display CPU topology information."""
 
-print(format("=" * (47 + len(base_path))))
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path))))
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
+import typing as T
 
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count)  \
-                      + len(", ") * (max_thread_count - 1) \
-                      + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
 
-output = " ".ljust(max_core_id_len + len('Core '))
-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 '))
-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)
+def range_expand(rstr: str) -> T.List[int]:
+    """Expand a range string into a list of integers."""
+    # 0,1-3 => [0, 1-3]
+    ranges = rstr.split(",")
+    valset: T.List[int] = []
+    for r in ranges:
+        # 1-3 => [1, 2, 3]
+        if "-" in r:
+            start, end = r.split("-")
+            valset.extend(range(int(start), int(end) + 1))
         else:
-            output += " " * (max_core_map_len + 1)
-    print(output)
+            valset.append(int(r))
+    return valset
+
+
+def read_sysfs(path: str) -> str:
+    """Read a sysfs file and return its contents."""
+    with open(path, encoding="utf-8") as fd:
+        return fd.read().strip()
+
+
+def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
+    """Print a row of a table with the given column widths."""
+    first, *rest = row
+    w_first, *w_rest = col_widths
+    first_end = " " * 4
+    rest_end = " " * 10
+
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
+
+
+def print_section(heading: str) -> None:
+    """Print a section heading."""
+    sep = "=" * len(heading)
+    print(sep)
+    print(heading)
+    print(sep)
+    print()
+
+
+def main() -> None:
+    """Print CPU topology information."""
+    sockets_s: T.Set[int] = set()
+    cores_s: T.Set[int] = set()
+    core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    base_path = "/sys/devices/system/cpu"
+
+    cpus = range_expand(read_sysfs(f"{base_path}/online"))
+
+    for cpu in cpus:
+        lcore_base = f"{base_path}/cpu{cpu}"
+        core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
+        socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+        cores_s.add(core)
+        sockets_s.add(socket)
+        key = (socket, core)
+        core_map.setdefault(key, [])
+        core_map[key].append(cpu)
+
+    cores = sorted(cores_s)
+    sockets = sorted(sockets_s)
+
+    print_section("Core and Socket Information "
+                  f"(as reported by '{base_path}')")
+
+    print("cores = ", cores)
+    print("sockets = ", sockets)
+    print()
+
+    # Core, [Socket, Socket, ...]
+    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
+    rows: T.List[T.Tuple[str, ...]] = []
+
+    for c in cores:
+        # Core,
+        row: T.Tuple[str, ...] = (f"Core {c}",)
+
+        # [lcores, lcores, ...]
+        for s in sockets:
+            try:
+                lcores = core_map[(s, c)]
+                row += (str(lcores),)
+            except KeyError:
+                row += ("",)
+        rows += [row]
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [heading_strs])
+        for col_idx in range(len(heading_strs))
+    ]
+
+    # print out table taking row widths into account
+    print_row(heading_strs, col_widths)
+    print_row(sep_strs, col_widths)
+    for row in rows:
+        print_row(row, col_widths)
+
+
+if __name__ == "__main__":
+    main()
-- 
2.43.5

[PATCH v3 2/4] usertools/cpu_layout: print out NUMA nodes

[Anatoly Burakov]

In traditional NUMA case, NUMA nodes and physical sockets were used
interchangeably, but there are cases where there can be multiple NUMA
nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
cases of multiple sockets. Use sysfs to print out NUMA information.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2 -> v3:
    - Sort imports alphabetically

 usertools/cpu_layout.py | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 1c255ff1a1..78b119d729 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -5,6 +5,7 @@
 
 """Display CPU topology information."""
 
+import glob
 import typing as T
 
 
@@ -29,12 +30,21 @@ def read_sysfs(path: str) -> str:
         return fd.read().strip()
 
 
+def read_numa_node(base: str) -> int:
+    """Read the NUMA node of a CPU."""
+    node_glob = f"{base}/node*"
+    node_dirs = glob.glob(node_glob)
+    if not node_dirs:
+        return 0  # default to node 0
+    return int(node_dirs[0].split("node")[1])
+
+
 def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
     """Print a row of a table with the given column widths."""
     first, *rest = row
     w_first, *w_rest = col_widths
     first_end = " " * 4
-    rest_end = " " * 10
+    rest_end = " " * 4
 
     print(first.ljust(w_first), end=first_end)
     for cell, width in zip(rest, w_rest):
@@ -56,6 +66,7 @@ def main() -> None:
     sockets_s: T.Set[int] = set()
     cores_s: T.Set[int] = set()
     core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    numa_map: T.Dict[int, int] = {}
     base_path = "/sys/devices/system/cpu"
 
     cpus = range_expand(read_sysfs(f"{base_path}/online"))
@@ -64,12 +75,14 @@ def main() -> None:
         lcore_base = f"{base_path}/cpu{cpu}"
         core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
         socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+        node = read_numa_node(lcore_base)
 
         cores_s.add(core)
         sockets_s.add(socket)
         key = (socket, core)
         core_map.setdefault(key, [])
         core_map[key].append(cpu)
+        numa_map[cpu] = node
 
     cores = sorted(cores_s)
     sockets = sorted(sockets_s)
@@ -79,24 +92,37 @@ def main() -> None:
 
     print("cores = ", cores)
     print("sockets = ", sockets)
+    print("numa = ", sorted(set(numa_map.values())))
     print()
 
-    # Core, [Socket, Socket, ...]
-    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    # Core, [NUMA, Socket, NUMA, Socket, ...]
+    heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
     sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
     rows: T.List[T.Tuple[str, ...]] = []
 
+    prev_numa = None
     for c in cores:
         # Core,
         row: T.Tuple[str, ...] = (f"Core {c}",)
 
-        # [lcores, lcores, ...]
+        # assume NUMA changes symmetrically
+        first_lcore = core_map[(0, c)][0]
+        cur_numa = numa_map[first_lcore]
+        numa_changed = prev_numa != cur_numa
+        prev_numa = cur_numa
+
+        # [NUMA, lcores, NUMA, lcores, ...]
         for s in sockets:
             try:
                 lcores = core_map[(s, c)]
+                numa = numa_map[lcores[0]]
+                if numa_changed:
+                    row += (f"NUMA {numa}",)
+                else:
+                    row += ("",)
                 row += (str(lcores),)
             except KeyError:
-                row += ("",)
+                row += ("", "")
         rows += [row]
 
     # find max widths for each column, including header and rows
-- 
2.43.5

Re: [PATCH v3 2/4] usertools/cpu_layout: print out NUMA nodes

[Robin Jarry]

Anatoly Burakov, Aug 20, 2024 at 17:35:
> In traditional NUMA case, NUMA nodes and physical sockets were used
> interchangeably, but there are cases where there can be multiple NUMA
> nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
> cases of multiple sockets. Use sysfs to print out NUMA information.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v2 -> v3:
>     - Sort imports alphabetically

Looks good to me, can you format the code for that commit as well?

Re: [PATCH v3 2/4] usertools/cpu_layout: print out NUMA nodes

[Burakov, Anatoly]

On 8/20/2024 9:22 PM, Robin Jarry wrote:
> Anatoly Burakov, Aug 20, 2024 at 17:35:
>> In traditional NUMA case, NUMA nodes and physical sockets were used
>> interchangeably, but there are cases where there can be multiple NUMA
>> nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
>> cases of multiple sockets. Use sysfs to print out NUMA information.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>
>> Notes:
>>     v2 -> v3:
>>     - Sort imports alphabetically
> 
> Looks good to me, can you format the code for that commit as well?
> 


Hi,

(duplicating here for posterity)

My IDE is already set up to auto-format with Ruff since our last 
conversation, so this is already formatted. I ran ruff format command 
just in case but it produced no changes.

-- 
Thanks,
Anatoly

[PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Anatoly Burakov]

Update coding style:

- Make the code PEP-484 compliant
- Add more comments, improve readability, use f-strings everywhere
- Use quotes consistently
- Address all Python static analysis (e.g. mypy, pylint) warnings
- Improve error handling
- Refactor printing and sysfs/procfs access functions
- Sort output by NUMA node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v1 -> v2:
      - Added commit that sorted output by NUMA node
    v2 -> v3:
      - Rewrite of the script as suggested by reviewers

 usertools/dpdk-hugepages.py | 456 +++++++++++++++++++++---------------
 1 file changed, 273 insertions(+), 183 deletions(-)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index bf2575ba36..510822af60 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -1,167 +1,136 @@
 #! /usr/bin/env python3
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright (c) 2020 Microsoft Corporation
-"""Script to query and setup huge pages for DPDK applications."""
+
+'''Script to query and setup huge pages for DPDK applications.'''
 
 import argparse
-import glob
 import os
 import re
+import subprocess
 import sys
+import typing as T
 from math import log2
 
 # Standard binary prefix
-BINARY_PREFIX = "KMG"
+BINARY_PREFIX = 'KMG'
 
 # systemd mount point for huge pages
-HUGE_MOUNT = "/dev/hugepages"
+HUGE_MOUNT = '/dev/hugepages'
+# default directory for non-NUMA huge pages
+NO_NUMA_HUGE_DIR = '/sys/kernel/mm/hugepages'
+# default base directory for NUMA nodes
+NUMA_NODE_BASE_DIR = '/sys/devices/system/node'
+# procfs paths
+MEMINFO_PATH = '/proc/meminfo'
+MOUNTS_PATH = '/proc/mounts'
 
 
-def fmt_memsize(kb):
+class HugepageMount:
+    '''Mount operations for huge page filesystem.'''
+
+    def __init__(self, path: str, mounted: bool):
+        self.path = path
+        # current mount status
+        self.mounted = mounted
+
+    def mount(self, pagesize_kb: int,
+              user: T.Optional[str], group: T.Optional[str]) -> None:
+        '''Mount the huge TLB file system'''
+        if self.mounted:
+            return
+        cmd = ['mount', '-t', 'hugetlbfs']
+        cmd += ['-o', f'pagesize={pagesize_kb * 1024}']
+        if user is not None:
+            cmd += ['-o', f'uid={user}']
+        if group is not None:
+            cmd += ['-o', f'gid={group}']
+        cmd += ['nodev', self.path]
+
+        subprocess.run(cmd, check=True)
+        self.mounted = True
+
+    def unmount(self) -> None:
+        '''Unmount the huge TLB file system (if mounted)'''
+        if self.mounted:
+            subprocess.run(['umount', self.path], check=True)
+            self.mounted = False
+
+
+class HugepageRes:
+    '''Huge page reserve operations. Can be NUMA-node-specific.'''
+
+    def __init__(self, path: str, node: T.Optional[int] = None):
+        self.path = path
+        # if this is a per-NUMA node huge page dir, store the node number
+        self.node = node
+        self.valid_page_sizes = self._get_valid_page_sizes()
+
+    def _get_valid_page_sizes(self) -> T.List[int]:
+        '''Extract valid huge page sizes'''
+        return [get_memsize(d.split('-')[1])
+                for d in os.listdir(self.path)]
+
+    def _nr_pages_path(self, sz: int) -> str:
+        if sz not in self.valid_page_sizes:
+            raise ValueError(f'Invalid page size {sz}. '
+                             f'Valid sizes: {self.valid_page_sizes}')
+        return os.path.join(self.path, f'hugepages-{sz}kB', 'nr_hugepages')
+
+    def __getitem__(self, sz: int) -> int:
+        '''Get current number of reserved pages of specified size'''
+        with open(self._nr_pages_path(sz), encoding='utf-8') as f:
+            return int(f.read())
+
+    def __setitem__(self, sz: int, nr_pages: int) -> None:
+        '''Set number of reserved pages of specified size'''
+        with open(self._nr_pages_path(sz), 'w', encoding='utf-8') as f:
+            f.write(f'{nr_pages}\n')
+
+
+def fmt_memsize(kb: int) -> str:
     '''Format memory size in kB into conventional format'''
     logk = int(log2(kb) / 10)
     suffix = BINARY_PREFIX[logk]
     unit = 2**(logk * 10)
-    return '{}{}b'.format(int(kb / unit), suffix)
+    return f'{int(kb / unit)}{suffix}b'
 
 
-def get_memsize(arg):
+def get_memsize(arg: str) -> int:
     '''Convert memory size with suffix to kB'''
-    match = re.match(r'(\d+)([' + BINARY_PREFIX + r']?)$', arg.upper())
+    # arg may have a 'b' at the end
+    if arg[-1].lower() == 'b':
+        arg = arg[:-1]
+    match = re.match(rf'(\d+)([{BINARY_PREFIX}]?)$', arg.upper())
     if match is None:
-        sys.exit('{} is not a valid size'.format(arg))
+        raise ValueError(f'{arg} is not a valid size')
     num = float(match.group(1))
     suffix = match.group(2)
-    if suffix == "":
+    if not suffix:
         return int(num / 1024)
     idx = BINARY_PREFIX.find(suffix)
     return int(num * (2**(idx * 10)))
 
 
-def is_numa():
-    '''Test if NUMA is necessary on this system'''
-    return os.path.exists('/sys/devices/system/node')
+def is_numa() -> bool:
+    '''Check if NUMA is supported'''
+    return os.path.exists(NUMA_NODE_BASE_DIR)
 
 
-def get_valid_page_sizes(path):
-    '''Extract valid hugepage sizes'''
-    dir = os.path.dirname(path)
-    pg_sizes = (d.split("-")[1] for d in os.listdir(dir))
-    return " ".join(pg_sizes)
-
-
-def get_hugepages(path):
-    '''Read number of reserved pages'''
-    with open(path + '/nr_hugepages') as nr_hugepages:
-        return int(nr_hugepages.read())
-    return 0
-
-
-def set_hugepages(path, reqpages):
-    '''Write the number of reserved huge pages'''
-    filename = path + '/nr_hugepages'
-    try:
-        with open(filename, 'w') as nr_hugepages:
-            nr_hugepages.write('{}\n'.format(reqpages))
-    except PermissionError:
-        sys.exit('Permission denied: need to be root!')
-    except FileNotFoundError:
-        sys.exit("Invalid page size. Valid page sizes: {}".format(
-                 get_valid_page_sizes(path)))
-    gotpages = get_hugepages(path)
-    if gotpages != reqpages:
-        sys.exit('Unable to set pages ({} instead of {} in {}).'.format(
-                 gotpages, reqpages, filename))
-
-
-def show_numa_pages():
-    '''Show huge page reservations on Numa system'''
-    print('Node Pages Size Total')
-    for numa_path in glob.glob('/sys/devices/system/node/node*'):
-        node = numa_path[29:]  # slice after /sys/devices/system/node/node
-        path = numa_path + '/hugepages'
-        if not os.path.exists(path):
-            continue
-        for hdir in os.listdir(path):
-            pages = get_hugepages(path + '/' + hdir)
-            if pages > 0:
-                kb = int(hdir[10:-2])  # slice out of hugepages-NNNkB
-                print('{:<4} {:<5} {:<6} {}'.format(node, pages,
-                                                    fmt_memsize(kb),
-                                                    fmt_memsize(pages * kb)))
-
-
-def show_non_numa_pages():
-    '''Show huge page reservations on non Numa system'''
-    print('Pages Size Total')
-    path = '/sys/kernel/mm/hugepages'
-    for hdir in os.listdir(path):
-        pages = get_hugepages(path + '/' + hdir)
-        if pages > 0:
-            kb = int(hdir[10:-2])
-            print('{:<5} {:<6} {}'.format(pages, fmt_memsize(kb),
-                                          fmt_memsize(pages * kb)))
-
-
-def show_pages():
-    '''Show existing huge page settings'''
-    if is_numa():
-        show_numa_pages()
-    else:
-        show_non_numa_pages()
-
-
-def clear_pages():
-    '''Clear all existing huge page mappings'''
-    if is_numa():
-        dirs = glob.glob(
-            '/sys/devices/system/node/node*/hugepages/hugepages-*')
-    else:
-        dirs = glob.glob('/sys/kernel/mm/hugepages/hugepages-*')
-
-    for path in dirs:
-        set_hugepages(path, 0)
-
-
-def default_pagesize():
+def default_pagesize() -> int:
     '''Get default huge page size from /proc/meminfo'''
-    with open('/proc/meminfo') as meminfo:
+    key = 'Hugepagesize'
+    with open(MEMINFO_PATH, encoding='utf-8') as meminfo:
         for line in meminfo:
-            if line.startswith('Hugepagesize:'):
+            if line.startswith(f'{key}:'):
                 return int(line.split()[1])
-    return None
+    raise KeyError(f'"{key}" not found in {MEMINFO_PATH}')
 
 
-def set_numa_pages(pages, hugepgsz, node=None):
-    '''Set huge page reservation on Numa system'''
-    if node:
-        nodes = ['/sys/devices/system/node/node{}/hugepages'.format(node)]
-    else:
-        nodes = glob.glob('/sys/devices/system/node/node*/hugepages')
-
-    for node_path in nodes:
-        huge_path = '{}/hugepages-{}kB'.format(node_path, hugepgsz)
-        set_hugepages(huge_path, pages)
-
-
-def set_non_numa_pages(pages, hugepgsz):
-    '''Set huge page reservation on non Numa system'''
-    path = '/sys/kernel/mm/hugepages/hugepages-{}kB'.format(hugepgsz)
-    set_hugepages(path, pages)
-
-
-def reserve_pages(pages, hugepgsz, node=None):
-    '''Set the number of huge pages to be reserved'''
-    if node or is_numa():
-        set_numa_pages(pages, hugepgsz, node=node)
-    else:
-        set_non_numa_pages(pages, hugepgsz)
-
-
-def get_mountpoints():
-    '''Get list of where hugepage filesystem is mounted'''
-    mounted = []
-    with open('/proc/mounts') as mounts:
+def get_hugetlbfs_mountpoints() -> T.List[str]:
+    '''Get list of where huge page filesystem is mounted'''
+    mounted: T.List[str] = []
+    with open(MOUNTS_PATH, encoding='utf-8') as mounts:
         for line in mounts:
             fields = line.split()
             if fields[2] != 'hugetlbfs':
@@ -170,43 +139,144 @@ def get_mountpoints():
     return mounted
 
 
-def mount_huge(pagesize, mountpoint, user, group):
-    '''Mount the huge TLB file system'''
-    if mountpoint in get_mountpoints():
-        print(mountpoint, "already mounted")
-        return
-    cmd = "mount -t hugetlbfs"
-    if pagesize:
-        cmd += ' -o pagesize={}'.format(pagesize * 1024)
-    if user:
-        cmd += ' -o uid=' + user
-    if group:
-        cmd += ' -o gid=' + group
-    cmd += ' nodev ' + mountpoint
-    os.system(cmd)
+def print_row(cells: T.Tuple[str, ...], widths: T.List[int]) -> None:
+    '''Print a row of a table with the given column widths'''
+    first, *rest = cells
+    w_first, *w_rest = widths
+    first_end = ' ' * 2
+    rest_end = ' ' * 2
 
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
 
-def umount_huge(mountpoint):
-    '''Unmount the huge TLB file system (if mounted)'''
-    if mountpoint in get_mountpoints():
-        os.system("umount " + mountpoint)
 
+def print_hp_status(hp_res: T.List[HugepageRes]) -> None:
+    '''Display status of huge page reservations'''
+    numa = is_numa()
 
-def show_mount():
-    '''Show where huge page filesystem is mounted'''
-    mounted = get_mountpoints()
-    if mounted:
-        print("Hugepages mounted on", *mounted)
+    # print out huge page information in a table
+    rows: T.List[T.Tuple[str, ...]]
+    headers: T.Tuple[str, ...]
+    if numa:
+        headers = 'Node', 'Pages', 'Size', 'Total'
+        rows = [
+            (
+                str(hp.node),
+                str(nr_pages),
+                fmt_memsize(sz),
+                fmt_memsize(sz * nr_pages)
+            )
+            # iterate over each huge page sysfs node...
+            for hp in hp_res
+            # ...and each page size within that node...
+            for sz in hp.valid_page_sizes
+            # ...we need number of pages multiple times, so we read it here...
+            for nr_pages in [hp[sz]]
+            # ...include this row only if there are pages reserved
+            if nr_pages
+        ]
     else:
-        print("Hugepages not mounted")
+        headers = 'Pages', 'Size', 'Total'
+        # if NUMA is disabled, we know there's only one huge page dir
+        hp = hp_res[0]
+        rows = [
+            (
+                str(nr_pages),
+                fmt_memsize(sz),
+                fmt_memsize(sz * nr_pages)
+            )
+            # iterate over each page size within the huge page dir
+            for sz in hp.valid_page_sizes
+            # read number of pages for this size
+            for nr_pages in [hp[sz]]
+            # skip if no pages
+            if nr_pages
+        ]
+    if not rows:
+        print('No huge pages reserved')
+        return
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [headers])
+        for col_idx in range(len(headers))
+    ]
+
+    # print everything
+    print_row(headers, col_widths)
+    for r in rows:
+        print_row(r, col_widths)
+
+
+def print_mount_status() -> None:
+    '''Display status of huge page filesystem mounts'''
+    mounted = get_hugetlbfs_mountpoints()
+    if not mounted:
+        print('No huge page filesystems mounted')
+        return
+    print('Huge page filesystems mounted at:', *mounted, sep=' ')
+
+
+def scan_huge_dirs(node: T.Optional[int]) -> T.List[HugepageRes]:
+    '''Return a HugepageRes object for each huge page directory'''
+    # if NUMA is enabled, scan per-NUMA node huge pages
+    if is_numa():
+        # helper function to extract node number from directory name
+        def _get_node(path: str) -> T.Optional[int]:
+            m = re.match(r'node(\d+)', os.path.basename(path))
+            return int(m.group(1)) if m else None
+        # we want a sorted list of NUMA nodes
+        nodes = sorted(n
+                       # iterate over all directories in the base directory
+                       for d in os.listdir(NUMA_NODE_BASE_DIR)
+                       # extract the node number from the directory name
+                       for n in [_get_node(d)]
+                       # filter out None values (non-NUMA node directories)
+                       if n is not None)
+        return [
+            HugepageRes(f'{NUMA_NODE_BASE_DIR}/node{n}/hugepages', n)
+            for n in nodes
+            # if user requested a specific node, only include that one
+            if node is None or n == node
+        ]
+    # otherwise, use non-NUMA huge page directory
+    if node is not None:
+        raise ValueError('NUMA node requested but not supported')
+    return [HugepageRes(NO_NUMA_HUGE_DIR)]
+
+
+def try_reserve_huge_pages(hp_res: T.List[HugepageRes], mem_sz: str,
+                           pagesize_kb: int) -> None:
+    '''Reserve huge pages if possible'''
+    reserve_kb = get_memsize(mem_sz)
+
+    # is this a valid request?
+    if reserve_kb % pagesize_kb != 0:
+        fmt_res = fmt_memsize(reserve_kb)
+        fmt_sz = fmt_memsize(pagesize_kb)
+        raise ValueError(f'Huge reservation {fmt_res} is '
+                         f'not a multiple of page size {fmt_sz}')
+
+    # request is valid, reserve pages
+    for hp in hp_res:
+        req = reserve_kb // pagesize_kb
+        hp[pagesize_kb] = req
+        got = hp[pagesize_kb]
+        # did we fulfill our request?
+        if got != req:
+            raise OSError(f'Failed to reserve {req} pages of size '
+                          f'{fmt_memsize(pagesize_kb)}, '
+                          f'got {got} pages instead')
 
 
 def main():
     '''Process the command line arguments and setup huge pages'''
     parser = argparse.ArgumentParser(
         formatter_class=argparse.RawDescriptionHelpFormatter,
-        description="Setup huge pages",
-        epilog="""
+        description='Setup huge pages',
+        epilog='''
 Examples:
 
 To display current huge page settings:
@@ -214,94 +284,114 @@ def main():
 
 To a complete setup of with 2 Gigabyte of 1G huge pages:
     %(prog)s -p 1G --setup 2G
-""")
+''')
     parser.add_argument(
         '--show',
         '-s',
         action='store_true',
-        help="print the current huge page configuration")
+        help='Print current huge page configuration')
     parser.add_argument(
-        '--clear', '-c', action='store_true', help="clear existing huge pages")
+        '--clear', '-c', action='store_true', help='Clear existing huge pages')
     parser.add_argument(
         '--mount',
         '-m',
         action='store_true',
-        help='mount the huge page filesystem')
+        help='Mount the huge page filesystem')
     parser.add_argument(
         '--unmount',
         '-u',
         action='store_true',
-        help='unmount the system huge page directory')
+        help='Unmount the system huge page directory')
     parser.add_argument(
         '--directory',
         '-d',
         metavar='DIR',
         default=HUGE_MOUNT,
-        help='mount point')
+        help='Mount point for huge pages')
     parser.add_argument(
         '--user',
         '-U',
         metavar='UID',
-        help='set the mounted directory owner user')
+        help='Set the mounted directory owner user')
     parser.add_argument(
         '--group',
         '-G',
         metavar='GID',
-        help='set the mounted directory owner group')
+        help='Set the mounted directory owner group')
     parser.add_argument(
-        '--node', '-n', help='select numa node to reserve pages on')
+        '--node', '-n', type=int, help='Select numa node to reserve pages on')
     parser.add_argument(
         '--pagesize',
         '-p',
         metavar='SIZE',
-        help='choose huge page size to use')
+        help='Choose huge page size to use')
     parser.add_argument(
         '--reserve',
         '-r',
         metavar='SIZE',
-        help='reserve huge pages. Size is in bytes with K, M, or G suffix')
+        help='Reserve huge pages. Size is in bytes with K, M, or G suffix')
     parser.add_argument(
         '--setup',
         metavar='SIZE',
-        help='setup huge pages by doing clear, unmount, reserve and mount')
+        help='Setup huge pages by doing clear, unmount, reserve and mount')
     args = parser.parse_args()
 
+    # setup is clear, then unmount, then reserve, then mount
     if args.setup:
         args.clear = True
         args.unmount = True
         args.reserve = args.setup
         args.mount = True
 
-    if not (args.show or args.mount or args.unmount or args.clear or args.reserve):
-        parser.error("no action specified")
+    if not (args.show or args.mount or args.unmount or
+            args.clear or args.reserve):
+        parser.error('no action specified')
 
+    # read huge page data from sysfs
+    hp_res = scan_huge_dirs(args.node)
+
+    # read huge page mountpoint data
+    hp_mountpoint = args.directory
+    hp_mounted = hp_mountpoint in get_hugetlbfs_mountpoints()
+    hp_mount = HugepageMount(hp_mountpoint, hp_mounted)
+
+    # get requested page size we will be working with
     if args.pagesize:
         pagesize_kb = get_memsize(args.pagesize)
     else:
         pagesize_kb = default_pagesize()
-    if not pagesize_kb:
-        sys.exit("Invalid page size: {}kB".format(pagesize_kb))
 
+    # were we asked to clear?
     if args.clear:
-        clear_pages()
+        for hp in hp_res:
+            for sz in hp.valid_page_sizes:
+                hp[sz] = 0
+
+    # were we asked to unmount?
     if args.unmount:
-        umount_huge(args.directory)
+        hp_mount.unmount()
 
+    # were we asked to reserve pages?
     if args.reserve:
-        reserve_kb = get_memsize(args.reserve)
-        if reserve_kb % pagesize_kb != 0:
-            sys.exit(
-                'Huge reservation {}kB is not a multiple of page size {}kB'.
-                format(reserve_kb, pagesize_kb))
-        reserve_pages(
-            int(reserve_kb / pagesize_kb), pagesize_kb, node=args.node)
+        try_reserve_huge_pages(hp_res, args.reserve, pagesize_kb)
+
+    # were we asked to mount?
     if args.mount:
-        mount_huge(pagesize_kb, args.directory, args.user, args.group)
+        hp_mount.mount(pagesize_kb, args.user, args.group)
+
+    # were we asked to display status?
     if args.show:
-        show_pages()
+        print_hp_status(hp_res)
         print()
-        show_mount()
+        print_mount_status()
 
 
-if __name__ == "__main__":
-    main()
+if __name__ == '__main__':
+    try:
+        main()
+    except PermissionError:
+        sys.exit('Permission denied: need to be root!')
+    except subprocess.CalledProcessError as e:
+        sys.exit(f'Command failed: {e}')
+    except (KeyError, ValueError, OSError) as e:
+        sys.exit(f'Error: {e}')
-- 
2.43.5

Re: [PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Stephen Hemminger]

On Tue, 20 Aug 2024 16:35:16 +0100
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> Update coding style:
> 
> - Make the code PEP-484 compliant
> - Add more comments, improve readability, use f-strings everywhere
> - Use quotes consistently
> - Address all Python static analysis (e.g. mypy, pylint) warnings
> - Improve error handling
> - Refactor printing and sysfs/procfs access functions
> - Sort output by NUMA node
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Looks good, but not sure if always using single quote is really necessary.
Many python programs seem to use either one indiscriminately.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Burakov, Anatoly]

On 8/20/2024 5:52 PM, Stephen Hemminger wrote:
> On Tue, 20 Aug 2024 16:35:16 +0100
> Anatoly Burakov <anatoly.burakov@intel.com> wrote:
> 
>> Update coding style:
>>
>> - Make the code PEP-484 compliant
>> - Add more comments, improve readability, use f-strings everywhere
>> - Use quotes consistently
>> - Address all Python static analysis (e.g. mypy, pylint) warnings
>> - Improve error handling
>> - Refactor printing and sysfs/procfs access functions
>> - Sort output by NUMA node
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Looks good, but not sure if always using single quote is really necessary.
> Many python programs seem to use either one indiscriminately.

Well, that doesn't mean it's a good example to follow :) At least it 
annoys me visually so I went ahead and fixed it since I was doing a 
rewrite anyway.

> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

-- 
Thanks,
Anatoly

Re: [PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Robin Jarry]

Anatoly Burakov, Aug 20, 2024 at 17:35:
> Update coding style:
>
> - Make the code PEP-484 compliant
> - Add more comments, improve readability, use f-strings everywhere
> - Use quotes consistently
> - Address all Python static analysis (e.g. mypy, pylint) warnings
> - Improve error handling
> - Refactor printing and sysfs/procfs access functions
> - Sort output by NUMA node
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v1 -> v2:
>       - Added commit that sorted output by NUMA node
>     v2 -> v3:
>       - Rewrite of the script as suggested by reviewers

Instead of debating about coding style. I'd like to enforce black/ruff 
for new scripts and/or rewrites.

The code looks good to me, but could you pass through one of these tools 
and send a v4?

    black usertools/dpdk-hugepages.py

or

    ruff format usertools/dpdk-hugepages.py

I think they output the exact same code formatting but I could be wrong.

Re: [PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Burakov, Anatoly]

On 8/20/2024 5:57 PM, Robin Jarry wrote:
> Anatoly Burakov, Aug 20, 2024 at 17:35:
>> Update coding style:
>>
>> - Make the code PEP-484 compliant
>> - Add more comments, improve readability, use f-strings everywhere
>> - Use quotes consistently
>> - Address all Python static analysis (e.g. mypy, pylint) warnings
>> - Improve error handling
>> - Refactor printing and sysfs/procfs access functions
>> - Sort output by NUMA node
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>
>> Notes:
>>     v1 -> v2:
>>       - Added commit that sorted output by NUMA node
>>     v2 -> v3:
>>       - Rewrite of the script as suggested by reviewers
> 
> Instead of debating about coding style. I'd like to enforce black/ruff 
> for new scripts and/or rewrites.
> 
> The code looks good to me, but could you pass through one of these tools 
> and send a v4?
> 
>     black usertools/dpdk-hugepages.py
> 
> or
> 
>     ruff format usertools/dpdk-hugepages.py
> 
> I think they output the exact same code formatting but I could be wrong.
> 

Hi,

My IDE is already set up to auto-format with Ruff since our last 
conversation, so this is already formatted. I ran ruff format command 
just in case but it produced no changes.

So, no v4 necessary unless you think there are any changes to be made 
about the code :)

-- 
Thanks,
Anatoly

Re: [PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Burakov, Anatoly]

On 8/21/2024 10:52 AM, Burakov, Anatoly wrote:
> On 8/20/2024 5:57 PM, Robin Jarry wrote:
>> Anatoly Burakov, Aug 20, 2024 at 17:35:
>>> Update coding style:
>>>
>>> - Make the code PEP-484 compliant
>>> - Add more comments, improve readability, use f-strings everywhere
>>> - Use quotes consistently
>>> - Address all Python static analysis (e.g. mypy, pylint) warnings
>>> - Improve error handling
>>> - Refactor printing and sysfs/procfs access functions
>>> - Sort output by NUMA node
>>>
>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>> ---
>>>
>>> Notes:
>>>     v1 -> v2:
>>>       - Added commit that sorted output by NUMA node
>>>     v2 -> v3:
>>>       - Rewrite of the script as suggested by reviewers
>>
>> Instead of debating about coding style. I'd like to enforce black/ruff 
>> for new scripts and/or rewrites.
>>
>> The code looks good to me, but could you pass through one of these 
>> tools and send a v4?
>>
>>     black usertools/dpdk-hugepages.py
>>
>> or
>>
>>     ruff format usertools/dpdk-hugepages.py
>>
>> I think they output the exact same code formatting but I could be wrong.
>>
> 
> Hi,
> 
> My IDE is already set up to auto-format with Ruff since our last 
> conversation, so this is already formatted. I ran ruff format command 
> just in case but it produced no changes.
> 
> So, no v4 necessary unless you think there are any changes to be made 
> about the code :)
> 

Actually, I take that back - I had a configuration mishap and didn't 
notice that I wasn't using Ruff for formatting on the machine I was 
creating the commits.

Still, cpu_layout's formatting is not affected, but hugepage script is.

However, after formatting with ruff, I can see that 1) most single 
quotes became double quotes, 2) some lines I broke up for readability, 
are no longer broken up, and 3) some lines I broke up to avoid exceeding 
the 80 symbols count, are no longer broken up.

I'll see if using Black yields different results.
-- 
Thanks,
Anatoly

Re: [PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Burakov, Anatoly]

On 8/21/2024 11:06 AM, Burakov, Anatoly wrote:
> On 8/21/2024 10:52 AM, Burakov, Anatoly wrote:
>> On 8/20/2024 5:57 PM, Robin Jarry wrote:
>>> Anatoly Burakov, Aug 20, 2024 at 17:35:
>>>> Update coding style:
>>>>
>>>> - Make the code PEP-484 compliant
>>>> - Add more comments, improve readability, use f-strings everywhere
>>>> - Use quotes consistently
>>>> - Address all Python static analysis (e.g. mypy, pylint) warnings
>>>> - Improve error handling
>>>> - Refactor printing and sysfs/procfs access functions
>>>> - Sort output by NUMA node
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>>
>>>> Notes:
>>>>     v1 -> v2:
>>>>       - Added commit that sorted output by NUMA node
>>>>     v2 -> v3:
>>>>       - Rewrite of the script as suggested by reviewers
>>>
>>> Instead of debating about coding style. I'd like to enforce 
>>> black/ruff for new scripts and/or rewrites.
>>>
>>> The code looks good to me, but could you pass through one of these 
>>> tools and send a v4?
>>>
>>>     black usertools/dpdk-hugepages.py
>>>
>>> or
>>>
>>>     ruff format usertools/dpdk-hugepages.py
>>>
>>> I think they output the exact same code formatting but I could be wrong.
>>>
>>
>> Hi,
>>
>> My IDE is already set up to auto-format with Ruff since our last 
>> conversation, so this is already formatted. I ran ruff format command 
>> just in case but it produced no changes.
>>
>> So, no v4 necessary unless you think there are any changes to be made 
>> about the code :)
>>
> 
> Actually, I take that back - I had a configuration mishap and didn't 
> notice that I wasn't using Ruff for formatting on the machine I was 
> creating the commits.
> 
> Still, cpu_layout's formatting is not affected, but hugepage script is.
> 
> However, after formatting with ruff, I can see that 1) most single 
> quotes became double quotes, 2) some lines I broke up for readability, 
> are no longer broken up, and 3) some lines I broke up to avoid exceeding 
> the 80 symbols count, are no longer broken up.
> 
> I'll see if using Black yields different results.

Regarding line length, it seems that it's configurable. Perhaps we could 
include a Ruff/Black configuration file with DPDK to solve this problem 
once and for all? Adding --line-length=79 to ruff config addresses the 
last issue, but it wouldn't be necessary if there was a Ruff 
configuration file in the repo. I can live with first two things that I 
highlighted.
-- 
Thanks,
Anatoly

Re: [PATCH v3 3/4] usertools/dpdk-hugepages.py: update coding style

[Robin Jarry]

Burakov, Anatoly, Aug 21, 2024 at 11:16:
> > Actually, I take that back - I had a configuration mishap and didn't 
> > notice that I wasn't using Ruff for formatting on the machine I was 
> > creating the commits.
> > 
> > Still, cpu_layout's formatting is not affected, but hugepage script is.
> > 
> > However, after formatting with ruff, I can see that 1) most single 
> > quotes became double quotes, 2) some lines I broke up for readability, 
> > are no longer broken up, and 3) some lines I broke up to avoid exceeding 
> > the 80 symbols count, are no longer broken up.

Using these tools allow developers to stop thinking about coding style 
and focus on more important matters :)

> > I'll see if using Black yields different results.
>
> Regarding line length, it seems that it's configurable. Perhaps we could 
> include a Ruff/Black configuration file with DPDK to solve this problem 
> once and for all? Adding --line-length=79 to ruff config addresses the 
> last issue, but it wouldn't be necessary if there was a Ruff 
> configuration file in the repo. I can live with first two things that I 
> highlighted.

Both black and ruff have the same formatting rules and use a default 88 
line length limit. Which yields good results in most cases:

https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

I would prefer if we kept the default settings without any 
customization. The DPDK code base is already allowed to go up to 100 
columns anyways.

[PATCH v3 4/4] usertools/dpdk-devbind: print NUMA node

[Anatoly Burakov]

Currently, devbind does not print out any NUMA information, which makes
figuring out which NUMA node device belongs to not trivial. Add printouts
for NUMA information if NUMA support is enabled on the system.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Robin Jarry <rjarry@redhat.com>
---

Notes:
    v1 -> v2:
    - Added commit to print out NUMA information in devbind

 usertools/dpdk-devbind.py | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index b276e8efc8..078e8c387b 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -110,6 +110,11 @@
 args = []
 
 
+# check if this system has NUMA support
+def is_numa():
+    return os.path.exists('/sys/devices/system/node')
+
+
 # check if a specific kernel module is loaded
 def module_is_loaded(module):
     global loaded_modules
@@ -577,20 +582,28 @@ def show_device_status(devices_type, device_name, if_field=False):
         print("".join('=' * len(msg)))
         return
 
+    print_numa = is_numa()
+
     # print each category separately, so we can clearly see what's used by DPDK
     if dpdk_drv:
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
         display_devices("%s devices using DPDK-compatible driver" % device_name,
-                        dpdk_drv, "drv=%(Driver_str)s unused=%(Module_str)s")
+                        dpdk_drv, extra_param)
     if kernel_drv:
-        if_text = ""
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
         if if_field:
-            if_text = "if=%(Interface)s "
-        display_devices("%s devices using kernel driver" % device_name, kernel_drv,
-                        if_text + "drv=%(Driver_str)s "
-                        "unused=%(Module_str)s %(Active)s")
+            extra_param = "if=%(Interface)s " + extra_param
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("%s devices using kernel driver" % device_name,
+                        kernel_drv, extra_param)
     if no_drv:
-        display_devices("Other %s devices" % device_name, no_drv,
-                        "unused=%(Module_str)s")
+        extra_param = "unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("Other %s devices" % device_name, no_drv, extra_param)
 
 
 def show_status():
-- 
2.43.5

Re: [PATCH v3 1/4] usertools/cpu_layout: update coding style

[Robin Jarry]

Anatoly Burakov, Aug 20, 2024 at 17:35:
> Update coding style:
>
> - make it PEP-484 compliant
> - address all flake8, mypy etc. warnings
> - use f-strings in place of old-style string interpolation
> - refactor printing to make the code more readable
> - read valid CPU ID's from "online" sysfs node
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v1,v2 -> v3:
>     - Import typing as T instead of individual types

Looks good to me. Same remark than dpdk-hugepages.py: could you format 
it using black or ruff?

Thanks!

Re: [PATCH v3 1/4] usertools/cpu_layout: update coding style

[Burakov, Anatoly]

On 8/20/2024 5:59 PM, Robin Jarry wrote:
> Anatoly Burakov, Aug 20, 2024 at 17:35:
>> Update coding style:
>>
>> - make it PEP-484 compliant
>> - address all flake8, mypy etc. warnings
>> - use f-strings in place of old-style string interpolation
>> - refactor printing to make the code more readable
>> - read valid CPU ID's from "online" sysfs node
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>
>> Notes:
>>     v1,v2 -> v3:
>>     - Import typing as T instead of individual types
> 
> Looks good to me. Same remark than dpdk-hugepages.py: could you format 
> it using black or ruff?
> 
> Thanks!
> 

Hi,

My IDE is already set up to auto-format with Ruff since our last 
conversation, so this is already formatted. I ran ruff format command 
just in case but it produced no changes.

-- 
Thanks,
Anatoly

[PATCH v4 1/4] usertools/cpu_layout: update coding style

[Anatoly Burakov]

Update coding style:

- make it PEP-484 compliant
- address all flake8, mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable
- read valid CPU ID's from "online" sysfs node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v3->v4:
    - Format with Ruff, line width 79
    
    v1,v2 -> v3:
    - Import typing as T instead of individual types

 usertools/cpu_layout.py | 163 ++++++++++++++++++++++++++--------------
 1 file changed, 108 insertions(+), 55 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..8812ea286b 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,115 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-    try:
-        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-    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)
-    if socket not in sockets:
-        sockets.append(socket)
-    key = (socket, core)
-    if key not in core_map:
-        core_map[key] = []
-    core_map[key].append(cpu)
+"""Display CPU topology information."""
 
-print(format("=" * (47 + len(base_path))))
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path))))
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
+import typing as T
 
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count)  \
-                      + len(", ") * (max_thread_count - 1) \
-                      + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
 
-output = " ".ljust(max_core_id_len + len('Core '))
-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 '))
-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)
+def range_expand(rstr: str) -> T.List[int]:
+    """Expand a range string into a list of integers."""
+    # 0,1-3 => [0, 1-3]
+    ranges = rstr.split(",")
+    valset: T.List[int] = []
+    for r in ranges:
+        # 1-3 => [1, 2, 3]
+        if "-" in r:
+            start, end = r.split("-")
+            valset.extend(range(int(start), int(end) + 1))
         else:
-            output += " " * (max_core_map_len + 1)
-    print(output)
+            valset.append(int(r))
+    return valset
+
+
+def read_sysfs(path: str) -> str:
+    """Read a sysfs file and return its contents."""
+    with open(path, encoding="utf-8") as fd:
+        return fd.read().strip()
+
+
+def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
+    """Print a row of a table with the given column widths."""
+    first, *rest = row
+    w_first, *w_rest = col_widths
+    first_end = " " * 4
+    rest_end = " " * 10
+
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
+
+
+def print_section(heading: str) -> None:
+    """Print a section heading."""
+    sep = "=" * len(heading)
+    print(sep)
+    print(heading)
+    print(sep)
+    print()
+
+
+def main() -> None:
+    """Print CPU topology information."""
+    sockets_s: T.Set[int] = set()
+    cores_s: T.Set[int] = set()
+    core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    base_path = "/sys/devices/system/cpu"
+
+    cpus = range_expand(read_sysfs(f"{base_path}/online"))
+
+    for cpu in cpus:
+        lcore_base = f"{base_path}/cpu{cpu}"
+        core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
+        socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+        cores_s.add(core)
+        sockets_s.add(socket)
+        key = (socket, core)
+        core_map.setdefault(key, [])
+        core_map[key].append(cpu)
+
+    cores = sorted(cores_s)
+    sockets = sorted(sockets_s)
+
+    print_section(
+        f"Core and Socket Information (as reported by '{base_path}')"
+    )
+
+    print("cores = ", cores)
+    print("sockets = ", sockets)
+    print()
+
+    # Core, [Socket, Socket, ...]
+    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
+    rows: T.List[T.Tuple[str, ...]] = []
+
+    for c in cores:
+        # Core,
+        row: T.Tuple[str, ...] = (f"Core {c}",)
+
+        # [lcores, lcores, ...]
+        for s in sockets:
+            try:
+                lcores = core_map[(s, c)]
+                row += (str(lcores),)
+            except KeyError:
+                row += ("",)
+        rows += [row]
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [heading_strs])
+        for col_idx in range(len(heading_strs))
+    ]
+
+    # print out table taking row widths into account
+    print_row(heading_strs, col_widths)
+    print_row(sep_strs, col_widths)
+    for row in rows:
+        print_row(row, col_widths)
+
+
+if __name__ == "__main__":
+    main()
-- 
2.43.5

[PATCH v4 2/4] usertools/cpu_layout: print out NUMA nodes

[Anatoly Burakov]

In traditional NUMA case, NUMA nodes and physical sockets were used
interchangeably, but there are cases where there can be multiple NUMA
nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
cases of multiple sockets. Use sysfs to print out NUMA information.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2 -> v3:
    - Sort imports alphabetically

 usertools/cpu_layout.py | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 8812ea286b..e4720e27db 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -5,6 +5,7 @@
 
 """Display CPU topology information."""
 
+import glob
 import typing as T
 
 
@@ -29,12 +30,21 @@ def read_sysfs(path: str) -> str:
         return fd.read().strip()
 
 
+def read_numa_node(base: str) -> int:
+    """Read the NUMA node of a CPU."""
+    node_glob = f"{base}/node*"
+    node_dirs = glob.glob(node_glob)
+    if not node_dirs:
+        return 0  # default to node 0
+    return int(node_dirs[0].split("node")[1])
+
+
 def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
     """Print a row of a table with the given column widths."""
     first, *rest = row
     w_first, *w_rest = col_widths
     first_end = " " * 4
-    rest_end = " " * 10
+    rest_end = " " * 4
 
     print(first.ljust(w_first), end=first_end)
     for cell, width in zip(rest, w_rest):
@@ -56,6 +66,7 @@ def main() -> None:
     sockets_s: T.Set[int] = set()
     cores_s: T.Set[int] = set()
     core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    numa_map: T.Dict[int, int] = {}
     base_path = "/sys/devices/system/cpu"
 
     cpus = range_expand(read_sysfs(f"{base_path}/online"))
@@ -64,12 +75,14 @@ def main() -> None:
         lcore_base = f"{base_path}/cpu{cpu}"
         core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
         socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+        node = read_numa_node(lcore_base)
 
         cores_s.add(core)
         sockets_s.add(socket)
         key = (socket, core)
         core_map.setdefault(key, [])
         core_map[key].append(cpu)
+        numa_map[cpu] = node
 
     cores = sorted(cores_s)
     sockets = sorted(sockets_s)
@@ -80,24 +93,37 @@ def main() -> None:
 
     print("cores = ", cores)
     print("sockets = ", sockets)
+    print("numa = ", sorted(set(numa_map.values())))
     print()
 
-    # Core, [Socket, Socket, ...]
-    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    # Core, [NUMA, Socket, NUMA, Socket, ...]
+    heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
     sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
     rows: T.List[T.Tuple[str, ...]] = []
 
+    prev_numa = None
     for c in cores:
         # Core,
         row: T.Tuple[str, ...] = (f"Core {c}",)
 
-        # [lcores, lcores, ...]
+        # assume NUMA changes symmetrically
+        first_lcore = core_map[(0, c)][0]
+        cur_numa = numa_map[first_lcore]
+        numa_changed = prev_numa != cur_numa
+        prev_numa = cur_numa
+
+        # [NUMA, lcores, NUMA, lcores, ...]
         for s in sockets:
             try:
                 lcores = core_map[(s, c)]
+                numa = numa_map[lcores[0]]
+                if numa_changed:
+                    row += (f"NUMA {numa}",)
+                else:
+                    row += ("",)
                 row += (str(lcores),)
             except KeyError:
-                row += ("",)
+                row += ("", "")
         rows += [row]
 
     # find max widths for each column, including header and rows
-- 
2.43.5

[PATCH v4 3/4] usertools/dpdk-hugepages.py: update coding style

[Anatoly Burakov]

Update coding style:

- Make the code PEP-484 compliant
- Add more comments, improve readability, use f-strings everywhere
- Address all Python static analysis (e.g. mypy, pylint) warnings
- Format code with Ruff
- Improve error handling
- Refactor printing and sysfs/procfs access functions
- Sort output by NUMA node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---

Notes:
    v3 -> v4:
      - Format code with Ruff, line width 79 to avoid flake8 warnings
        (Flake8 is by default configured with line width 79 on my system)
    v2 -> v3:
      - Rewrite of the script as suggested by reviewers
    v1 -> v2:
      - Added commit that sorted output by NUMA node

 usertools/dpdk-hugepages.py | 524 ++++++++++++++++++++++--------------
 1 file changed, 315 insertions(+), 209 deletions(-)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index bf2575ba36..4c99682848 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -1,13 +1,15 @@
 #! /usr/bin/env python3
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright (c) 2020 Microsoft Corporation
+
 """Script to query and setup huge pages for DPDK applications."""
 
 import argparse
-import glob
 import os
 import re
+import subprocess
 import sys
+import typing as T
 from math import log2
 
 # Standard binary prefix
@@ -15,194 +17,268 @@
 
 # systemd mount point for huge pages
 HUGE_MOUNT = "/dev/hugepages"
+# default directory for non-NUMA huge pages
+NO_NUMA_HUGE_DIR = "/sys/kernel/mm/hugepages"
+# default base directory for NUMA nodes
+NUMA_NODE_BASE_DIR = "/sys/devices/system/node"
+# procfs paths
+MEMINFO_PATH = "/proc/meminfo"
+MOUNTS_PATH = "/proc/mounts"
 
 
-def fmt_memsize(kb):
-    '''Format memory size in kB into conventional format'''
+class HugepageMount:
+    """Mount operations for huge page filesystem."""
+
+    def __init__(self, path: str, mounted: bool):
+        self.path = path
+        # current mount status
+        self.mounted = mounted
+
+    def mount(
+        self, pagesize_kb: int, user: T.Optional[str], group: T.Optional[str]
+    ) -> None:
+        """Mount the huge TLB file system"""
+        if self.mounted:
+            return
+        cmd = ["mount", "-t", "hugetlbfs"]
+        cmd += ["-o", f"pagesize={pagesize_kb * 1024}"]
+        if user is not None:
+            cmd += ["-o", f"uid={user}"]
+        if group is not None:
+            cmd += ["-o", f"gid={group}"]
+        cmd += ["nodev", self.path]
+
+        subprocess.run(cmd, check=True)
+        self.mounted = True
+
+    def unmount(self) -> None:
+        """Unmount the huge TLB file system (if mounted)"""
+        if self.mounted:
+            subprocess.run(["umount", self.path], check=True)
+            self.mounted = False
+
+
+class HugepageRes:
+    """Huge page reserve operations. Can be NUMA-node-specific."""
+
+    def __init__(self, path: str, node: T.Optional[int] = None):
+        self.path = path
+        # if this is a per-NUMA node huge page dir, store the node number
+        self.node = node
+        self.valid_page_sizes = self._get_valid_page_sizes()
+
+    def _get_valid_page_sizes(self) -> T.List[int]:
+        """Extract valid huge page sizes"""
+        return [get_memsize(d.split("-")[1]) for d in os.listdir(self.path)]
+
+    def _nr_pages_path(self, sz: int) -> str:
+        if sz not in self.valid_page_sizes:
+            raise ValueError(
+                f"Invalid page size {sz}. "
+                f"Valid sizes: {self.valid_page_sizes}"
+            )
+        return os.path.join(self.path, f"hugepages-{sz}kB", "nr_hugepages")
+
+    def __getitem__(self, sz: int) -> int:
+        """Get current number of reserved pages of specified size"""
+        with open(self._nr_pages_path(sz), encoding="utf-8") as f:
+            return int(f.read())
+
+    def __setitem__(self, sz: int, nr_pages: int) -> None:
+        """Set number of reserved pages of specified size"""
+        with open(self._nr_pages_path(sz), "w", encoding="utf-8") as f:
+            f.write(f"{nr_pages}\n")
+
+
+def fmt_memsize(kb: int) -> str:
+    """Format memory size in kB into conventional format"""
     logk = int(log2(kb) / 10)
     suffix = BINARY_PREFIX[logk]
-    unit = 2**(logk * 10)
-    return '{}{}b'.format(int(kb / unit), suffix)
+    unit = 2 ** (logk * 10)
+    return f"{int(kb / unit)}{suffix}b"
 
 
-def get_memsize(arg):
-    '''Convert memory size with suffix to kB'''
-    match = re.match(r'(\d+)([' + BINARY_PREFIX + r']?)$', arg.upper())
+def get_memsize(arg: str) -> int:
+    """Convert memory size with suffix to kB"""
+    # arg may have a 'b' at the end
+    if arg[-1].lower() == "b":
+        arg = arg[:-1]
+    match = re.match(rf"(\d+)([{BINARY_PREFIX}]?)$", arg.upper())
     if match is None:
-        sys.exit('{} is not a valid size'.format(arg))
+        raise ValueError(f"{arg} is not a valid size")
     num = float(match.group(1))
     suffix = match.group(2)
-    if suffix == "":
+    if not suffix:
         return int(num / 1024)
     idx = BINARY_PREFIX.find(suffix)
-    return int(num * (2**(idx * 10)))
+    return int(num * (2 ** (idx * 10)))
 
 
-def is_numa():
-    '''Test if NUMA is necessary on this system'''
-    return os.path.exists('/sys/devices/system/node')
+def is_numa() -> bool:
+    """Check if NUMA is supported"""
+    return os.path.exists(NUMA_NODE_BASE_DIR)
 
 
-def get_valid_page_sizes(path):
-    '''Extract valid hugepage sizes'''
-    dir = os.path.dirname(path)
-    pg_sizes = (d.split("-")[1] for d in os.listdir(dir))
-    return " ".join(pg_sizes)
-
-
-def get_hugepages(path):
-    '''Read number of reserved pages'''
-    with open(path + '/nr_hugepages') as nr_hugepages:
-        return int(nr_hugepages.read())
-    return 0
-
-
-def set_hugepages(path, reqpages):
-    '''Write the number of reserved huge pages'''
-    filename = path + '/nr_hugepages'
-    try:
-        with open(filename, 'w') as nr_hugepages:
-            nr_hugepages.write('{}\n'.format(reqpages))
-    except PermissionError:
-        sys.exit('Permission denied: need to be root!')
-    except FileNotFoundError:
-        sys.exit("Invalid page size. Valid page sizes: {}".format(
-                 get_valid_page_sizes(path)))
-    gotpages = get_hugepages(path)
-    if gotpages != reqpages:
-        sys.exit('Unable to set pages ({} instead of {} in {}).'.format(
-                 gotpages, reqpages, filename))
-
-
-def show_numa_pages():
-    '''Show huge page reservations on Numa system'''
-    print('Node Pages Size Total')
-    for numa_path in glob.glob('/sys/devices/system/node/node*'):
-        node = numa_path[29:]  # slice after /sys/devices/system/node/node
-        path = numa_path + '/hugepages'
-        if not os.path.exists(path):
-            continue
-        for hdir in os.listdir(path):
-            pages = get_hugepages(path + '/' + hdir)
-            if pages > 0:
-                kb = int(hdir[10:-2])  # slice out of hugepages-NNNkB
-                print('{:<4} {:<5} {:<6} {}'.format(node, pages,
-                                                    fmt_memsize(kb),
-                                                    fmt_memsize(pages * kb)))
-
-
-def show_non_numa_pages():
-    '''Show huge page reservations on non Numa system'''
-    print('Pages Size Total')
-    path = '/sys/kernel/mm/hugepages'
-    for hdir in os.listdir(path):
-        pages = get_hugepages(path + '/' + hdir)
-        if pages > 0:
-            kb = int(hdir[10:-2])
-            print('{:<5} {:<6} {}'.format(pages, fmt_memsize(kb),
-                                          fmt_memsize(pages * kb)))
-
-
-def show_pages():
-    '''Show existing huge page settings'''
-    if is_numa():
-        show_numa_pages()
-    else:
-        show_non_numa_pages()
-
-
-def clear_pages():
-    '''Clear all existing huge page mappings'''
-    if is_numa():
-        dirs = glob.glob(
-            '/sys/devices/system/node/node*/hugepages/hugepages-*')
-    else:
-        dirs = glob.glob('/sys/kernel/mm/hugepages/hugepages-*')
-
-    for path in dirs:
-        set_hugepages(path, 0)
-
-
-def default_pagesize():
-    '''Get default huge page size from /proc/meminfo'''
-    with open('/proc/meminfo') as meminfo:
+def default_pagesize() -> int:
+    """Get default huge page size from /proc/meminfo"""
+    key = "Hugepagesize"
+    with open(MEMINFO_PATH, encoding="utf-8") as meminfo:
         for line in meminfo:
-            if line.startswith('Hugepagesize:'):
+            if line.startswith(f"{key}:"):
                 return int(line.split()[1])
-    return None
+    raise KeyError(f'"{key}" not found in {MEMINFO_PATH}')
 
 
-def set_numa_pages(pages, hugepgsz, node=None):
-    '''Set huge page reservation on Numa system'''
-    if node:
-        nodes = ['/sys/devices/system/node/node{}/hugepages'.format(node)]
-    else:
-        nodes = glob.glob('/sys/devices/system/node/node*/hugepages')
-
-    for node_path in nodes:
-        huge_path = '{}/hugepages-{}kB'.format(node_path, hugepgsz)
-        set_hugepages(huge_path, pages)
-
-
-def set_non_numa_pages(pages, hugepgsz):
-    '''Set huge page reservation on non Numa system'''
-    path = '/sys/kernel/mm/hugepages/hugepages-{}kB'.format(hugepgsz)
-    set_hugepages(path, pages)
-
-
-def reserve_pages(pages, hugepgsz, node=None):
-    '''Set the number of huge pages to be reserved'''
-    if node or is_numa():
-        set_numa_pages(pages, hugepgsz, node=node)
-    else:
-        set_non_numa_pages(pages, hugepgsz)
-
-
-def get_mountpoints():
-    '''Get list of where hugepage filesystem is mounted'''
-    mounted = []
-    with open('/proc/mounts') as mounts:
+def get_hugetlbfs_mountpoints() -> T.List[str]:
+    """Get list of where huge page filesystem is mounted"""
+    mounted: T.List[str] = []
+    with open(MOUNTS_PATH, encoding="utf-8") as mounts:
         for line in mounts:
             fields = line.split()
-            if fields[2] != 'hugetlbfs':
+            if fields[2] != "hugetlbfs":
                 continue
             mounted.append(fields[1])
     return mounted
 
 
-def mount_huge(pagesize, mountpoint, user, group):
-    '''Mount the huge TLB file system'''
-    if mountpoint in get_mountpoints():
-        print(mountpoint, "already mounted")
-        return
-    cmd = "mount -t hugetlbfs"
-    if pagesize:
-        cmd += ' -o pagesize={}'.format(pagesize * 1024)
-    if user:
-        cmd += ' -o uid=' + user
-    if group:
-        cmd += ' -o gid=' + group
-    cmd += ' nodev ' + mountpoint
-    os.system(cmd)
+def print_row(cells: T.Tuple[str, ...], widths: T.List[int]) -> None:
+    """Print a row of a table with the given column widths"""
+    first, *rest = cells
+    w_first, *w_rest = widths
+    first_end = " " * 2
+    rest_end = " " * 2
 
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
 
-def umount_huge(mountpoint):
-    '''Unmount the huge TLB file system (if mounted)'''
-    if mountpoint in get_mountpoints():
-        os.system("umount " + mountpoint)
 
+def print_hp_status(hp_res: T.List[HugepageRes]) -> None:
+    """Display status of huge page reservations"""
+    numa = is_numa()
 
-def show_mount():
-    '''Show where huge page filesystem is mounted'''
-    mounted = get_mountpoints()
-    if mounted:
-        print("Hugepages mounted on", *mounted)
+    # print out huge page information in a table
+    rows: T.List[T.Tuple[str, ...]]
+    headers: T.Tuple[str, ...]
+    if numa:
+        headers = "Node", "Pages", "Size", "Total"
+        rows = [
+            (
+                str(hp.node),
+                str(nr_pages),
+                fmt_memsize(sz),
+                fmt_memsize(sz * nr_pages),
+            )
+            # iterate over each huge page sysfs node...
+            for hp in hp_res
+            # ...and each page size within that node...
+            for sz in hp.valid_page_sizes
+            # ...we need number of pages multiple times, so we read it here...
+            for nr_pages in [hp[sz]]
+            # ...include this row only if there are pages reserved
+            if nr_pages
+        ]
     else:
-        print("Hugepages not mounted")
+        headers = "Pages", "Size", "Total"
+        # if NUMA is disabled, we know there's only one huge page dir
+        hp = hp_res[0]
+        rows = [
+            (str(nr_pages), fmt_memsize(sz), fmt_memsize(sz * nr_pages))
+            # iterate over each page size within the huge page dir
+            for sz in hp.valid_page_sizes
+            # read number of pages for this size
+            for nr_pages in [hp[sz]]
+            # skip if no pages
+            if nr_pages
+        ]
+    if not rows:
+        print("No huge pages reserved")
+        return
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [headers])
+        for col_idx in range(len(headers))
+    ]
+
+    # print everything
+    print_row(headers, col_widths)
+    for r in rows:
+        print_row(r, col_widths)
+
+
+def print_mount_status() -> None:
+    """Display status of huge page filesystem mounts"""
+    mounted = get_hugetlbfs_mountpoints()
+    if not mounted:
+        print("No huge page filesystems mounted")
+        return
+    print("Huge page filesystems mounted at:", *mounted, sep=" ")
+
+
+def scan_huge_dirs(node: T.Optional[int]) -> T.List[HugepageRes]:
+    """Return a HugepageRes object for each huge page directory"""
+    # if NUMA is enabled, scan per-NUMA node huge pages
+    if is_numa():
+        # helper function to extract node number from directory name
+        def _get_node(path: str) -> T.Optional[int]:
+            m = re.match(r"node(\d+)", os.path.basename(path))
+            return int(m.group(1)) if m else None
+
+        # we want a sorted list of NUMA nodes
+        nodes = sorted(
+            n
+            # iterate over all directories in the base directory
+            for d in os.listdir(NUMA_NODE_BASE_DIR)
+            # extract the node number from the directory name
+            for n in [_get_node(d)]
+            # filter out None values (non-NUMA node directories)
+            if n is not None
+        )
+        return [
+            HugepageRes(f"{NUMA_NODE_BASE_DIR}/node{n}/hugepages", n)
+            for n in nodes
+            # if user requested a specific node, only include that one
+            if node is None or n == node
+        ]
+    # otherwise, use non-NUMA huge page directory
+    if node is not None:
+        raise ValueError("NUMA node requested but not supported")
+    return [HugepageRes(NO_NUMA_HUGE_DIR)]
+
+
+def try_reserve_huge_pages(
+    hp_res: T.List[HugepageRes], mem_sz: str, pagesize_kb: int
+) -> None:
+    """Reserve huge pages if possible"""
+    reserve_kb = get_memsize(mem_sz)
+
+    # is this a valid request?
+    if reserve_kb % pagesize_kb != 0:
+        fmt_res = fmt_memsize(reserve_kb)
+        fmt_sz = fmt_memsize(pagesize_kb)
+        raise ValueError(
+            f"Huge reservation {fmt_res} is "
+            f"not a multiple of page size {fmt_sz}"
+        )
+
+    # request is valid, reserve pages
+    for hp in hp_res:
+        req = reserve_kb // pagesize_kb
+        hp[pagesize_kb] = req
+        got = hp[pagesize_kb]
+        # did we fulfill our request?
+        if got != req:
+            raise OSError(
+                f"Failed to reserve {req} pages of size "
+                f"{fmt_memsize(pagesize_kb)}, "
+                f"got {got} pages instead"
+            )
 
 
 def main():
-    '''Process the command line arguments and setup huge pages'''
+    """Process the command line arguments and setup huge pages"""
     parser = argparse.ArgumentParser(
         formatter_class=argparse.RawDescriptionHelpFormatter,
         description="Setup huge pages",
@@ -214,94 +290,124 @@ def main():
 
 To a complete setup of with 2 Gigabyte of 1G huge pages:
     %(prog)s -p 1G --setup 2G
-""")
+""",
+    )
     parser.add_argument(
-        '--show',
-        '-s',
-        action='store_true',
-        help="print the current huge page configuration")
+        "--show",
+        "-s",
+        action="store_true",
+        help="Print current huge page configuration",
+    )
     parser.add_argument(
-        '--clear', '-c', action='store_true', help="clear existing huge pages")
+        "--clear", "-c", action="store_true", help="Clear existing huge pages"
+    )
     parser.add_argument(
-        '--mount',
-        '-m',
-        action='store_true',
-        help='mount the huge page filesystem')
+        "--mount",
+        "-m",
+        action="store_true",
+        help="Mount the huge page filesystem",
+    )
     parser.add_argument(
-        '--unmount',
-        '-u',
-        action='store_true',
-        help='unmount the system huge page directory')
+        "--unmount",
+        "-u",
+        action="store_true",
+        help="Unmount the system huge page directory",
+    )
     parser.add_argument(
-        '--directory',
-        '-d',
-        metavar='DIR',
+        "--directory",
+        "-d",
+        metavar="DIR",
         default=HUGE_MOUNT,
-        help='mount point')
+        help="Mount point for huge pages",
+    )
     parser.add_argument(
-        '--user',
-        '-U',
-        metavar='UID',
-        help='set the mounted directory owner user')
+        "--user",
+        "-U",
+        metavar="UID",
+        help="Set the mounted directory owner user",
+    )
     parser.add_argument(
-        '--group',
-        '-G',
-        metavar='GID',
-        help='set the mounted directory owner group')
+        "--group",
+        "-G",
+        metavar="GID",
+        help="Set the mounted directory owner group",
+    )
     parser.add_argument(
-        '--node', '-n', help='select numa node to reserve pages on')
+        "--node", "-n", type=int, help="Select numa node to reserve pages on"
+    )
     parser.add_argument(
-        '--pagesize',
-        '-p',
-        metavar='SIZE',
-        help='choose huge page size to use')
+        "--pagesize", "-p", metavar="SIZE", help="Choose huge page size to use"
+    )
     parser.add_argument(
-        '--reserve',
-        '-r',
-        metavar='SIZE',
-        help='reserve huge pages. Size is in bytes with K, M, or G suffix')
+        "--reserve",
+        "-r",
+        metavar="SIZE",
+        help="Reserve huge pages. Size is in bytes with K, M, or G suffix",
+    )
     parser.add_argument(
-        '--setup',
-        metavar='SIZE',
-        help='setup huge pages by doing clear, unmount, reserve and mount')
+        "--setup",
+        metavar="SIZE",
+        help="Setup huge pages by doing clear, unmount, reserve and mount",
+    )
     args = parser.parse_args()
 
+    # setup is clear, then unmount, then reserve, then mount
     if args.setup:
         args.clear = True
         args.unmount = True
         args.reserve = args.setup
         args.mount = True
 
-    if not (args.show or args.mount or args.unmount or args.clear or args.reserve):
+    if not (
+        args.show or args.mount or args.unmount or args.clear or args.reserve
+    ):
         parser.error("no action specified")
 
+    # read huge page data from sysfs
+    hp_res = scan_huge_dirs(args.node)
+
+    # read huge page mountpoint data
+    hp_mountpoint = args.directory
+    hp_mounted = hp_mountpoint in get_hugetlbfs_mountpoints()
+    hp_mount = HugepageMount(hp_mountpoint, hp_mounted)
+
+    # get requested page size we will be working with
     if args.pagesize:
         pagesize_kb = get_memsize(args.pagesize)
     else:
         pagesize_kb = default_pagesize()
-    if not pagesize_kb:
-        sys.exit("Invalid page size: {}kB".format(pagesize_kb))
 
+    # were we asked to clear?
     if args.clear:
-        clear_pages()
+        for hp in hp_res:
+            for sz in hp.valid_page_sizes:
+                hp[sz] = 0
+
+    # were we asked to unmount?
     if args.unmount:
-        umount_huge(args.directory)
+        hp_mount.unmount()
 
+    # were we asked to reserve pages?
     if args.reserve:
-        reserve_kb = get_memsize(args.reserve)
-        if reserve_kb % pagesize_kb != 0:
-            sys.exit(
-                'Huge reservation {}kB is not a multiple of page size {}kB'.
-                format(reserve_kb, pagesize_kb))
-        reserve_pages(
-            int(reserve_kb / pagesize_kb), pagesize_kb, node=args.node)
+        try_reserve_huge_pages(hp_res, args.reserve, pagesize_kb)
+
+    # were we asked to mount?
     if args.mount:
-        mount_huge(pagesize_kb, args.directory, args.user, args.group)
+        hp_mount.mount(pagesize_kb, args.user, args.group)
+
+    # were we asked to display status?
     if args.show:
-        show_pages()
+        print_hp_status(hp_res)
         print()
-        show_mount()
+        print_mount_status()
 
 
 if __name__ == "__main__":
-    main()
+    try:
+        main()
+    except PermissionError:
+        sys.exit("Permission denied: need to be root!")
+    except subprocess.CalledProcessError as e:
+        sys.exit(f"Command failed: {e}")
+    except (KeyError, ValueError, OSError) as e:
+        sys.exit(f"Error: {e}")
-- 
2.43.5

Re: [PATCH v4 3/4] usertools/dpdk-hugepages.py: update coding style

[Robin Jarry]

Anatoly Burakov, Aug 21, 2024 at 11:22:
> Update coding style:
>
> - Make the code PEP-484 compliant
> - Add more comments, improve readability, use f-strings everywhere
> - Address all Python static analysis (e.g. mypy, pylint) warnings
> - Format code with Ruff
> - Improve error handling
> - Refactor printing and sysfs/procfs access functions
> - Sort output by NUMA node
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>
> Notes:
>     v3 -> v4:
>       - Format code with Ruff, line width 79 to avoid flake8 warnings
>         (Flake8 is by default configured with line width 79 on my system)

Please keep the default ruff/black settings. And when formatting with 
these tools, flake8 is mostly useless.

If you want to check your code for defects, you are probably best with 
`ruff check` which combines the features of multiple python linters and 
runs much faster.


>     v2 -> v3:
>       - Rewrite of the script as suggested by reviewers
>     v1 -> v2:
>       - Added commit that sorted output by NUMA node

Re: [PATCH v4 3/4] usertools/dpdk-hugepages.py: update coding style

[Burakov, Anatoly]

On 8/21/2024 11:26 AM, Robin Jarry wrote:
> Anatoly Burakov, Aug 21, 2024 at 11:22:
>> Update coding style:
>>
>> - Make the code PEP-484 compliant
>> - Add more comments, improve readability, use f-strings everywhere
>> - Address all Python static analysis (e.g. mypy, pylint) warnings
>> - Format code with Ruff
>> - Improve error handling
>> - Refactor printing and sysfs/procfs access functions
>> - Sort output by NUMA node
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>> ---
>>
>> Notes:
>>     v3 -> v4:
>>       - Format code with Ruff, line width 79 to avoid flake8 warnings
>>         (Flake8 is by default configured with line width 79 on my system)
> 
> Please keep the default ruff/black settings. And when formatting with 
> these tools, flake8 is mostly useless.
> 
> If you want to check your code for defects, you are probably best with 
> `ruff check` which combines the features of multiple python linters and 
> runs much faster.

OK, I'll reformat with default settings then! v5 incoming

> 
> 
>>     v2 -> v3:
>>       - Rewrite of the script as suggested by reviewers
>>     v1 -> v2:
>>       - Added commit that sorted output by NUMA node
> 

-- 
Thanks,
Anatoly

[PATCH v4 4/4] usertools/dpdk-devbind: print NUMA node

[Anatoly Burakov]

Currently, devbind does not print out any NUMA information, which makes
figuring out which NUMA node device belongs to not trivial. Add printouts
for NUMA information if NUMA support is enabled on the system.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Robin Jarry <rjarry@redhat.com>
---

Notes:
    v1 -> v2:
    - Added commit to print out NUMA information in devbind

 usertools/dpdk-devbind.py | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index b276e8efc8..078e8c387b 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -110,6 +110,11 @@
 args = []
 
 
+# check if this system has NUMA support
+def is_numa():
+    return os.path.exists('/sys/devices/system/node')
+
+
 # check if a specific kernel module is loaded
 def module_is_loaded(module):
     global loaded_modules
@@ -577,20 +582,28 @@ def show_device_status(devices_type, device_name, if_field=False):
         print("".join('=' * len(msg)))
         return
 
+    print_numa = is_numa()
+
     # print each category separately, so we can clearly see what's used by DPDK
     if dpdk_drv:
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
         display_devices("%s devices using DPDK-compatible driver" % device_name,
-                        dpdk_drv, "drv=%(Driver_str)s unused=%(Module_str)s")
+                        dpdk_drv, extra_param)
     if kernel_drv:
-        if_text = ""
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
         if if_field:
-            if_text = "if=%(Interface)s "
-        display_devices("%s devices using kernel driver" % device_name, kernel_drv,
-                        if_text + "drv=%(Driver_str)s "
-                        "unused=%(Module_str)s %(Active)s")
+            extra_param = "if=%(Interface)s " + extra_param
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("%s devices using kernel driver" % device_name,
+                        kernel_drv, extra_param)
     if no_drv:
-        display_devices("Other %s devices" % device_name, no_drv,
-                        "unused=%(Module_str)s")
+        extra_param = "unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("Other %s devices" % device_name, no_drv, extra_param)
 
 
 def show_status():
-- 
2.43.5

[PATCH v5 1/4] usertools/cpu_layout: update coding style

[Anatoly Burakov]

Update coding style:

- make it PEP-484 compliant
- format code with Ruff
- address all mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable
- read valid CPU ID's from "online" sysfs node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v4-v5:
    - Format with Ruff on default settings
    
    v3->v4:
    - Format with Ruff, line width 79
    
    v1,v2 -> v3:
    - Import typing as T instead of individual types

 usertools/cpu_layout.py | 161 ++++++++++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 55 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..e133fb8ad3 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,113 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-    try:
-        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-    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)
-    if socket not in sockets:
-        sockets.append(socket)
-    key = (socket, core)
-    if key not in core_map:
-        core_map[key] = []
-    core_map[key].append(cpu)
+"""Display CPU topology information."""
 
-print(format("=" * (47 + len(base_path))))
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path))))
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
+import typing as T
 
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count)  \
-                      + len(", ") * (max_thread_count - 1) \
-                      + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
 
-output = " ".ljust(max_core_id_len + len('Core '))
-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 '))
-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)
+def range_expand(rstr: str) -> T.List[int]:
+    """Expand a range string into a list of integers."""
+    # 0,1-3 => [0, 1-3]
+    ranges = rstr.split(",")
+    valset: T.List[int] = []
+    for r in ranges:
+        # 1-3 => [1, 2, 3]
+        if "-" in r:
+            start, end = r.split("-")
+            valset.extend(range(int(start), int(end) + 1))
         else:
-            output += " " * (max_core_map_len + 1)
-    print(output)
+            valset.append(int(r))
+    return valset
+
+
+def read_sysfs(path: str) -> str:
+    """Read a sysfs file and return its contents."""
+    with open(path, encoding="utf-8") as fd:
+        return fd.read().strip()
+
+
+def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
+    """Print a row of a table with the given column widths."""
+    first, *rest = row
+    w_first, *w_rest = col_widths
+    first_end = " " * 4
+    rest_end = " " * 10
+
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
+
+
+def print_section(heading: str) -> None:
+    """Print a section heading."""
+    sep = "=" * len(heading)
+    print(sep)
+    print(heading)
+    print(sep)
+    print()
+
+
+def main() -> None:
+    """Print CPU topology information."""
+    sockets_s: T.Set[int] = set()
+    cores_s: T.Set[int] = set()
+    core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    base_path = "/sys/devices/system/cpu"
+
+    cpus = range_expand(read_sysfs(f"{base_path}/online"))
+
+    for cpu in cpus:
+        lcore_base = f"{base_path}/cpu{cpu}"
+        core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
+        socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+        cores_s.add(core)
+        sockets_s.add(socket)
+        key = (socket, core)
+        core_map.setdefault(key, [])
+        core_map[key].append(cpu)
+
+    cores = sorted(cores_s)
+    sockets = sorted(sockets_s)
+
+    print_section(f"Core and Socket Information (as reported by '{base_path}')")
+
+    print("cores = ", cores)
+    print("sockets = ", sockets)
+    print()
+
+    # Core, [Socket, Socket, ...]
+    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
+    rows: T.List[T.Tuple[str, ...]] = []
+
+    for c in cores:
+        # Core,
+        row: T.Tuple[str, ...] = (f"Core {c}",)
+
+        # [lcores, lcores, ...]
+        for s in sockets:
+            try:
+                lcores = core_map[(s, c)]
+                row += (str(lcores),)
+            except KeyError:
+                row += ("",)
+        rows += [row]
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [heading_strs])
+        for col_idx in range(len(heading_strs))
+    ]
+
+    # print out table taking row widths into account
+    print_row(heading_strs, col_widths)
+    print_row(sep_strs, col_widths)
+    for row in rows:
+        print_row(row, col_widths)
+
+
+if __name__ == "__main__":
+    main()
-- 
2.43.5

[PATCH v5 2/4] usertools/cpu_layout: print out NUMA nodes

[Anatoly Burakov]

In traditional NUMA case, NUMA nodes and physical sockets were used
interchangeably, but there are cases where there can be multiple NUMA
nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
cases of multiple sockets. Use sysfs to print out NUMA information.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2 -> v3:
    - Sort imports alphabetically

 usertools/cpu_layout.py | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index e133fb8ad3..976be1f8b2 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -5,6 +5,7 @@
 
 """Display CPU topology information."""
 
+import glob
 import typing as T
 
 
@@ -29,12 +30,21 @@ def read_sysfs(path: str) -> str:
         return fd.read().strip()
 
 
+def read_numa_node(base: str) -> int:
+    """Read the NUMA node of a CPU."""
+    node_glob = f"{base}/node*"
+    node_dirs = glob.glob(node_glob)
+    if not node_dirs:
+        return 0  # default to node 0
+    return int(node_dirs[0].split("node")[1])
+
+
 def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
     """Print a row of a table with the given column widths."""
     first, *rest = row
     w_first, *w_rest = col_widths
     first_end = " " * 4
-    rest_end = " " * 10
+    rest_end = " " * 4
 
     print(first.ljust(w_first), end=first_end)
     for cell, width in zip(rest, w_rest):
@@ -56,6 +66,7 @@ def main() -> None:
     sockets_s: T.Set[int] = set()
     cores_s: T.Set[int] = set()
     core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    numa_map: T.Dict[int, int] = {}
     base_path = "/sys/devices/system/cpu"
 
     cpus = range_expand(read_sysfs(f"{base_path}/online"))
@@ -64,12 +75,14 @@ def main() -> None:
         lcore_base = f"{base_path}/cpu{cpu}"
         core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
         socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+        node = read_numa_node(lcore_base)
 
         cores_s.add(core)
         sockets_s.add(socket)
         key = (socket, core)
         core_map.setdefault(key, [])
         core_map[key].append(cpu)
+        numa_map[cpu] = node
 
     cores = sorted(cores_s)
     sockets = sorted(sockets_s)
@@ -78,24 +91,37 @@ def main() -> None:
 
     print("cores = ", cores)
     print("sockets = ", sockets)
+    print("numa = ", sorted(set(numa_map.values())))
     print()
 
-    # Core, [Socket, Socket, ...]
-    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    # Core, [NUMA, Socket, NUMA, Socket, ...]
+    heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
     sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
     rows: T.List[T.Tuple[str, ...]] = []
 
+    prev_numa = None
     for c in cores:
         # Core,
         row: T.Tuple[str, ...] = (f"Core {c}",)
 
-        # [lcores, lcores, ...]
+        # assume NUMA changes symmetrically
+        first_lcore = core_map[(0, c)][0]
+        cur_numa = numa_map[first_lcore]
+        numa_changed = prev_numa != cur_numa
+        prev_numa = cur_numa
+
+        # [NUMA, lcores, NUMA, lcores, ...]
         for s in sockets:
             try:
                 lcores = core_map[(s, c)]
+                numa = numa_map[lcores[0]]
+                if numa_changed:
+                    row += (f"NUMA {numa}",)
+                else:
+                    row += ("",)
                 row += (str(lcores),)
             except KeyError:
-                row += ("",)
+                row += ("", "")
         rows += [row]
 
     # find max widths for each column, including header and rows
-- 
2.43.5

Re: [PATCH v5 2/4] usertools/cpu_layout: print out NUMA nodes

[Robin Jarry]

Anatoly Burakov, Aug 21, 2024 at 11:44:
> In traditional NUMA case, NUMA nodes and physical sockets were used
> interchangeably, but there are cases where there can be multiple NUMA
> nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
> cases of multiple sockets. Use sysfs to print out NUMA information.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v2 -> v3:
>     - Sort imports alphabetically
>
>  usertools/cpu_layout.py | 36 +++++++++++++++++++++++++++++++-----
>  1 file changed, 31 insertions(+), 5 deletions(-)

Acked-by: Robin Jarry <rjarry@redhat.com>

Thanks!

[PATCH v5 3/4] usertools/dpdk-hugepages.py: update coding style

[Anatoly Burakov]

Update coding style:

- make the code PEP-484 compliant
- add more comments, improve readability, use f-strings everywhere
- address all Python static analysis (e.g. mypy, pylint) warnings
- format code with Ruff
- improve error handling
- refactor printing and sysfs/procfs access functions
- sort huge page reservation status output by NUMA node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---

Notes:
    v4 -> v5:
    - Format with Ruff on default settings
    - Replaced all instances of raw path strings with os.path.join
    v3 -> v4:
    - Format code with Ruff, line width 79 to avoid flake8 warnings
      (Flake8 is by default configured with line width 79 on my system)
    v2 -> v3:
    - Rewrite of the script as suggested by reviewers
    v1 -> v2:
    - Added commit that sorted output by NUMA node

 usertools/dpdk-hugepages.py | 518 +++++++++++++++++++++---------------
 1 file changed, 310 insertions(+), 208 deletions(-)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index bf2575ba36..3fc3269c83 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -1,13 +1,15 @@
 #! /usr/bin/env python3
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright (c) 2020 Microsoft Corporation
+
 """Script to query and setup huge pages for DPDK applications."""
 
 import argparse
-import glob
 import os
 import re
+import subprocess
 import sys
+import typing as T
 from math import log2
 
 # Standard binary prefix
@@ -15,194 +17,266 @@
 
 # systemd mount point for huge pages
 HUGE_MOUNT = "/dev/hugepages"
+# default directory for non-NUMA huge pages
+NO_NUMA_HUGE_DIR = "/sys/kernel/mm/hugepages"
+# default base directory for NUMA nodes
+NUMA_NODE_BASE_DIR = "/sys/devices/system/node"
+# procfs paths
+MEMINFO_PATH = "/proc/meminfo"
+MOUNTS_PATH = "/proc/mounts"
 
 
-def fmt_memsize(kb):
-    '''Format memory size in kB into conventional format'''
+class HugepageMount:
+    """Mount operations for huge page filesystem."""
+
+    def __init__(self, path: str, mounted: bool):
+        self.path = path
+        # current mount status
+        self.mounted = mounted
+
+    def mount(
+        self, pagesize_kb: int, user: T.Optional[str], group: T.Optional[str]
+    ) -> None:
+        """Mount the huge TLB file system"""
+        if self.mounted:
+            return
+        cmd = ["mount", "-t", "hugetlbfs"]
+        cmd += ["-o", f"pagesize={pagesize_kb * 1024}"]
+        if user is not None:
+            cmd += ["-o", f"uid={user}"]
+        if group is not None:
+            cmd += ["-o", f"gid={group}"]
+        cmd += ["nodev", self.path]
+
+        subprocess.run(cmd, check=True)
+        self.mounted = True
+
+    def unmount(self) -> None:
+        """Unmount the huge TLB file system (if mounted)"""
+        if self.mounted:
+            subprocess.run(["umount", self.path], check=True)
+            self.mounted = False
+
+
+class HugepageRes:
+    """Huge page reserve operations. Can be NUMA-node-specific."""
+
+    def __init__(self, path: str, node: T.Optional[int] = None):
+        self.path = path
+        # if this is a per-NUMA node huge page dir, store the node number
+        self.node = node
+        self.valid_page_sizes = self._get_valid_page_sizes()
+
+    def _get_valid_page_sizes(self) -> T.List[int]:
+        """Extract valid huge page sizes"""
+        return [get_memsize(d.split("-")[1]) for d in os.listdir(self.path)]
+
+    def _nr_pages_path(self, sz: int) -> str:
+        if sz not in self.valid_page_sizes:
+            raise ValueError(
+                f"Invalid page size {sz}. " f"Valid sizes: {self.valid_page_sizes}"
+            )
+        return os.path.join(self.path, f"hugepages-{sz}kB", "nr_hugepages")
+
+    def __getitem__(self, sz: int) -> int:
+        """Get current number of reserved pages of specified size"""
+        with open(self._nr_pages_path(sz), encoding="utf-8") as f:
+            return int(f.read())
+
+    def __setitem__(self, sz: int, nr_pages: int) -> None:
+        """Set number of reserved pages of specified size"""
+        with open(self._nr_pages_path(sz), "w", encoding="utf-8") as f:
+            f.write(f"{nr_pages}\n")
+
+
+def fmt_memsize(kb: int) -> str:
+    """Format memory size in kB into conventional format"""
     logk = int(log2(kb) / 10)
     suffix = BINARY_PREFIX[logk]
-    unit = 2**(logk * 10)
-    return '{}{}b'.format(int(kb / unit), suffix)
+    unit = 2 ** (logk * 10)
+    return f"{int(kb / unit)}{suffix}b"
 
 
-def get_memsize(arg):
-    '''Convert memory size with suffix to kB'''
-    match = re.match(r'(\d+)([' + BINARY_PREFIX + r']?)$', arg.upper())
+def get_memsize(arg: str) -> int:
+    """Convert memory size with suffix to kB"""
+    # arg may have a 'b' at the end
+    if arg[-1].lower() == "b":
+        arg = arg[:-1]
+    match = re.match(rf"(\d+)([{BINARY_PREFIX}]?)$", arg.upper())
     if match is None:
-        sys.exit('{} is not a valid size'.format(arg))
+        raise ValueError(f"{arg} is not a valid size")
     num = float(match.group(1))
     suffix = match.group(2)
-    if suffix == "":
+    if not suffix:
         return int(num / 1024)
     idx = BINARY_PREFIX.find(suffix)
-    return int(num * (2**(idx * 10)))
+    return int(num * (2 ** (idx * 10)))
 
 
-def is_numa():
-    '''Test if NUMA is necessary on this system'''
-    return os.path.exists('/sys/devices/system/node')
+def is_numa() -> bool:
+    """Check if NUMA is supported"""
+    return os.path.exists(NUMA_NODE_BASE_DIR)
 
 
-def get_valid_page_sizes(path):
-    '''Extract valid hugepage sizes'''
-    dir = os.path.dirname(path)
-    pg_sizes = (d.split("-")[1] for d in os.listdir(dir))
-    return " ".join(pg_sizes)
-
-
-def get_hugepages(path):
-    '''Read number of reserved pages'''
-    with open(path + '/nr_hugepages') as nr_hugepages:
-        return int(nr_hugepages.read())
-    return 0
-
-
-def set_hugepages(path, reqpages):
-    '''Write the number of reserved huge pages'''
-    filename = path + '/nr_hugepages'
-    try:
-        with open(filename, 'w') as nr_hugepages:
-            nr_hugepages.write('{}\n'.format(reqpages))
-    except PermissionError:
-        sys.exit('Permission denied: need to be root!')
-    except FileNotFoundError:
-        sys.exit("Invalid page size. Valid page sizes: {}".format(
-                 get_valid_page_sizes(path)))
-    gotpages = get_hugepages(path)
-    if gotpages != reqpages:
-        sys.exit('Unable to set pages ({} instead of {} in {}).'.format(
-                 gotpages, reqpages, filename))
-
-
-def show_numa_pages():
-    '''Show huge page reservations on Numa system'''
-    print('Node Pages Size Total')
-    for numa_path in glob.glob('/sys/devices/system/node/node*'):
-        node = numa_path[29:]  # slice after /sys/devices/system/node/node
-        path = numa_path + '/hugepages'
-        if not os.path.exists(path):
-            continue
-        for hdir in os.listdir(path):
-            pages = get_hugepages(path + '/' + hdir)
-            if pages > 0:
-                kb = int(hdir[10:-2])  # slice out of hugepages-NNNkB
-                print('{:<4} {:<5} {:<6} {}'.format(node, pages,
-                                                    fmt_memsize(kb),
-                                                    fmt_memsize(pages * kb)))
-
-
-def show_non_numa_pages():
-    '''Show huge page reservations on non Numa system'''
-    print('Pages Size Total')
-    path = '/sys/kernel/mm/hugepages'
-    for hdir in os.listdir(path):
-        pages = get_hugepages(path + '/' + hdir)
-        if pages > 0:
-            kb = int(hdir[10:-2])
-            print('{:<5} {:<6} {}'.format(pages, fmt_memsize(kb),
-                                          fmt_memsize(pages * kb)))
-
-
-def show_pages():
-    '''Show existing huge page settings'''
-    if is_numa():
-        show_numa_pages()
-    else:
-        show_non_numa_pages()
-
-
-def clear_pages():
-    '''Clear all existing huge page mappings'''
-    if is_numa():
-        dirs = glob.glob(
-            '/sys/devices/system/node/node*/hugepages/hugepages-*')
-    else:
-        dirs = glob.glob('/sys/kernel/mm/hugepages/hugepages-*')
-
-    for path in dirs:
-        set_hugepages(path, 0)
-
-
-def default_pagesize():
-    '''Get default huge page size from /proc/meminfo'''
-    with open('/proc/meminfo') as meminfo:
+def default_pagesize() -> int:
+    """Get default huge page size from /proc/meminfo"""
+    key = "Hugepagesize"
+    with open(MEMINFO_PATH, encoding="utf-8") as meminfo:
         for line in meminfo:
-            if line.startswith('Hugepagesize:'):
+            if line.startswith(f"{key}:"):
                 return int(line.split()[1])
-    return None
+    raise KeyError(f'"{key}" not found in {MEMINFO_PATH}')
 
 
-def set_numa_pages(pages, hugepgsz, node=None):
-    '''Set huge page reservation on Numa system'''
-    if node:
-        nodes = ['/sys/devices/system/node/node{}/hugepages'.format(node)]
-    else:
-        nodes = glob.glob('/sys/devices/system/node/node*/hugepages')
-
-    for node_path in nodes:
-        huge_path = '{}/hugepages-{}kB'.format(node_path, hugepgsz)
-        set_hugepages(huge_path, pages)
-
-
-def set_non_numa_pages(pages, hugepgsz):
-    '''Set huge page reservation on non Numa system'''
-    path = '/sys/kernel/mm/hugepages/hugepages-{}kB'.format(hugepgsz)
-    set_hugepages(path, pages)
-
-
-def reserve_pages(pages, hugepgsz, node=None):
-    '''Set the number of huge pages to be reserved'''
-    if node or is_numa():
-        set_numa_pages(pages, hugepgsz, node=node)
-    else:
-        set_non_numa_pages(pages, hugepgsz)
-
-
-def get_mountpoints():
-    '''Get list of where hugepage filesystem is mounted'''
-    mounted = []
-    with open('/proc/mounts') as mounts:
+def get_hugetlbfs_mountpoints() -> T.List[str]:
+    """Get list of where huge page filesystem is mounted"""
+    mounted: T.List[str] = []
+    with open(MOUNTS_PATH, encoding="utf-8") as mounts:
         for line in mounts:
             fields = line.split()
-            if fields[2] != 'hugetlbfs':
+            if fields[2] != "hugetlbfs":
                 continue
             mounted.append(fields[1])
     return mounted
 
 
-def mount_huge(pagesize, mountpoint, user, group):
-    '''Mount the huge TLB file system'''
-    if mountpoint in get_mountpoints():
-        print(mountpoint, "already mounted")
-        return
-    cmd = "mount -t hugetlbfs"
-    if pagesize:
-        cmd += ' -o pagesize={}'.format(pagesize * 1024)
-    if user:
-        cmd += ' -o uid=' + user
-    if group:
-        cmd += ' -o gid=' + group
-    cmd += ' nodev ' + mountpoint
-    os.system(cmd)
+def print_row(cells: T.Tuple[str, ...], widths: T.List[int]) -> None:
+    """Print a row of a table with the given column widths"""
+    first, *rest = cells
+    w_first, *w_rest = widths
+    first_end = " " * 2
+    rest_end = " " * 2
 
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
 
-def umount_huge(mountpoint):
-    '''Unmount the huge TLB file system (if mounted)'''
-    if mountpoint in get_mountpoints():
-        os.system("umount " + mountpoint)
 
+def print_hp_status(hp_res: T.List[HugepageRes]) -> None:
+    """Display status of huge page reservations"""
+    numa = is_numa()
 
-def show_mount():
-    '''Show where huge page filesystem is mounted'''
-    mounted = get_mountpoints()
-    if mounted:
-        print("Hugepages mounted on", *mounted)
+    # print out huge page information in a table
+    rows: T.List[T.Tuple[str, ...]]
+    headers: T.Tuple[str, ...]
+    if numa:
+        headers = "Node", "Pages", "Size", "Total"
+        rows = [
+            (
+                str(hp.node),
+                str(nr_pages),
+                fmt_memsize(sz),
+                fmt_memsize(sz * nr_pages),
+            )
+            # iterate over each huge page sysfs node...
+            for hp in hp_res
+            # ...and each page size within that node...
+            for sz in hp.valid_page_sizes
+            # ...we need number of pages multiple times, so we read it here...
+            for nr_pages in [hp[sz]]
+            # ...include this row only if there are pages reserved
+            if nr_pages
+        ]
     else:
-        print("Hugepages not mounted")
+        headers = "Pages", "Size", "Total"
+        # if NUMA is disabled, we know there's only one huge page dir
+        hp = hp_res[0]
+        rows = [
+            (str(nr_pages), fmt_memsize(sz), fmt_memsize(sz * nr_pages))
+            # iterate over each page size within the huge page dir
+            for sz in hp.valid_page_sizes
+            # read number of pages for this size
+            for nr_pages in [hp[sz]]
+            # skip if no pages
+            if nr_pages
+        ]
+    if not rows:
+        print("No huge pages reserved")
+        return
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [headers])
+        for col_idx in range(len(headers))
+    ]
+
+    # print everything
+    print_row(headers, col_widths)
+    for r in rows:
+        print_row(r, col_widths)
+
+
+def print_mount_status() -> None:
+    """Display status of huge page filesystem mounts"""
+    mounted = get_hugetlbfs_mountpoints()
+    if not mounted:
+        print("No huge page filesystems mounted")
+        return
+    print("Huge page filesystems mounted at:", *mounted, sep=" ")
+
+
+def scan_huge_dirs(node: T.Optional[int]) -> T.List[HugepageRes]:
+    """Return a HugepageRes object for each huge page directory"""
+    # if NUMA is enabled, scan per-NUMA node huge pages
+    if is_numa():
+        # helper function to extract node number from directory name
+        def _get_node(path: str) -> T.Optional[int]:
+            m = re.match(r"node(\d+)", os.path.basename(path))
+            return int(m.group(1)) if m else None
+
+        # we want a sorted list of NUMA nodes
+        nodes = sorted(
+            n
+            # iterate over all directories in the base directory
+            for d in os.listdir(NUMA_NODE_BASE_DIR)
+            # extract the node number from the directory name
+            for n in [_get_node(d)]
+            # filter out None values (non-NUMA node directories)
+            if n is not None
+        )
+        return [
+            HugepageRes(os.path.join(NUMA_NODE_BASE_DIR, f"node{n}", "hugepages"), n)
+            for n in nodes
+            # if user requested a specific node, only include that one
+            if node is None or n == node
+        ]
+    # otherwise, use non-NUMA huge page directory
+    if node is not None:
+        raise ValueError("NUMA node requested but not supported")
+    return [HugepageRes(NO_NUMA_HUGE_DIR)]
+
+
+def try_reserve_huge_pages(
+    hp_res: T.List[HugepageRes], mem_sz: str, pagesize_kb: int
+) -> None:
+    """Reserve huge pages if possible"""
+    reserve_kb = get_memsize(mem_sz)
+
+    # is this a valid request?
+    if reserve_kb % pagesize_kb != 0:
+        fmt_res = fmt_memsize(reserve_kb)
+        fmt_sz = fmt_memsize(pagesize_kb)
+        raise ValueError(
+            f"Huge reservation {fmt_res} is " f"not a multiple of page size {fmt_sz}"
+        )
+
+    # request is valid, reserve pages
+    for hp in hp_res:
+        req = reserve_kb // pagesize_kb
+        hp[pagesize_kb] = req
+        got = hp[pagesize_kb]
+        # did we fulfill our request?
+        if got != req:
+            raise OSError(
+                f"Failed to reserve {req} pages of size "
+                f"{fmt_memsize(pagesize_kb)}, "
+                f"got {got} pages instead"
+            )
 
 
 def main():
-    '''Process the command line arguments and setup huge pages'''
+    """Process the command line arguments and setup huge pages"""
     parser = argparse.ArgumentParser(
         formatter_class=argparse.RawDescriptionHelpFormatter,
         description="Setup huge pages",
@@ -214,58 +288,68 @@ def main():
 
 To a complete setup of with 2 Gigabyte of 1G huge pages:
     %(prog)s -p 1G --setup 2G
-""")
+""",
+    )
     parser.add_argument(
-        '--show',
-        '-s',
-        action='store_true',
-        help="print the current huge page configuration")
+        "--show",
+        "-s",
+        action="store_true",
+        help="Print current huge page configuration",
+    )
     parser.add_argument(
-        '--clear', '-c', action='store_true', help="clear existing huge pages")
+        "--clear", "-c", action="store_true", help="Clear existing huge pages"
+    )
     parser.add_argument(
-        '--mount',
-        '-m',
-        action='store_true',
-        help='mount the huge page filesystem')
+        "--mount",
+        "-m",
+        action="store_true",
+        help="Mount the huge page filesystem",
+    )
     parser.add_argument(
-        '--unmount',
-        '-u',
-        action='store_true',
-        help='unmount the system huge page directory')
+        "--unmount",
+        "-u",
+        action="store_true",
+        help="Unmount the system huge page directory",
+    )
     parser.add_argument(
-        '--directory',
-        '-d',
-        metavar='DIR',
+        "--directory",
+        "-d",
+        metavar="DIR",
         default=HUGE_MOUNT,
-        help='mount point')
+        help="Mount point for huge pages",
+    )
     parser.add_argument(
-        '--user',
-        '-U',
-        metavar='UID',
-        help='set the mounted directory owner user')
+        "--user",
+        "-U",
+        metavar="UID",
+        help="Set the mounted directory owner user",
+    )
     parser.add_argument(
-        '--group',
-        '-G',
-        metavar='GID',
-        help='set the mounted directory owner group')
+        "--group",
+        "-G",
+        metavar="GID",
+        help="Set the mounted directory owner group",
+    )
     parser.add_argument(
-        '--node', '-n', help='select numa node to reserve pages on')
+        "--node", "-n", type=int, help="Select numa node to reserve pages on"
+    )
     parser.add_argument(
-        '--pagesize',
-        '-p',
-        metavar='SIZE',
-        help='choose huge page size to use')
+        "--pagesize", "-p", metavar="SIZE", help="Choose huge page size to use"
+    )
     parser.add_argument(
-        '--reserve',
-        '-r',
-        metavar='SIZE',
-        help='reserve huge pages. Size is in bytes with K, M, or G suffix')
+        "--reserve",
+        "-r",
+        metavar="SIZE",
+        help="Reserve huge pages. Size is in bytes with K, M, or G suffix",
+    )
     parser.add_argument(
-        '--setup',
-        metavar='SIZE',
-        help='setup huge pages by doing clear, unmount, reserve and mount')
+        "--setup",
+        metavar="SIZE",
+        help="Setup huge pages by doing clear, unmount, reserve and mount",
+    )
     args = parser.parse_args()
 
+    # setup is clear, then unmount, then reserve, then mount
     if args.setup:
         args.clear = True
         args.unmount = True
@@ -275,33 +359,51 @@ def main():
     if not (args.show or args.mount or args.unmount or args.clear or args.reserve):
         parser.error("no action specified")
 
+    # read huge page data from sysfs
+    hp_res = scan_huge_dirs(args.node)
+
+    # read huge page mountpoint data
+    hp_mountpoint = args.directory
+    hp_mounted = hp_mountpoint in get_hugetlbfs_mountpoints()
+    hp_mount = HugepageMount(hp_mountpoint, hp_mounted)
+
+    # get requested page size we will be working with
     if args.pagesize:
         pagesize_kb = get_memsize(args.pagesize)
     else:
         pagesize_kb = default_pagesize()
-    if not pagesize_kb:
-        sys.exit("Invalid page size: {}kB".format(pagesize_kb))
 
+    # were we asked to clear?
     if args.clear:
-        clear_pages()
+        for hp in hp_res:
+            for sz in hp.valid_page_sizes:
+                hp[sz] = 0
+
+    # were we asked to unmount?
     if args.unmount:
-        umount_huge(args.directory)
+        hp_mount.unmount()
 
+    # were we asked to reserve pages?
     if args.reserve:
-        reserve_kb = get_memsize(args.reserve)
-        if reserve_kb % pagesize_kb != 0:
-            sys.exit(
-                'Huge reservation {}kB is not a multiple of page size {}kB'.
-                format(reserve_kb, pagesize_kb))
-        reserve_pages(
-            int(reserve_kb / pagesize_kb), pagesize_kb, node=args.node)
+        try_reserve_huge_pages(hp_res, args.reserve, pagesize_kb)
+
+    # were we asked to mount?
     if args.mount:
-        mount_huge(pagesize_kb, args.directory, args.user, args.group)
+        hp_mount.mount(pagesize_kb, args.user, args.group)
+
+    # were we asked to display status?
     if args.show:
-        show_pages()
+        print_hp_status(hp_res)
         print()
-        show_mount()
+        print_mount_status()
 
 
 if __name__ == "__main__":
-    main()
+    try:
+        main()
+    except PermissionError:
+        sys.exit("Permission denied: need to be root!")
+    except subprocess.CalledProcessError as e:
+        sys.exit(f"Command failed: {e}")
+    except (KeyError, ValueError, OSError) as e:
+        sys.exit(f"Error: {e}")
-- 
2.43.5

Re: [PATCH v5 3/4] usertools/dpdk-hugepages.py: update coding style

[Robin Jarry]

Anatoly Burakov, Aug 21, 2024 at 11:44:
> Update coding style:
>
> - make the code PEP-484 compliant
> - add more comments, improve readability, use f-strings everywhere
> - address all Python static analysis (e.g. mypy, pylint) warnings
> - format code with Ruff
> - improve error handling
> - refactor printing and sysfs/procfs access functions
> - sort huge page reservation status output by NUMA node
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>
> Notes:
>     v4 -> v5:
>     - Format with Ruff on default settings
>     - Replaced all instances of raw path strings with os.path.join
>     v3 -> v4:
>     - Format code with Ruff, line width 79 to avoid flake8 warnings
>       (Flake8 is by default configured with line width 79 on my system)
>     v2 -> v3:
>     - Rewrite of the script as suggested by reviewers
>     v1 -> v2:
>     - Added commit that sorted output by NUMA node
>
>  usertools/dpdk-hugepages.py | 518 +++++++++++++++++++++---------------
>  1 file changed, 310 insertions(+), 208 deletions(-)

Acked-by: Robin Jarry <rjarry@redhat.com>

Thanks!

[PATCH v5 4/4] usertools/dpdk-devbind: print NUMA node

[Anatoly Burakov]

Currently, devbind does not print out any NUMA information, which makes
figuring out which NUMA node device belongs to not trivial. Add printouts
for NUMA information if NUMA support is enabled on the system.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Robin Jarry <rjarry@redhat.com>
---

Notes:
    v1 -> v2:
    - Added commit to print out NUMA information in devbind

 usertools/dpdk-devbind.py | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index b276e8efc8..078e8c387b 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -110,6 +110,11 @@
 args = []
 
 
+# check if this system has NUMA support
+def is_numa():
+    return os.path.exists('/sys/devices/system/node')
+
+
 # check if a specific kernel module is loaded
 def module_is_loaded(module):
     global loaded_modules
@@ -577,20 +582,28 @@ def show_device_status(devices_type, device_name, if_field=False):
         print("".join('=' * len(msg)))
         return
 
+    print_numa = is_numa()
+
     # print each category separately, so we can clearly see what's used by DPDK
     if dpdk_drv:
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
         display_devices("%s devices using DPDK-compatible driver" % device_name,
-                        dpdk_drv, "drv=%(Driver_str)s unused=%(Module_str)s")
+                        dpdk_drv, extra_param)
     if kernel_drv:
-        if_text = ""
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
         if if_field:
-            if_text = "if=%(Interface)s "
-        display_devices("%s devices using kernel driver" % device_name, kernel_drv,
-                        if_text + "drv=%(Driver_str)s "
-                        "unused=%(Module_str)s %(Active)s")
+            extra_param = "if=%(Interface)s " + extra_param
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("%s devices using kernel driver" % device_name,
+                        kernel_drv, extra_param)
     if no_drv:
-        display_devices("Other %s devices" % device_name, no_drv,
-                        "unused=%(Module_str)s")
+        extra_param = "unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("Other %s devices" % device_name, no_drv, extra_param)
 
 
 def show_status():
-- 
2.43.5

Re: [PATCH v5 4/4] usertools/dpdk-devbind: print NUMA node

[Robin Jarry]

Anatoly Burakov, Aug 21, 2024 at 11:44:
> Currently, devbind does not print out any NUMA information, which makes
> figuring out which NUMA node device belongs to not trivial. Add printouts
> for NUMA information if NUMA support is enabled on the system.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Robin Jarry <rjarry@redhat.com>
> ---
>
> Notes:
>     v1 -> v2:
>     - Added commit to print out NUMA information in devbind
>
>  usertools/dpdk-devbind.py | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)

Acked-by: Robin Jarry <rjarry@redhat.com>

Thanks!

Re: [PATCH v5 1/4] usertools/cpu_layout: update coding style

[Robin Jarry]

Anatoly Burakov, Aug 21, 2024 at 11:44:
> Update coding style:
>
> - make it PEP-484 compliant
> - format code with Ruff
> - address all mypy etc. warnings
> - use f-strings in place of old-style string interpolation
> - refactor printing to make the code more readable
> - read valid CPU ID's from "online" sysfs node
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v4-v5:
>     - Format with Ruff on default settings
>     
>     v3->v4:
>     - Format with Ruff, line width 79
>     
>     v1,v2 -> v3:
>     - Import typing as T instead of individual types
>
>  usertools/cpu_layout.py | 161 ++++++++++++++++++++++++++--------------
>  1 file changed, 106 insertions(+), 55 deletions(-)

Acked-by: Robin Jarry <rjarry@redhat.com>

Thanks!

[PATCH v6 1/4] usertools/cpu_layout: update coding style

[Anatoly Burakov]

Update coding style:

- make it PEP-484 compliant
- format code with Ruff
- address all mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable
- read valid CPU ID's from "online" sysfs node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Robin Jarry <rjarry@redhat.com>
---

Notes:
    v4-v5:
    - Format with Ruff on default settings
    
    v3->v4:
    - Format with Ruff, line width 79
    
    v1,v2 -> v3:
    - Import typing as T instead of individual types

 usertools/cpu_layout.py | 161 ++++++++++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 55 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..e133fb8ad3 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,113 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2017 Cavium, Inc. All rights reserved.
 
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-    try:
-        fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-    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)
-    if socket not in sockets:
-        sockets.append(socket)
-    key = (socket, core)
-    if key not in core_map:
-        core_map[key] = []
-    core_map[key].append(cpu)
+"""Display CPU topology information."""
 
-print(format("=" * (47 + len(base_path))))
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path))))
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
+import typing as T
 
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count)  \
-                      + len(", ") * (max_thread_count - 1) \
-                      + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
 
-output = " ".ljust(max_core_id_len + len('Core '))
-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 '))
-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)
+def range_expand(rstr: str) -> T.List[int]:
+    """Expand a range string into a list of integers."""
+    # 0,1-3 => [0, 1-3]
+    ranges = rstr.split(",")
+    valset: T.List[int] = []
+    for r in ranges:
+        # 1-3 => [1, 2, 3]
+        if "-" in r:
+            start, end = r.split("-")
+            valset.extend(range(int(start), int(end) + 1))
         else:
-            output += " " * (max_core_map_len + 1)
-    print(output)
+            valset.append(int(r))
+    return valset
+
+
+def read_sysfs(path: str) -> str:
+    """Read a sysfs file and return its contents."""
+    with open(path, encoding="utf-8") as fd:
+        return fd.read().strip()
+
+
+def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
+    """Print a row of a table with the given column widths."""
+    first, *rest = row
+    w_first, *w_rest = col_widths
+    first_end = " " * 4
+    rest_end = " " * 10
+
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
+
+
+def print_section(heading: str) -> None:
+    """Print a section heading."""
+    sep = "=" * len(heading)
+    print(sep)
+    print(heading)
+    print(sep)
+    print()
+
+
+def main() -> None:
+    """Print CPU topology information."""
+    sockets_s: T.Set[int] = set()
+    cores_s: T.Set[int] = set()
+    core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    base_path = "/sys/devices/system/cpu"
+
+    cpus = range_expand(read_sysfs(f"{base_path}/online"))
+
+    for cpu in cpus:
+        lcore_base = f"{base_path}/cpu{cpu}"
+        core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
+        socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+        cores_s.add(core)
+        sockets_s.add(socket)
+        key = (socket, core)
+        core_map.setdefault(key, [])
+        core_map[key].append(cpu)
+
+    cores = sorted(cores_s)
+    sockets = sorted(sockets_s)
+
+    print_section(f"Core and Socket Information (as reported by '{base_path}')")
+
+    print("cores = ", cores)
+    print("sockets = ", sockets)
+    print()
+
+    # Core, [Socket, Socket, ...]
+    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
+    rows: T.List[T.Tuple[str, ...]] = []
+
+    for c in cores:
+        # Core,
+        row: T.Tuple[str, ...] = (f"Core {c}",)
+
+        # [lcores, lcores, ...]
+        for s in sockets:
+            try:
+                lcores = core_map[(s, c)]
+                row += (str(lcores),)
+            except KeyError:
+                row += ("",)
+        rows += [row]
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [heading_strs])
+        for col_idx in range(len(heading_strs))
+    ]
+
+    # print out table taking row widths into account
+    print_row(heading_strs, col_widths)
+    print_row(sep_strs, col_widths)
+    for row in rows:
+        print_row(row, col_widths)
+
+
+if __name__ == "__main__":
+    main()
-- 
2.43.5

[PATCH v6 2/4] usertools/cpu_layout: print out NUMA nodes

[Anatoly Burakov]

In traditional NUMA case, NUMA nodes and physical sockets were used
interchangeably, but there are cases where there can be multiple NUMA
nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
cases of multiple sockets. Use sysfs to print out NUMA information.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v5 -> v6:
    - Track NUMA changes per socket to avoid issues with missing cores
    
    v2 -> v3:
    - Sort imports alphabetically

 usertools/cpu_layout.py | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index e133fb8ad3..5972cfecdb 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -5,6 +5,7 @@
 
 """Display CPU topology information."""
 
+import glob
 import typing as T
 
 
@@ -29,12 +30,21 @@ def read_sysfs(path: str) -> str:
         return fd.read().strip()
 
 
+def read_numa_node(base: str) -> int:
+    """Read the NUMA node of a CPU."""
+    node_glob = f"{base}/node*"
+    node_dirs = glob.glob(node_glob)
+    if not node_dirs:
+        return 0  # default to node 0
+    return int(node_dirs[0].split("node")[1])
+
+
 def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
     """Print a row of a table with the given column widths."""
     first, *rest = row
     w_first, *w_rest = col_widths
     first_end = " " * 4
-    rest_end = " " * 10
+    rest_end = " " * 4
 
     print(first.ljust(w_first), end=first_end)
     for cell, width in zip(rest, w_rest):
@@ -56,6 +66,7 @@ def main() -> None:
     sockets_s: T.Set[int] = set()
     cores_s: T.Set[int] = set()
     core_map: T.Dict[T.Tuple[int, int], T.List[int]] = {}
+    numa_map: T.Dict[int, int] = {}
     base_path = "/sys/devices/system/cpu"
 
     cpus = range_expand(read_sysfs(f"{base_path}/online"))
@@ -64,12 +75,14 @@ def main() -> None:
         lcore_base = f"{base_path}/cpu{cpu}"
         core = int(read_sysfs(f"{lcore_base}/topology/core_id"))
         socket = int(read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+        node = read_numa_node(lcore_base)
 
         cores_s.add(core)
         sockets_s.add(socket)
         key = (socket, core)
         core_map.setdefault(key, [])
         core_map[key].append(cpu)
+        numa_map[cpu] = node
 
     cores = sorted(cores_s)
     sockets = sorted(sockets_s)
@@ -78,24 +91,36 @@ def main() -> None:
 
     print("cores = ", cores)
     print("sockets = ", sockets)
+    print("numa = ", sorted(set(numa_map.values())))
     print()
 
-    # Core, [Socket, Socket, ...]
-    heading_strs = "", *[f"Socket {s}" for s in sockets]
+    # Core, [NUMA, Socket, NUMA, Socket, ...]
+    heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
     sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
     rows: T.List[T.Tuple[str, ...]] = []
 
+    # track NUMA changes per socket
+    prev_numa: T.Dict[int, T.Optional[int]] = {socket: None for socket in sockets}
     for c in cores:
         # Core,
         row: T.Tuple[str, ...] = (f"Core {c}",)
 
-        # [lcores, lcores, ...]
+        # [NUMA, lcores, NUMA, lcores, ...]
         for s in sockets:
             try:
                 lcores = core_map[(s, c)]
+
+                numa = numa_map[lcores[0]]
+                numa_changed = prev_numa[s] != numa
+                prev_numa[s] = numa
+
+                if numa_changed:
+                    row += (f"NUMA {numa}",)
+                else:
+                    row += ("",)
                 row += (str(lcores),)
             except KeyError:
-                row += ("",)
+                row += ("", "")
         rows += [row]
 
     # find max widths for each column, including header and rows
-- 
2.43.5

Re: [PATCH v6 2/4] usertools/cpu_layout: print out NUMA nodes

[Robin Jarry]

Anatoly Burakov, Aug 22, 2024 at 12:38:
> In traditional NUMA case, NUMA nodes and physical sockets were used
> interchangeably, but there are cases where there can be multiple NUMA
> nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
> cases of multiple sockets. Use sysfs to print out NUMA information.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v5 -> v6:
>     - Track NUMA changes per socket to avoid issues with missing cores

Acked-by: Robin Jarry <rjarry@redhat.com>

Re: [PATCH v6 2/4] usertools/cpu_layout: print out NUMA nodes

[Stephen Hemminger]

On Thu, 22 Aug 2024 19:43:05 +0200
"Robin Jarry" <rjarry@redhat.com> wrote:

> Anatoly Burakov, Aug 22, 2024 at 12:38:
> > In traditional NUMA case, NUMA nodes and physical sockets were used
> > interchangeably, but there are cases where there can be multiple NUMA
> > nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
> > cases of multiple sockets. Use sysfs to print out NUMA information.
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> >
> > Notes:
> >     v5 -> v6:
> >     - Track NUMA changes per socket to avoid issues with missing cores  
> 
> Acked-by: Robin Jarry <rjarry@redhat.com>
> 


Acked-by: Stephen Hemminger <stephen@networkplumber.org>

[PATCH v6 3/4] usertools/dpdk-hugepages.py: update coding style

[Anatoly Burakov]

Update coding style:

- make the code PEP-484 compliant
- add more comments, improve readability, use f-strings everywhere
- address all Python static analysis (e.g. mypy, pylint) warnings
- format code with Ruff
- improve error handling
- refactor printing and sysfs/procfs access functions
- sort huge page reservation status output by NUMA node

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Robin Jarry <rjarry@redhat.com>
---

Notes:
    v4 -> v5:
    - Format with Ruff on default settings
    - Replaced all instances of raw path strings with os.path.join
    v3 -> v4:
    - Format code with Ruff, line width 79 to avoid flake8 warnings
      (Flake8 is by default configured with line width 79 on my system)
    v2 -> v3:
    - Rewrite of the script as suggested by reviewers
    v1 -> v2:
    - Added commit that sorted output by NUMA node

 usertools/dpdk-hugepages.py | 518 +++++++++++++++++++++---------------
 1 file changed, 310 insertions(+), 208 deletions(-)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index bf2575ba36..3fc3269c83 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -1,13 +1,15 @@
 #! /usr/bin/env python3
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright (c) 2020 Microsoft Corporation
+
 """Script to query and setup huge pages for DPDK applications."""
 
 import argparse
-import glob
 import os
 import re
+import subprocess
 import sys
+import typing as T
 from math import log2
 
 # Standard binary prefix
@@ -15,194 +17,266 @@
 
 # systemd mount point for huge pages
 HUGE_MOUNT = "/dev/hugepages"
+# default directory for non-NUMA huge pages
+NO_NUMA_HUGE_DIR = "/sys/kernel/mm/hugepages"
+# default base directory for NUMA nodes
+NUMA_NODE_BASE_DIR = "/sys/devices/system/node"
+# procfs paths
+MEMINFO_PATH = "/proc/meminfo"
+MOUNTS_PATH = "/proc/mounts"
 
 
-def fmt_memsize(kb):
-    '''Format memory size in kB into conventional format'''
+class HugepageMount:
+    """Mount operations for huge page filesystem."""
+
+    def __init__(self, path: str, mounted: bool):
+        self.path = path
+        # current mount status
+        self.mounted = mounted
+
+    def mount(
+        self, pagesize_kb: int, user: T.Optional[str], group: T.Optional[str]
+    ) -> None:
+        """Mount the huge TLB file system"""
+        if self.mounted:
+            return
+        cmd = ["mount", "-t", "hugetlbfs"]
+        cmd += ["-o", f"pagesize={pagesize_kb * 1024}"]
+        if user is not None:
+            cmd += ["-o", f"uid={user}"]
+        if group is not None:
+            cmd += ["-o", f"gid={group}"]
+        cmd += ["nodev", self.path]
+
+        subprocess.run(cmd, check=True)
+        self.mounted = True
+
+    def unmount(self) -> None:
+        """Unmount the huge TLB file system (if mounted)"""
+        if self.mounted:
+            subprocess.run(["umount", self.path], check=True)
+            self.mounted = False
+
+
+class HugepageRes:
+    """Huge page reserve operations. Can be NUMA-node-specific."""
+
+    def __init__(self, path: str, node: T.Optional[int] = None):
+        self.path = path
+        # if this is a per-NUMA node huge page dir, store the node number
+        self.node = node
+        self.valid_page_sizes = self._get_valid_page_sizes()
+
+    def _get_valid_page_sizes(self) -> T.List[int]:
+        """Extract valid huge page sizes"""
+        return [get_memsize(d.split("-")[1]) for d in os.listdir(self.path)]
+
+    def _nr_pages_path(self, sz: int) -> str:
+        if sz not in self.valid_page_sizes:
+            raise ValueError(
+                f"Invalid page size {sz}. " f"Valid sizes: {self.valid_page_sizes}"
+            )
+        return os.path.join(self.path, f"hugepages-{sz}kB", "nr_hugepages")
+
+    def __getitem__(self, sz: int) -> int:
+        """Get current number of reserved pages of specified size"""
+        with open(self._nr_pages_path(sz), encoding="utf-8") as f:
+            return int(f.read())
+
+    def __setitem__(self, sz: int, nr_pages: int) -> None:
+        """Set number of reserved pages of specified size"""
+        with open(self._nr_pages_path(sz), "w", encoding="utf-8") as f:
+            f.write(f"{nr_pages}\n")
+
+
+def fmt_memsize(kb: int) -> str:
+    """Format memory size in kB into conventional format"""
     logk = int(log2(kb) / 10)
     suffix = BINARY_PREFIX[logk]
-    unit = 2**(logk * 10)
-    return '{}{}b'.format(int(kb / unit), suffix)
+    unit = 2 ** (logk * 10)
+    return f"{int(kb / unit)}{suffix}b"
 
 
-def get_memsize(arg):
-    '''Convert memory size with suffix to kB'''
-    match = re.match(r'(\d+)([' + BINARY_PREFIX + r']?)$', arg.upper())
+def get_memsize(arg: str) -> int:
+    """Convert memory size with suffix to kB"""
+    # arg may have a 'b' at the end
+    if arg[-1].lower() == "b":
+        arg = arg[:-1]
+    match = re.match(rf"(\d+)([{BINARY_PREFIX}]?)$", arg.upper())
     if match is None:
-        sys.exit('{} is not a valid size'.format(arg))
+        raise ValueError(f"{arg} is not a valid size")
     num = float(match.group(1))
     suffix = match.group(2)
-    if suffix == "":
+    if not suffix:
         return int(num / 1024)
     idx = BINARY_PREFIX.find(suffix)
-    return int(num * (2**(idx * 10)))
+    return int(num * (2 ** (idx * 10)))
 
 
-def is_numa():
-    '''Test if NUMA is necessary on this system'''
-    return os.path.exists('/sys/devices/system/node')
+def is_numa() -> bool:
+    """Check if NUMA is supported"""
+    return os.path.exists(NUMA_NODE_BASE_DIR)
 
 
-def get_valid_page_sizes(path):
-    '''Extract valid hugepage sizes'''
-    dir = os.path.dirname(path)
-    pg_sizes = (d.split("-")[1] for d in os.listdir(dir))
-    return " ".join(pg_sizes)
-
-
-def get_hugepages(path):
-    '''Read number of reserved pages'''
-    with open(path + '/nr_hugepages') as nr_hugepages:
-        return int(nr_hugepages.read())
-    return 0
-
-
-def set_hugepages(path, reqpages):
-    '''Write the number of reserved huge pages'''
-    filename = path + '/nr_hugepages'
-    try:
-        with open(filename, 'w') as nr_hugepages:
-            nr_hugepages.write('{}\n'.format(reqpages))
-    except PermissionError:
-        sys.exit('Permission denied: need to be root!')
-    except FileNotFoundError:
-        sys.exit("Invalid page size. Valid page sizes: {}".format(
-                 get_valid_page_sizes(path)))
-    gotpages = get_hugepages(path)
-    if gotpages != reqpages:
-        sys.exit('Unable to set pages ({} instead of {} in {}).'.format(
-                 gotpages, reqpages, filename))
-
-
-def show_numa_pages():
-    '''Show huge page reservations on Numa system'''
-    print('Node Pages Size Total')
-    for numa_path in glob.glob('/sys/devices/system/node/node*'):
-        node = numa_path[29:]  # slice after /sys/devices/system/node/node
-        path = numa_path + '/hugepages'
-        if not os.path.exists(path):
-            continue
-        for hdir in os.listdir(path):
-            pages = get_hugepages(path + '/' + hdir)
-            if pages > 0:
-                kb = int(hdir[10:-2])  # slice out of hugepages-NNNkB
-                print('{:<4} {:<5} {:<6} {}'.format(node, pages,
-                                                    fmt_memsize(kb),
-                                                    fmt_memsize(pages * kb)))
-
-
-def show_non_numa_pages():
-    '''Show huge page reservations on non Numa system'''
-    print('Pages Size Total')
-    path = '/sys/kernel/mm/hugepages'
-    for hdir in os.listdir(path):
-        pages = get_hugepages(path + '/' + hdir)
-        if pages > 0:
-            kb = int(hdir[10:-2])
-            print('{:<5} {:<6} {}'.format(pages, fmt_memsize(kb),
-                                          fmt_memsize(pages * kb)))
-
-
-def show_pages():
-    '''Show existing huge page settings'''
-    if is_numa():
-        show_numa_pages()
-    else:
-        show_non_numa_pages()
-
-
-def clear_pages():
-    '''Clear all existing huge page mappings'''
-    if is_numa():
-        dirs = glob.glob(
-            '/sys/devices/system/node/node*/hugepages/hugepages-*')
-    else:
-        dirs = glob.glob('/sys/kernel/mm/hugepages/hugepages-*')
-
-    for path in dirs:
-        set_hugepages(path, 0)
-
-
-def default_pagesize():
-    '''Get default huge page size from /proc/meminfo'''
-    with open('/proc/meminfo') as meminfo:
+def default_pagesize() -> int:
+    """Get default huge page size from /proc/meminfo"""
+    key = "Hugepagesize"
+    with open(MEMINFO_PATH, encoding="utf-8") as meminfo:
         for line in meminfo:
-            if line.startswith('Hugepagesize:'):
+            if line.startswith(f"{key}:"):
                 return int(line.split()[1])
-    return None
+    raise KeyError(f'"{key}" not found in {MEMINFO_PATH}')
 
 
-def set_numa_pages(pages, hugepgsz, node=None):
-    '''Set huge page reservation on Numa system'''
-    if node:
-        nodes = ['/sys/devices/system/node/node{}/hugepages'.format(node)]
-    else:
-        nodes = glob.glob('/sys/devices/system/node/node*/hugepages')
-
-    for node_path in nodes:
-        huge_path = '{}/hugepages-{}kB'.format(node_path, hugepgsz)
-        set_hugepages(huge_path, pages)
-
-
-def set_non_numa_pages(pages, hugepgsz):
-    '''Set huge page reservation on non Numa system'''
-    path = '/sys/kernel/mm/hugepages/hugepages-{}kB'.format(hugepgsz)
-    set_hugepages(path, pages)
-
-
-def reserve_pages(pages, hugepgsz, node=None):
-    '''Set the number of huge pages to be reserved'''
-    if node or is_numa():
-        set_numa_pages(pages, hugepgsz, node=node)
-    else:
-        set_non_numa_pages(pages, hugepgsz)
-
-
-def get_mountpoints():
-    '''Get list of where hugepage filesystem is mounted'''
-    mounted = []
-    with open('/proc/mounts') as mounts:
+def get_hugetlbfs_mountpoints() -> T.List[str]:
+    """Get list of where huge page filesystem is mounted"""
+    mounted: T.List[str] = []
+    with open(MOUNTS_PATH, encoding="utf-8") as mounts:
         for line in mounts:
             fields = line.split()
-            if fields[2] != 'hugetlbfs':
+            if fields[2] != "hugetlbfs":
                 continue
             mounted.append(fields[1])
     return mounted
 
 
-def mount_huge(pagesize, mountpoint, user, group):
-    '''Mount the huge TLB file system'''
-    if mountpoint in get_mountpoints():
-        print(mountpoint, "already mounted")
-        return
-    cmd = "mount -t hugetlbfs"
-    if pagesize:
-        cmd += ' -o pagesize={}'.format(pagesize * 1024)
-    if user:
-        cmd += ' -o uid=' + user
-    if group:
-        cmd += ' -o gid=' + group
-    cmd += ' nodev ' + mountpoint
-    os.system(cmd)
+def print_row(cells: T.Tuple[str, ...], widths: T.List[int]) -> None:
+    """Print a row of a table with the given column widths"""
+    first, *rest = cells
+    w_first, *w_rest = widths
+    first_end = " " * 2
+    rest_end = " " * 2
 
+    print(first.ljust(w_first), end=first_end)
+    for cell, width in zip(rest, w_rest):
+        print(cell.rjust(width), end=rest_end)
+    print()
 
-def umount_huge(mountpoint):
-    '''Unmount the huge TLB file system (if mounted)'''
-    if mountpoint in get_mountpoints():
-        os.system("umount " + mountpoint)
 
+def print_hp_status(hp_res: T.List[HugepageRes]) -> None:
+    """Display status of huge page reservations"""
+    numa = is_numa()
 
-def show_mount():
-    '''Show where huge page filesystem is mounted'''
-    mounted = get_mountpoints()
-    if mounted:
-        print("Hugepages mounted on", *mounted)
+    # print out huge page information in a table
+    rows: T.List[T.Tuple[str, ...]]
+    headers: T.Tuple[str, ...]
+    if numa:
+        headers = "Node", "Pages", "Size", "Total"
+        rows = [
+            (
+                str(hp.node),
+                str(nr_pages),
+                fmt_memsize(sz),
+                fmt_memsize(sz * nr_pages),
+            )
+            # iterate over each huge page sysfs node...
+            for hp in hp_res
+            # ...and each page size within that node...
+            for sz in hp.valid_page_sizes
+            # ...we need number of pages multiple times, so we read it here...
+            for nr_pages in [hp[sz]]
+            # ...include this row only if there are pages reserved
+            if nr_pages
+        ]
     else:
-        print("Hugepages not mounted")
+        headers = "Pages", "Size", "Total"
+        # if NUMA is disabled, we know there's only one huge page dir
+        hp = hp_res[0]
+        rows = [
+            (str(nr_pages), fmt_memsize(sz), fmt_memsize(sz * nr_pages))
+            # iterate over each page size within the huge page dir
+            for sz in hp.valid_page_sizes
+            # read number of pages for this size
+            for nr_pages in [hp[sz]]
+            # skip if no pages
+            if nr_pages
+        ]
+    if not rows:
+        print("No huge pages reserved")
+        return
+
+    # find max widths for each column, including header and rows
+    col_widths = [
+        max(len(tup[col_idx]) for tup in rows + [headers])
+        for col_idx in range(len(headers))
+    ]
+
+    # print everything
+    print_row(headers, col_widths)
+    for r in rows:
+        print_row(r, col_widths)
+
+
+def print_mount_status() -> None:
+    """Display status of huge page filesystem mounts"""
+    mounted = get_hugetlbfs_mountpoints()
+    if not mounted:
+        print("No huge page filesystems mounted")
+        return
+    print("Huge page filesystems mounted at:", *mounted, sep=" ")
+
+
+def scan_huge_dirs(node: T.Optional[int]) -> T.List[HugepageRes]:
+    """Return a HugepageRes object for each huge page directory"""
+    # if NUMA is enabled, scan per-NUMA node huge pages
+    if is_numa():
+        # helper function to extract node number from directory name
+        def _get_node(path: str) -> T.Optional[int]:
+            m = re.match(r"node(\d+)", os.path.basename(path))
+            return int(m.group(1)) if m else None
+
+        # we want a sorted list of NUMA nodes
+        nodes = sorted(
+            n
+            # iterate over all directories in the base directory
+            for d in os.listdir(NUMA_NODE_BASE_DIR)
+            # extract the node number from the directory name
+            for n in [_get_node(d)]
+            # filter out None values (non-NUMA node directories)
+            if n is not None
+        )
+        return [
+            HugepageRes(os.path.join(NUMA_NODE_BASE_DIR, f"node{n}", "hugepages"), n)
+            for n in nodes
+            # if user requested a specific node, only include that one
+            if node is None or n == node
+        ]
+    # otherwise, use non-NUMA huge page directory
+    if node is not None:
+        raise ValueError("NUMA node requested but not supported")
+    return [HugepageRes(NO_NUMA_HUGE_DIR)]
+
+
+def try_reserve_huge_pages(
+    hp_res: T.List[HugepageRes], mem_sz: str, pagesize_kb: int
+) -> None:
+    """Reserve huge pages if possible"""
+    reserve_kb = get_memsize(mem_sz)
+
+    # is this a valid request?
+    if reserve_kb % pagesize_kb != 0:
+        fmt_res = fmt_memsize(reserve_kb)
+        fmt_sz = fmt_memsize(pagesize_kb)
+        raise ValueError(
+            f"Huge reservation {fmt_res} is " f"not a multiple of page size {fmt_sz}"
+        )
+
+    # request is valid, reserve pages
+    for hp in hp_res:
+        req = reserve_kb // pagesize_kb
+        hp[pagesize_kb] = req
+        got = hp[pagesize_kb]
+        # did we fulfill our request?
+        if got != req:
+            raise OSError(
+                f"Failed to reserve {req} pages of size "
+                f"{fmt_memsize(pagesize_kb)}, "
+                f"got {got} pages instead"
+            )
 
 
 def main():
-    '''Process the command line arguments and setup huge pages'''
+    """Process the command line arguments and setup huge pages"""
     parser = argparse.ArgumentParser(
         formatter_class=argparse.RawDescriptionHelpFormatter,
         description="Setup huge pages",
@@ -214,58 +288,68 @@ def main():
 
 To a complete setup of with 2 Gigabyte of 1G huge pages:
     %(prog)s -p 1G --setup 2G
-""")
+""",
+    )
     parser.add_argument(
-        '--show',
-        '-s',
-        action='store_true',
-        help="print the current huge page configuration")
+        "--show",
+        "-s",
+        action="store_true",
+        help="Print current huge page configuration",
+    )
     parser.add_argument(
-        '--clear', '-c', action='store_true', help="clear existing huge pages")
+        "--clear", "-c", action="store_true", help="Clear existing huge pages"
+    )
     parser.add_argument(
-        '--mount',
-        '-m',
-        action='store_true',
-        help='mount the huge page filesystem')
+        "--mount",
+        "-m",
+        action="store_true",
+        help="Mount the huge page filesystem",
+    )
     parser.add_argument(
-        '--unmount',
-        '-u',
-        action='store_true',
-        help='unmount the system huge page directory')
+        "--unmount",
+        "-u",
+        action="store_true",
+        help="Unmount the system huge page directory",
+    )
     parser.add_argument(
-        '--directory',
-        '-d',
-        metavar='DIR',
+        "--directory",
+        "-d",
+        metavar="DIR",
         default=HUGE_MOUNT,
-        help='mount point')
+        help="Mount point for huge pages",
+    )
     parser.add_argument(
-        '--user',
-        '-U',
-        metavar='UID',
-        help='set the mounted directory owner user')
+        "--user",
+        "-U",
+        metavar="UID",
+        help="Set the mounted directory owner user",
+    )
     parser.add_argument(
-        '--group',
-        '-G',
-        metavar='GID',
-        help='set the mounted directory owner group')
+        "--group",
+        "-G",
+        metavar="GID",
+        help="Set the mounted directory owner group",
+    )
     parser.add_argument(
-        '--node', '-n', help='select numa node to reserve pages on')
+        "--node", "-n", type=int, help="Select numa node to reserve pages on"
+    )
     parser.add_argument(
-        '--pagesize',
-        '-p',
-        metavar='SIZE',
-        help='choose huge page size to use')
+        "--pagesize", "-p", metavar="SIZE", help="Choose huge page size to use"
+    )
     parser.add_argument(
-        '--reserve',
-        '-r',
-        metavar='SIZE',
-        help='reserve huge pages. Size is in bytes with K, M, or G suffix')
+        "--reserve",
+        "-r",
+        metavar="SIZE",
+        help="Reserve huge pages. Size is in bytes with K, M, or G suffix",
+    )
     parser.add_argument(
-        '--setup',
-        metavar='SIZE',
-        help='setup huge pages by doing clear, unmount, reserve and mount')
+        "--setup",
+        metavar="SIZE",
+        help="Setup huge pages by doing clear, unmount, reserve and mount",
+    )
     args = parser.parse_args()
 
+    # setup is clear, then unmount, then reserve, then mount
     if args.setup:
         args.clear = True
         args.unmount = True
@@ -275,33 +359,51 @@ def main():
     if not (args.show or args.mount or args.unmount or args.clear or args.reserve):
         parser.error("no action specified")
 
+    # read huge page data from sysfs
+    hp_res = scan_huge_dirs(args.node)
+
+    # read huge page mountpoint data
+    hp_mountpoint = args.directory
+    hp_mounted = hp_mountpoint in get_hugetlbfs_mountpoints()
+    hp_mount = HugepageMount(hp_mountpoint, hp_mounted)
+
+    # get requested page size we will be working with
     if args.pagesize:
         pagesize_kb = get_memsize(args.pagesize)
     else:
         pagesize_kb = default_pagesize()
-    if not pagesize_kb:
-        sys.exit("Invalid page size: {}kB".format(pagesize_kb))
 
+    # were we asked to clear?
     if args.clear:
-        clear_pages()
+        for hp in hp_res:
+            for sz in hp.valid_page_sizes:
+                hp[sz] = 0
+
+    # were we asked to unmount?
     if args.unmount:
-        umount_huge(args.directory)
+        hp_mount.unmount()
 
+    # were we asked to reserve pages?
     if args.reserve:
-        reserve_kb = get_memsize(args.reserve)
-        if reserve_kb % pagesize_kb != 0:
-            sys.exit(
-                'Huge reservation {}kB is not a multiple of page size {}kB'.
-                format(reserve_kb, pagesize_kb))
-        reserve_pages(
-            int(reserve_kb / pagesize_kb), pagesize_kb, node=args.node)
+        try_reserve_huge_pages(hp_res, args.reserve, pagesize_kb)
+
+    # were we asked to mount?
     if args.mount:
-        mount_huge(pagesize_kb, args.directory, args.user, args.group)
+        hp_mount.mount(pagesize_kb, args.user, args.group)
+
+    # were we asked to display status?
     if args.show:
-        show_pages()
+        print_hp_status(hp_res)
         print()
-        show_mount()
+        print_mount_status()
 
 
 if __name__ == "__main__":
-    main()
+    try:
+        main()
+    except PermissionError:
+        sys.exit("Permission denied: need to be root!")
+    except subprocess.CalledProcessError as e:
+        sys.exit(f"Command failed: {e}")
+    except (KeyError, ValueError, OSError) as e:
+        sys.exit(f"Error: {e}")
-- 
2.43.5

[PATCH v6 4/4] usertools/dpdk-devbind: print NUMA node

[Anatoly Burakov]

Currently, devbind does not print out any NUMA information, which makes
figuring out which NUMA node device belongs to not trivial. Add printouts
for NUMA information if NUMA support is enabled on the system.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Robin Jarry <rjarry@redhat.com>
---

Notes:
    v1 -> v2:
    - Added commit to print out NUMA information in devbind

 usertools/dpdk-devbind.py | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index b276e8efc8..078e8c387b 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -110,6 +110,11 @@
 args = []
 
 
+# check if this system has NUMA support
+def is_numa():
+    return os.path.exists('/sys/devices/system/node')
+
+
 # check if a specific kernel module is loaded
 def module_is_loaded(module):
     global loaded_modules
@@ -577,20 +582,28 @@ def show_device_status(devices_type, device_name, if_field=False):
         print("".join('=' * len(msg)))
         return
 
+    print_numa = is_numa()
+
     # print each category separately, so we can clearly see what's used by DPDK
     if dpdk_drv:
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
         display_devices("%s devices using DPDK-compatible driver" % device_name,
-                        dpdk_drv, "drv=%(Driver_str)s unused=%(Module_str)s")
+                        dpdk_drv, extra_param)
     if kernel_drv:
-        if_text = ""
+        extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
         if if_field:
-            if_text = "if=%(Interface)s "
-        display_devices("%s devices using kernel driver" % device_name, kernel_drv,
-                        if_text + "drv=%(Driver_str)s "
-                        "unused=%(Module_str)s %(Active)s")
+            extra_param = "if=%(Interface)s " + extra_param
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("%s devices using kernel driver" % device_name,
+                        kernel_drv, extra_param)
     if no_drv:
-        display_devices("Other %s devices" % device_name, no_drv,
-                        "unused=%(Module_str)s")
+        extra_param = "unused=%(Module_str)s"
+        if print_numa:
+            extra_param = "numa_node=%(NUMANode)s " + extra_param
+        display_devices("Other %s devices" % device_name, no_drv, extra_param)
 
 
 def show_status():
-- 
2.43.5

Re: [PATCH v6 4/4] usertools/dpdk-devbind: print NUMA node

[Stephen Hemminger]

On Thu, 22 Aug 2024 11:38:26 +0100
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> Currently, devbind does not print out any NUMA information, which makes
> figuring out which NUMA node device belongs to not trivial. Add printouts
> for NUMA information if NUMA support is enabled on the system.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Robin Jarry <rjarry@redhat.com>
> ---

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH v6 1/4] usertools/cpu_layout: update coding style

[Stephen Hemminger]

On Thu, 22 Aug 2024 11:38:23 +0100
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> Update coding style:
> 
> - make it PEP-484 compliant
> - format code with Ruff
> - address all mypy etc. warnings
> - use f-strings in place of old-style string interpolation
> - refactor printing to make the code more readable
> - read valid CPU ID's from "online" sysfs node
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Robin Jarry <rjarry@redhat.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>