All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Create more common interfaces in misc and use them
@ 2022-07-26 13:35 John Kacur
  2022-07-26 13:35 ` [PATCH 1/6] rteval: Create common functions in CpuList and SysTopology John Kacur
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: John Kacur @ 2022-07-26 13:35 UTC (permalink / raw)
  To: RT; +Cc: Clark Williams, Leah Leshchinsky, Manasi Godse, John Kacur

The purpose of these patches is to reduce the number of interfaces that
do the same thing and standardize on the functions in systopology. This
has already been to some degree with past patches, but is not complete.

In particular I added the method online_cpus() to class SysTopology
and the function collapse_cpulist() to the file systopology.py

Then I visited each file that imported from misc and made use of the new
interfaces. This leaves only the function cpuinfo() in misc. I have not
yet removed the duplicate functions in misc, but that will be done in a
subsequent set of patches.

While making use of the interfaces in systopology from various other
files, I took the opportunity to clean-up the files, namely I used
f-strings instead of regular strings for readability, added some
docstrings to functions and methods, and made use of the more
pythonesque "in" instead of chained "ifs"


John Kacur (6):
  rteval: Create common  functions in CpuList and SysTopology
  rteval: Make use of systopology instead of misc in rteval-cmd
  rteval: Make use of systopology instead of misc in hackbench
  rteval: Make use of systopology instead of misc in kcompile
  rteval: Make use of systopology instead of misc in stressng module
  rteval: Make use of systopology instead of misc in cyclictest

 rteval-cmd                               | 45 ++++++------
 rteval/modules/loads/hackbench.py        | 30 ++++----
 rteval/modules/loads/kcompile.py         | 86 +++++++++++-----------
 rteval/modules/loads/stressng.py         | 15 ++--
 rteval/modules/measurement/cyclictest.py | 67 ++++++++++--------
 rteval/systopology.py                    | 90 +++++++++++++++++++-----
 6 files changed, 201 insertions(+), 132 deletions(-)

-- 
2.37.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/6] rteval: Create common  functions in CpuList and SysTopology
  2022-07-26 13:35 [PATCH 0/6] Create more common interfaces in misc and use them John Kacur
@ 2022-07-26 13:35 ` John Kacur
  2022-07-27 20:32   ` Leah Leshchinsky
  2022-07-26 13:35 ` [PATCH 2/6] rteval: Make use of systopology instead of misc in rteval-cmd John Kacur
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 8+ messages in thread
From: John Kacur @ 2022-07-26 13:35 UTC (permalink / raw)
  To: RT; +Cc: Clark Williams, Leah Leshchinsky, Manasi Godse, John Kacur

The purpose is to remove functions out of misc and use the ones in the
file systopolgy.py

- Add function collapse_cpulist(cpulist)  outside of the CpuList
  class
- Make methods longest_sequence and expand_cpulist accesible outside of
  their class
- Add function online_cpus(self) to class SysTopology
- Add function online_cpus_str(self) to class SysTopology
- Add function invert_cpulist(self, cpulist) to class SysTopology
- Convert strings to f-strings for better readability
- Add a few missing docstrings to methods / functions, module etc
- Add a few more tests to the unit test

TODO: CpuList is suited for use by SysTopology, but is not ideal for a
generic CpuList. A more generally usable CpuList should be created

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 rteval/systopology.py | 90 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 74 insertions(+), 16 deletions(-)

diff --git a/rteval/systopology.py b/rteval/systopology.py
index e852f86e450f..ce8d02cf7318 100644
--- a/rteval/systopology.py
+++ b/rteval/systopology.py
@@ -23,11 +23,32 @@
 #   including keys needed to generate an equivalently functional executable
 #   are deemed to be part of the source code.
 #
+""" Module for querying cpu cores and nodes """
 
 import os
 import os.path
 import glob
 
+# Utility version of collapse_cpulist that doesn't require a CpuList object
+def collapse_cpulist(cpulist):
+    """ Collapse a list of cpu numbers into a string range
+        of cpus (e.g. 0-5, 7, 9) """
+    if len(cpulist) == 0:
+        return ""
+    idx = CpuList.longest_sequence(cpulist)
+    if idx == 0:
+        seq = str(cpulist[0])
+    else:
+        if idx == 1:
+            seq = f"{cpulist[0]},{cpulist[idx]}"
+        else:
+            seq = f"{cpulist[0]}-{cpulist[idx]}"
+
+    rest = collapse_cpulist(cpulist[idx+1:])
+    if rest == "":
+        return seq
+    return ",".join((seq, rest))
+
 def sysread(path, obj):
     """ Helper function for reading system files """
     with open(os.path.join(path, obj), "r") as fp:
@@ -46,7 +67,7 @@ class CpuList:
         if isinstance(cpulist, list):
             self.cpulist = cpulist
         elif isinstance(cpulist, str):
-            self.cpulist = self.__expand_cpulist(cpulist)
+            self.cpulist = self.expand_cpulist(cpulist)
         self.cpulist = self.online_cpulist(self.cpulist)
         self.cpulist.sort()
 
@@ -67,7 +88,7 @@ class CpuList:
         return False
 
     @staticmethod
-    def __longest_sequence(cpulist):
+    def longest_sequence(cpulist):
         """ return index of last element of a sequence that steps by one """
         lim = len(cpulist)
         for idx, _ in enumerate(cpulist):
@@ -83,14 +104,14 @@ class CpuList:
         """
         if len(cpulist) == 0:
             return ""
-        idx = self.__longest_sequence(cpulist)
+        idx = self.longest_sequence(cpulist)
         if idx == 0:
             seq = str(cpulist[0])
         else:
             if idx == 1:
-                seq = "%d,%d" % (cpulist[0], cpulist[idx])
+                seq = f"{cpulist[0]},{cpulist[idx]}"
             else:
-                seq = "%d-%d" % (cpulist[0], cpulist[idx])
+                seq = f"{cpulist[0]}-{cpulist[idx]}"
 
         rest = self.__collapse_cpulist(cpulist[idx+1:])
         if rest == "":
@@ -98,7 +119,14 @@ class CpuList:
         return ",".join((seq, rest))
 
     @staticmethod
-    def __expand_cpulist(cpulist):
+    def compress_cpulist(cpulist):
+        """ return a string representation of cpulist """
+        if isinstance(cpulist[0], int):
+            return ",".join(str(e) for e in cpulist)
+        return ",".join(cpulist)
+
+    @staticmethod
+    def expand_cpulist(cpulist):
         """ expand a range string into an array of cpu numbers
         don't error check against online cpus
         """
@@ -124,8 +152,8 @@ class CpuList:
     def is_online(self, n):
         """ check whether cpu n is online """
         if n not in self.cpulist:
-            raise RuntimeError("invalid cpu number %d" % n)
-        path = os.path.join(CpuList.cpupath, 'cpu%d' % n)
+            raise RuntimeError(f"invalid cpu number {n}")
+        path = os.path.join(CpuList.cpupath, f'cpu{n}')
 
         # Some hardware doesn't allow cpu0 to be turned off
         if not os.path.exists(path + '/online') and n == 0:
@@ -240,8 +268,8 @@ class SysTopology:
         return len(list(self.nodes.keys()))
 
     def __str__(self):
-        s = "%d node system" % len(list(self.nodes.keys()))
-        s += " (%d cores per node)" % (len(self.nodes[list(self.nodes.keys())[0]]))
+        s = f"{len(list(self.nodes.keys()))} node system "
+        s += f"({(len(self.nodes[list(self.nodes.keys())[0]]))} cores per node)"
         return s
 
     def __contains__(self, node):
@@ -268,6 +296,7 @@ class SysTopology:
         return n
 
     def getinfo(self):
+        """ Initialize class Systopology """
         nodes = glob.glob(os.path.join(SysTopology.nodepath, 'node[0-9]*'))
         if nodes:
             nodes.sort()
@@ -278,27 +307,56 @@ class SysTopology:
             self.nodes[0] = SimNumaNode()
 
     def getnodes(self):
+        """ return a list of nodes """
         return list(self.nodes.keys())
 
     def getcpus(self, node):
+        """ return a dictionary of cpus keyed with the node """
         return self.nodes[node].getcpulist()
 
+    def online_cpus(self):
+        """ return a list of integers of all online cpus """
+        cpulist = []
+        for n in self.nodes:
+            cpulist += self.getcpus(n)
+        cpulist.sort()
+        return cpulist
+
+    def online_cpus_str(self):
+        """ return a list of strings of numbers of all online cpus """
+        cpulist = [str(cpu) for cpu in self.online_cpus()]
+        return cpulist
+
+    def invert_cpulist(self, cpulist):
+        """ return a list of online cpus not in cpulist """
+        return [c for c in self.online_cpus_str() if c not in cpulist]
 
 if __name__ == "__main__":
 
     def unit_test():
+        """ unit test, run python rteval/systopology.py """
         s = SysTopology()
         print(s)
-        print("number of nodes: %d" % len(s))
+        print(f"number of nodes: {len(s)}")
         for n in s:
-            print("node[%d]: %s" % (n.nodeid, n))
-        print("system has numa node 0: %s" % (0 in s))
-        print("system has numa node 2: %s" % (2 in s))
-        print("system has numa node 24: %s" % (24 in s))
+            print(f"node[{n.nodeid}]: {n}")
+        print(f"system has numa node 0: {0 in s}")
+        print(f"system has numa node 2: {2 in s}")
+        print(f"system has numa node 24: {24 in s}")
 
         cpus = {}
+        print(f"nodes = {s.getnodes()}")
         for node in s.getnodes():
             cpus[node] = s.getcpus(int(node))
-        print(f'cpus = {cpus}')
+            print(f'cpus = {cpus}')
+
+        onlcpus = s.online_cpus()
+        print(f'onlcpus = {onlcpus}')
+        onlcpus = collapse_cpulist(onlcpus)
+        print(f'onlcpus = {onlcpus}')
+
+        onlcpus_str = s.online_cpus_str()
+        print(f'onlcpus_str = {onlcpus_str}')
 
+        print(f"invert of [ 2, 4, 5 ] = {s.invert_cpulist([2, 3, 4])}")
     unit_test()
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/6] rteval: Make use of systopology instead of misc in rteval-cmd
  2022-07-26 13:35 [PATCH 0/6] Create more common interfaces in misc and use them John Kacur
  2022-07-26 13:35 ` [PATCH 1/6] rteval: Create common functions in CpuList and SysTopology John Kacur
@ 2022-07-26 13:35 ` John Kacur
  2022-07-26 13:35 ` [PATCH 3/6] rteval: Make use of systopology instead of misc in hackbench John Kacur
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: John Kacur @ 2022-07-26 13:35 UTC (permalink / raw)
  To: RT; +Cc: Clark Williams, Leah Leshchinsky, Manasi Godse, John Kacur

- convert rteval-cmd to use methods / functions in systopology instead of misc.
- strings converted to f-strings

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 rteval-cmd | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/rteval-cmd b/rteval-cmd
index c1a68bd5133b..24ff5df76883 100755
--- a/rteval-cmd
+++ b/rteval-cmd
@@ -33,6 +33,7 @@
 #   including keys needed to generate an equivalently functional executable
 #   are deemed to be part of the source code.
 #
+""" Main module of the rteval program """
 
 import sys
 import os
@@ -48,9 +49,11 @@ from rteval import RtEval, rtevalConfig
 from rteval.modules.loads import LoadModules
 from rteval.modules.measurement import MeasurementModules
 from rteval.version import RTEVAL_VERSION
-from rteval.misc import invert_cpulist, compress_cpulist
+from rteval.systopology import CpuList, SysTopology
 from rteval.modules.loads.kcompile import ModuleParameters
 
+compress_cpulist = CpuList.compress_cpulist
+
 def summarize(repfile, xslt):
     """ Summarize an already existing XML report """
     isarchive = False
@@ -60,7 +63,7 @@ def summarize(repfile, xslt):
         try:
             t = tarfile.open(repfile)
         except:
-            print("Don't know how to summarize %s (tarfile open failed)" % repfile)
+            print(f"Don't know how to summarize {repfile} (tarfile open failed)")
             return
         element = None
         for f in t.getnames():
@@ -68,7 +71,7 @@ def summarize(repfile, xslt):
                 element = f
                 break
         if element is None:
-            print("No summary.xml found in tar archive %s" % repfile)
+            print(f"No summary.xml found in tar archive {repfile}")
             return
         tmp = tempfile.gettempdir()
         t.extract(element, path=tmp)
@@ -172,7 +175,7 @@ def parse_options(cfg, parser, cmdargs):
 
     (cmd_opts, cmd_args) = parser.parse_args(args=cmdargs)
     if cmd_opts.rteval___version:
-        print(("rteval version %s" % RTEVAL_VERSION))
+        print(f"rteval version {RTEVAL_VERSION}")
         sys.exit(0)
 
     if cmd_opts.rteval___duration:
@@ -320,11 +323,13 @@ if __name__ == '__main__':
         ldcfg = config.GetSection('loads')
         msrcfg = config.GetSection('measurement')
         if not ldcfg.cpulist and msrcfg.cpulist:
-            ldcfg.cpulist = compress_cpulist(invert_cpulist(msrcfg.cpulist))
+            invlist = SysTopology().invert_cpulist(msrcfg.cpulist)
+            ldcfg.cpulist = compress_cpulist(invlist)
         if not msrcfg.cpulist and ldcfg.cpulist:
-            msrcfg.cpulist = compress_cpulist(invert_cpulist(ldcfg.cpulist))
+            invlist = SysTopology().invert_cpulist(ldcfg.cpulist)
+            msrcfg.cpulist = compress_cpulist(invlist)
 
-        logger.log(Log.DEBUG, "workdir: %s" % rtevcfg.workdir)
+        logger.log(Log.DEBUG, f"workdir: {rtevcfg.workdir}")
 
         # if --summarize was specified then just parse the XML, print it and exit
         if rtevcfg.summarize or rtevcfg.rawhistogram:
@@ -343,22 +348,18 @@ if __name__ == '__main__':
             print("Must be root to run rteval!")
             sys.exit(-1)
 
-        logger.log(Log.DEBUG, '''rteval options:
-     workdir: %s
-     loaddir: %s
-     reportdir: %s
-     verbose: %s
-     debugging: %s
-     logging:  %s
-     duration: %f
-     sysreport: %s''' % (
-         rtevcfg.workdir, rtevcfg.srcdir,
-         rtevcfg.reportdir, rtevcfg.verbose,
-         rtevcfg.debugging, rtevcfg.logging,
-         rtevcfg.duration, rtevcfg.sysreport))
+        logger.log(Log.DEBUG, f'''rteval options:
+     workdir: {rtevcfg.workdir}
+     loaddir: {rtevcfg.srcdir}
+     reportdir: {rtevcfg.reportdir}
+     verbose: {rtevcfg.verbose}
+     debugging: {rtevcfg.debugging}
+     logging:  {rtevcfg.logging}
+     duration: {rtevcfg.duration}
+     sysreport: {rtevcfg.sysreport}''')
 
         if not os.path.isdir(rtevcfg.workdir):
-            raise RuntimeError("work directory %s does not exist" % rtevcfg.workdir)
+            raise RuntimeError(f"work directory {rtevcfg.workdir} does not exist")
 
 
         rteval = RtEval(config, loadmods, measuremods, logger)
@@ -378,7 +379,7 @@ if __name__ == '__main__':
         else:
             # ... otherwise, run the full measurement suite with loads
             ec = rteval.Measure()
-            logger.log(Log.DEBUG, "exiting with exit code: %d" % ec)
+            logger.log(Log.DEBUG, f"exiting with exit code: {ec}")
 
         sys.exit(ec)
     except KeyboardInterrupt:
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/6] rteval: Make use of systopology instead of misc in hackbench
  2022-07-26 13:35 [PATCH 0/6] Create more common interfaces in misc and use them John Kacur
  2022-07-26 13:35 ` [PATCH 1/6] rteval: Create common functions in CpuList and SysTopology John Kacur
  2022-07-26 13:35 ` [PATCH 2/6] rteval: Make use of systopology instead of misc in rteval-cmd John Kacur
@ 2022-07-26 13:35 ` John Kacur
  2022-07-26 13:35 ` [PATCH 4/6] rteval: Make use of systopology instead of misc in kcompile John Kacur
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: John Kacur @ 2022-07-26 13:35 UTC (permalink / raw)
  To: RT; +Cc: Clark Williams, Leah Leshchinsky, Manasi Godse, John Kacur

- Make use of systopology instead of misc in hackbench
- Add a module docstring
- Make use of f-strings

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 rteval/modules/loads/hackbench.py | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/rteval/modules/loads/hackbench.py b/rteval/modules/loads/hackbench.py
index ddd1378bac75..45bcd546e3b3 100644
--- a/rteval/modules/loads/hackbench.py
+++ b/rteval/modules/loads/hackbench.py
@@ -24,6 +24,7 @@
 #   including keys needed to generate an equivalently functional executable
 #   are deemed to be part of the source code.
 #
+""" Load module - run the hackbench program from rt-tests ad a load """
 
 import sys
 import os
@@ -34,8 +35,9 @@ import errno
 from signal import SIGKILL
 from rteval.modules.loads import CommandLineLoad
 from rteval.Log import Log
-from rteval.misc import expand_cpulist
-from rteval.systopology import SysTopology
+from rteval.systopology import CpuList, SysTopology
+
+expand_cpulist = CpuList.expand_cpulist
 
 class Hackbench(CommandLineLoad):
     def __init__(self, config, logger):
@@ -46,7 +48,7 @@ class Hackbench(CommandLineLoad):
         if self._donotrun:
             return
 
-        'calculate arguments based on input parameters'
+        # calculate arguments based on input parameters
         (mem, units) = self.memsize
         if units == 'KB':
             mem = mem / (1024.0 * 1024.0)
@@ -58,9 +60,9 @@ class Hackbench(CommandLineLoad):
         ratio = float(mem) / float(self.num_cpus)
         if ratio < 0.75:
             if self.__cfg.runlowmem:
-                self._log(Log.WARN, "Low memory system (%f GB/core)!" % ratio)
+                self._log(Log.WARN, f"Low memory system ({ratio} GB/core)!")
             else:
-                self._log(Log.WARN, "Low memory system (%f GB/core)! Not running hackbench" % ratio)
+                self._log(Log.WARN, f"Low memory system ({ratio} GB/core)! Not running hackbench")
                 self._donotrun = True
 
         sysTop = SysTopology()
@@ -85,7 +87,7 @@ class Hackbench(CommandLineLoad):
         for node, cpus in list(self.cpus.items()):
             if not cpus:
                 self.nodes.remove(node)
-                self._log(Log.DEBUG, "node %s has no available cpus, removing" % node)
+                self._log(Log.DEBUG, f"node {node} has no available cpus, removing")
 
         # setup jobs based on the number of cores available per node
         self.jobs = biggest * 3
@@ -95,7 +97,7 @@ class Hackbench(CommandLineLoad):
         self.__multinodes = False
         if len(self.nodes) > 1:
             self.__multinodes = True
-            self._log(Log.INFO, "running with multiple nodes (%d)" % len(self.nodes))
+            self._log(Log.INFO, f"running with multiple nodes ({len(self.nodes)})")
             if os.path.exists('/usr/bin/numactl') and not self.cpulist:
                 self.__usenumactl = True
                 self._log(Log.INFO, "using numactl for thread affinity")
@@ -121,7 +123,7 @@ class Hackbench(CommandLineLoad):
 
         self.tasks = {}
 
-        self._log(Log.DEBUG, "starting loop (jobs: %d)" % self.jobs)
+        self._log(Log.DEBUG, f"starting loop (jobs: {self.jobs})")
 
         self.started = False
 
@@ -135,14 +137,14 @@ class Hackbench(CommandLineLoad):
         else:
             args = self.args
 
-        self._log(Log.DEBUG, "starting on node %s: args = %s" % (node, args))
+        self._log(Log.DEBUG, "starting on node node{}: args = {args}")
         p = subprocess.Popen(args,
                              stdin=self.__nullfp,
                              stdout=self.__out,
                              stderr=self.__err)
         if not p:
-            self._log(Log.DEBUG, "hackbench failed to start on node %s" % node)
-            raise RuntimeError("hackbench failed to start on node %s" % node)
+            self._log(Log.DEBUG, f"hackbench failed to start on node {node}")
+            raise RuntimeError(f"hackbench failed to start on node {node}")
         return p
 
     def _WorkloadTask(self):
@@ -181,7 +183,7 @@ class Hackbench(CommandLineLoad):
 
         for node in self.nodes:
             if node in self.tasks and self.tasks[node].poll() is None:
-                self._log(Log.INFO, "cleaning up hackbench on node %s" % node)
+                self._log(Log.INFO, f"cleaning up hackbench on node {node}")
                 self.tasks[node].send_signal(SIGKILL)
                 if self.tasks[node].poll() is None:
                     time.sleep(2)
@@ -213,7 +215,3 @@ def ModuleParameters():
 def create(config, logger):
     return Hackbench(config, logger)
 
-# TODO: The following test is broken
-#if __name__ == '__main__':
-#    h = Hackbench(params={'debugging':True, 'verbose':True})
-#    h.run()
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/6] rteval: Make use of systopology instead of misc in kcompile
  2022-07-26 13:35 [PATCH 0/6] Create more common interfaces in misc and use them John Kacur
                   ` (2 preceding siblings ...)
  2022-07-26 13:35 ` [PATCH 3/6] rteval: Make use of systopology instead of misc in hackbench John Kacur
@ 2022-07-26 13:35 ` John Kacur
  2022-07-26 13:35 ` [PATCH 5/6] rteval: Make use of systopology instead of misc in stressng module John Kacur
  2022-07-26 13:35 ` [PATCH 6/6] rteval: Make use of systopology instead of misc in cyclictest John Kacur
  5 siblings, 0 replies; 8+ messages in thread
From: John Kacur @ 2022-07-26 13:35 UTC (permalink / raw)
  To: RT; +Cc: Clark Williams, Leah Leshchinsky, Manasi Godse, John Kacur

- make use of systopology instead of misc in kcompile
- use f-strings where necessary
- Change """ to comment style # when not a docstring
- Add docstrings to functions
- Use python in (0, -2) for readability

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 rteval/modules/loads/kcompile.py | 86 +++++++++++++++++---------------
 1 file changed, 46 insertions(+), 40 deletions(-)

diff --git a/rteval/modules/loads/kcompile.py b/rteval/modules/loads/kcompile.py
index 2c93c794fe75..4a8659c042e6 100644
--- a/rteval/modules/loads/kcompile.py
+++ b/rteval/modules/loads/kcompile.py
@@ -33,8 +33,10 @@ import subprocess
 from rteval.modules import rtevalRuntimeError
 from rteval.modules.loads import CommandLineLoad
 from rteval.Log import Log
-from rteval.misc import expand_cpulist, compress_cpulist
-from rteval.systopology import SysTopology
+from rteval.systopology import CpuList, SysTopology
+
+expand_cpulist = CpuList.expand_cpulist
+compress_cpulist = CpuList.compress_cpulist
 
 DEFAULT_KERNEL_PREFIX = "linux-5.18"
 
@@ -48,21 +50,21 @@ class KBuildJob:
         self.logger = logger
         self.binder = None
         self.builddir = os.path.dirname(kdir)
-        self.objdir = "%s/node%d" % (self.builddir, int(node))
+        self.objdir = f"{self.builddir}/node{int(node)}"
 
         if not os.path.isdir(self.objdir):
             os.mkdir(self.objdir)
 
         if os.path.exists('/usr/bin/numactl') and not cpulist:
-            """ Use numactl """
-            self.binder = 'numactl --cpunodebind %d' % int(self.node)
+            # Use numactl
+            self.binder = f'numactl --cpunodebind {int(self.node)}'
             self.jobs = self.calc_jobs_per_cpu() * len(self.node)
         elif cpulist:
-            """ Use taskset """
+            # Use taskset
             self.jobs = self.calc_jobs_per_cpu() * len(cpulist)
-            self.binder = 'taskset -c %s' % compress_cpulist(cpulist)
+            self.binder = f'taskset -c {compress_cpulist(cpulist)}'
         else:
-            """ Without numactl calculate number of jobs from the node """
+            # Without numactl calculate number of jobs from the node
             self.jobs = self.calc_jobs_per_cpu() * len(self.node)
 
         self.runcmd = f"make O={self.objdir} -C {self.kdir} -j{self.jobs}"
@@ -72,56 +74,61 @@ class KBuildJob:
             self.runcmd = self.binder + " " + self.runcmd
             self.cleancmd = self.binder + " " + self.cleancmd
 
-        self.log(Log.DEBUG, "node %d: jobs == %d" % (int(node), self.jobs))
+        self.log(Log.DEBUG, f"node {int(node)}: jobs == {self.jobs}")
         self.log(Log.DEBUG, f"cleancmd = {self.cleancmd}")
-        self.log(Log.DEBUG, "node%d kcompile command: %s" \
-                % (int(node), self.runcmd))
+        self.log(Log.DEBUG, f"node{int(node)} kcompile command: {self.runcmd}")
 
     def __str__(self):
         return self.runcmd
 
     def log(self, logtype, msg):
+        """ starting logging for the kcompile module """
         if self.logger:
-            self.logger.log(logtype, "[kcompile node%d] %s" % (int(self.node), msg))
+            self.logger.log(logtype, f"[kcompile node{int(self.node)}] {msg}")
 
     def calc_jobs_per_cpu(self):
+        """ Calculate the number of kcompile jobs to do """
         mult = 2
-        self.log(Log.DEBUG, "calulating jobs for node %d" % int(self.node))
+        self.log(Log.DEBUG, f"calulating jobs for node {int(self.node)}")
         # get memory total in gigabytes
         mem = int(self.node.meminfo['MemTotal']) / 1024.0 / 1024.0 / 1024.0
         # ratio of gigabytes to #cores
         ratio = float(mem) / float(len(self.node))
-        if ratio < 1.0:
-            ratio = 1.0
-        if ratio < 1.0 or ratio > 2.0:
+        ratio = max(ratio, 1.0)
+        if ratio > 2.0:
             mult = 1
-        self.log(Log.DEBUG, "memory/cores ratio on node %d: %f" % (int(self.node), ratio))
-        self.log(Log.DEBUG, "returning jobs/core value of: %d" % int(ratio) * mult)
+        self.log(Log.DEBUG, f"memory/cores ratio on node {int(self.node)}: {ratio}")
+        self.log(Log.DEBUG, f"returning jobs/core value of: {int(ratio) * mult}")
         return int(int(ratio) * int(mult))
 
     def clean(self, sin=None, sout=None, serr=None):
-        self.log(Log.DEBUG, "cleaning objdir %s" % self.objdir)
+        """ Runs command to clean any previous builds and configure kernel """
+        self.log(Log.DEBUG, f"cleaning objdir {self.objdir}")
         subprocess.call(self.cleancmd, shell=True,
                         stdin=sin, stdout=sout, stderr=serr)
 
     def run(self, sin=None, sout=None, serr=None):
-        self.log(Log.INFO, "starting workload on node %d" % int(self.node))
-        self.log(Log.DEBUG, "running on node %d: %s" % (int(self.node), self.runcmd))
+        """ Use Popen to launch a kcompile job """
+        self.log(Log.INFO, f"starting workload on node {int(self.node)}")
+        self.log(Log.DEBUG, f"running on node {int(self.node)}: {self.runcmd}")
         self.jobid = subprocess.Popen(self.runcmd, shell=True,
                                       stdin=sin, stdout=sout, stderr=serr)
 
     def isrunning(self):
+        """ Query whether a job is running, returns True or False """
         if self.jobid is None:
             return False
         return self.jobid.poll() is None
 
     def stop(self):
+        """ stop a kcompile job """
         if not self.jobid:
             return True
         return self.jobid.terminate()
 
 
 class Kcompile(CommandLineLoad):
+    """ class to compile the kernel as an rteval load """
     def __init__(self, config, logger):
         self.buildjobs = {}
         self.config = config
@@ -150,14 +157,14 @@ class Kcompile(CommandLineLoad):
     def _remove_build_dirs(self):
         if not os.path.isdir(self.builddir):
             return
-        self._log(Log.DEBUG, "removing kcompile directories in %s" % self.builddir)
+        self._log(Log.DEBUG, f"removing kcompile directories in {self.builddir}")
         null = os.open("/dev/null", os.O_RDWR)
         cmd = ["rm", "-rf", os.path.join(self.builddir, "kernel*"),
                os.path.join(self.builddir, "node*")]
         ret = subprocess.call(cmd, stdin=null, stdout=null, stderr=null)
         if ret:
             raise rtevalRuntimeError(self, \
-                "error removing builddir (%s) (ret=%d)" % (self.builddir, ret))
+                f"error removing builddir ({self.buildir}) (ret={ret})")
 
     def _WorkloadSetup(self):
         if self._donotrun:
@@ -167,15 +174,15 @@ class Kcompile(CommandLineLoad):
         if self._cfg.source:
             tarfile = os.path.join(self.srcdir, self._cfg.source)
             if not os.path.exists(tarfile):
-                raise rtevalRuntimeError(self, " tarfile %s does not exist!" % tarfile)
+                raise rtevalRuntimeError(self, f" tarfile {tarfile} does not exist!")
             self.source = tarfile
             kernel_prefix = re.search(r"linux-\d{1,2}\.\d{1,3}", self.source).group(0)
         else:
-            tarfiles = glob.glob(os.path.join(self.srcdir, "%s*" % DEFAULT_KERNEL_PREFIX))
+            tarfiles = glob.glob(os.path.join(self.srcdir, f"{DEFAULT_KERNEL_PREFIX}*"))
             if tarfiles:
                 self.source = tarfiles[0]
             else:
-                raise rtevalRuntimeError(self, " no kernel tarballs found in %s" % self.srcdir)
+                raise rtevalRuntimeError(self, f" no kernel tarballs found in {self.srcdir}")
             kernel_prefix = DEFAULT_KERNEL_PREFIX
         self._log(Log.DEBUG, f"kernel_prefix = {kernel_prefix}")
 
@@ -190,15 +197,15 @@ class Kcompile(CommandLineLoad):
             self._extract_tarball()
             names = os.listdir(self.builddir)
             for d in names:
-                self._log(Log.DEBUG, "checking %s" % d)
+                self._log(Log.DEBUG, f"checking {d}")
                 if d.startswith(kernel_prefix):
                     kdir = d
                     break
         if kdir is None:
             raise rtevalRuntimeError(self, "Can't find kernel directory!")
         self.mydir = os.path.join(self.builddir, kdir)
-        self._log(Log.DEBUG, "mydir = %s" % self.mydir)
-        self._log(Log.DEBUG, "systopology: %s" % self.topology)
+        self._log(Log.DEBUG, f"mydir = {self.mydir}")
+        self._log(Log.DEBUG, f"systopology: {self.topology}")
         self.jobs = len(self.topology)
         self.args = []
 
@@ -217,10 +224,10 @@ class Kcompile(CommandLineLoad):
         for node, cpus in self.cpus.items():
             if not cpus:
                 self.nodes.remove(node)
-                self._log(Log.DEBUG, "node %s has no available cpus, removing" % node)
+                self._log(Log.DEBUG, f"node {node} has no available cpus, removing")
 
         for n in self.nodes:
-            self._log(Log.DEBUG, "Configuring build job for node %d" % int(n))
+            self._log(Log.DEBUG, f"Configuring build job for node {int(n)}")
             self.buildjobs[n] = KBuildJob(self.topology[n], self.mydir, \
                 self.logger, self.cpus[n] if self.cpulist else None)
             self.args.append(str(self.buildjobs[n])+";")
@@ -249,7 +256,7 @@ class Kcompile(CommandLineLoad):
                 ret = subprocess.call(cmd, stdin=null, stdout=out, stderr=err)
                 if ret:
                     # give up
-                    raise rtevalRuntimeError(self, "kcompile setup failed: %d" % ret)
+                    raise rtevalRuntimeError(self, f"kcompile setup failed: {ret}")
         except KeyboardInterrupt as m:
             self._log(Log.DEBUG, "keyboard interrupt, aborting")
             return
@@ -280,15 +287,14 @@ class Kcompile(CommandLineLoad):
     def _WorkloadTask(self):
         for n in self.nodes:
             if not self.buildjobs[n]:
-                raise RuntimeError("Build job not set up for node %d" % int(n))
+                raise RuntimeError(f"Build job not set up for node {int(n)}")
             if self.buildjobs[n].jobid is None or self.buildjobs[n].jobid.poll() is not None:
                 # A jobs was started, but now it finished. Check return code.
                 # -2 is returned when user forced stop of execution (CTRL-C).
                 if self.buildjobs[n].jobid is not None:
-                    if self.buildjobs[n].jobid.returncode != 0 and self.buildjobs[n].jobid.returncode != -2:
-                        raise RuntimeError("kcompile module failed to run (returned %d), please check logs for more detail" \
-                            % self.buildjobs[n].jobid.returncode)
-                self._log(Log.INFO, "Starting load on node %d" % n)
+                    if self.buildjobs[n].jobid.returncode not in (0, -2):
+                        raise RuntimeError(f"kcompile module failed to run (returned {self.buildjobs[n].jobid.returncode}), please check logs for more detail")
+                self._log(Log.INFO, f"Starting load on node {n}")
                 self.buildjobs[n].run(self.__nullfd, self.__outfd, self.__errfd)
 
     def WorkloadAlive(self):
@@ -296,8 +302,8 @@ class Kcompile(CommandLineLoad):
         for n in self.nodes:
             if self.buildjobs[n].jobid.poll() is not None:
                 # Check return code (see above).
-                if self.buildjobs[n].jobid.returncode != 0 and self.buildjobs[n].jobid.returncode != -2:
-                    raise RuntimeError("kcompile module failed to run (returned %d), please check logs for more detail" % self.buildjobs[n].jobid.returncode)
+                if self.buildjobs[n].jobid.returncode not in (0, -2):
+                    raise RuntimeError(f"kcompile module failed to run (returned {self.buildjobs[n].jobid.returncode}), please check logs for more detail")
                 return False
 
         return True
@@ -310,7 +316,7 @@ class Kcompile(CommandLineLoad):
         self._log(Log.DEBUG, "out of stopevent loop")
         for n in self.buildjobs:
             if self.buildjobs[n].jobid.poll() is None:
-                self._log(Log.DEBUG, "stopping job on node %d" % int(n))
+                self._log(Log.DEBUG, f"stopping job on node {int(n)}")
                 self.buildjobs[n].jobid.terminate()
                 self.buildjobs[n].jobid.wait()
                 del self.buildjobs[n].jobid
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/6] rteval: Make use of systopology instead of misc in stressng module
  2022-07-26 13:35 [PATCH 0/6] Create more common interfaces in misc and use them John Kacur
                   ` (3 preceding siblings ...)
  2022-07-26 13:35 ` [PATCH 4/6] rteval: Make use of systopology instead of misc in kcompile John Kacur
@ 2022-07-26 13:35 ` John Kacur
  2022-07-26 13:35 ` [PATCH 6/6] rteval: Make use of systopology instead of misc in cyclictest John Kacur
  5 siblings, 0 replies; 8+ messages in thread
From: John Kacur @ 2022-07-26 13:35 UTC (permalink / raw)
  To: RT; +Cc: Clark Williams, Leah Leshchinsky, Manasi Godse, John Kacur

- rteval: Make use of systopology instead of misc in stressng module
- make use of f-strings instead of regular strings

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 rteval/modules/loads/stressng.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/rteval/modules/loads/stressng.py b/rteval/modules/loads/stressng.py
index fe97189d3816..287f4e232d17 100644
--- a/rteval/modules/loads/stressng.py
+++ b/rteval/modules/loads/stressng.py
@@ -6,8 +6,9 @@ import subprocess
 import signal
 from rteval.modules.loads import CommandLineLoad
 from rteval.Log import Log
-from rteval.misc import expand_cpulist
-from rteval.systopology import SysTopology
+from rteval.systopology import CpuList, SysTopology
+
+expand_cpulist = CpuList.expand_cpulist
 
 class Stressng(CommandLineLoad):
     " This class creates a load module that runs stress-ng "
@@ -27,7 +28,7 @@ class Stressng(CommandLineLoad):
             self._donotrun = False
         else:
             self._donotrun = True
-        """ When this module runs, other load modules should not """
+        # When this module runs, other load modules should not
         self._exclusive = True
 
     def _WorkloadSetup(self):
@@ -50,7 +51,7 @@ class Stressng(CommandLineLoad):
 
         # stress-ng is only run if the user specifies an option
         self.args = ['stress-ng']
-        self.args.append('--%s' % str(self.cfg.option))
+        self.args.append(f'--{str(self.cfg.option)}')
         if self.cfg.arg is not None:
             self.args.append(self.cfg.arg)
         if self.cfg.timeout is not None:
@@ -73,11 +74,11 @@ class Stressng(CommandLineLoad):
         for node, cpu in cpus.items():
             if not cpu:
                 nodes.remove(node)
-                self._log(Log.DEBUG, "node %s has no available cpus, removing" % node)
+                self._log(Log.DEBUG, f"node {node} has no available cpus, removing")
         if self.cpulist:
             for node in nodes:
                 cpulist = ",".join([str(n) for n in cpus[node]])
-                self.args.append('--taskset %s' % cpulist)
+                self.args.append(f'--taskset {cpulist}')
 
     def _WorkloadTask(self):
         """ Kick of the workload here """
@@ -85,7 +86,7 @@ class Stressng(CommandLineLoad):
             # Only start the task once
             return
 
-        self._log(Log.DEBUG, "starting with %s" % " ".join(self.args))
+        self._log(Log.DEBUG, f'starting with {" ".join(self.args)}')
         try:
             self.process = subprocess.Popen(self.args,
                                             stdout=self.__out,
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/6] rteval: Make use of systopology instead of misc in cyclictest
  2022-07-26 13:35 [PATCH 0/6] Create more common interfaces in misc and use them John Kacur
                   ` (4 preceding siblings ...)
  2022-07-26 13:35 ` [PATCH 5/6] rteval: Make use of systopology instead of misc in stressng module John Kacur
@ 2022-07-26 13:35 ` John Kacur
  5 siblings, 0 replies; 8+ messages in thread
From: John Kacur @ 2022-07-26 13:35 UTC (permalink / raw)
  To: RT; +Cc: Clark Williams, Leah Leshchinsky, Manasi Godse, John Kacur

- Make use of systopology instead of misc in cyclictest
- Use f-strings instead of regular strings
- Use "with" for an open

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 rteval/modules/measurement/cyclictest.py | 67 +++++++++++++-----------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/rteval/modules/measurement/cyclictest.py b/rteval/modules/measurement/cyclictest.py
index 4c8d510c4a34..9e7f4ba25eab 100644
--- a/rteval/modules/measurement/cyclictest.py
+++ b/rteval/modules/measurement/cyclictest.py
@@ -35,7 +35,10 @@ import math
 import libxml2
 from rteval.Log import Log
 from rteval.modules import rtevalModulePrototype
-from rteval.misc import expand_cpulist, online_cpus, cpuinfo
+from rteval.misc import cpuinfo
+from rteval.systopology import CpuList, SysTopology
+
+expand_cpulist = CpuList.expand_cpulist
 
 class RunData:
     '''class to keep instance data from a cyclictest run'''
@@ -58,14 +61,14 @@ class RunData:
         self._log = logfnc
 
     def __str__(self):
-        retval = "id:         %s\n" % self.__id
-        retval += "type:       %s\n" % self.__type
-        retval += "numsamples: %d\n" % self.__numsamples
-        retval += "min:        %d\n" % self.__min
-        retval += "max:        %d\n" % self.__max
-        retval += "stddev:     %f\n" % self.__stddev
-        retval += "mad:        %f\n" % self.__mad
-        retval += "mean:       %f\n" % self.__mean
+        retval = f"id:         {self.__id}\n"
+        retval += f"type:       {self.__type}\n"
+        retval += f"numsamples: {self.__numsamples}\n"
+        retval += f"min:        {self.__min}\n"
+        retval += f"max:        {self.__max}\n"
+        retval += f"stddev:     {self.__stddev}\n"
+        retval += f"mad:        {self.__mad}\n"
+        retval += f"mean:       {self.__mean}\n"
         return retval
 
     def get_max(self):
@@ -98,12 +101,12 @@ class RunData:
         # only have 1 (or none) set the calculated values
         # to zero and return
         if self.__numsamples <= 1:
-            self._log(Log.DEBUG, "skipping %s (%d samples)" % (self.__id, self.__numsamples))
+            self._log(Log.DEBUG, f"skipping {self.__id} ({self.__numsamples} samples)")
             self.__mad = 0
             self.__stddev = 0
             return
 
-        self._log(Log.INFO, "reducing %s" % self.__id)
+        self._log(Log.INFO, f"reducing {self.__id}")
         total = 0
         keys = list(self.__samples.keys())
         keys.sort()
@@ -198,6 +201,7 @@ class RunData:
 
 
 class Cyclictest(rtevalModulePrototype):
+    """ measurement module for rteval """
     def __init__(self, config, logger=None):
         rtevalModulePrototype.__init__(self, 'measurement', 'cyclictest', logger)
         self.__cfg = config
@@ -214,9 +218,12 @@ class Cyclictest(rtevalModulePrototype):
         if self.__cfg.cpulist:
             self.__cpulist = self.__cfg.cpulist
             self.__cpus = expand_cpulist(self.__cpulist)
+            # Only include online cpus
+            self.__cpus = CpuList(self.__cpus).cpulist
+            self.__cpus = [str(c) for c in self.__cpus]
             self.__sparse = True
         else:
-            self.__cpus = online_cpus()
+            self.__cpus = SysTopology().online_cpus_str()
             # Get the cpuset from the environment
             cpuset = os.sched_getaffinity(0)
             # Convert the elements to strings
@@ -241,12 +248,12 @@ class Cyclictest(rtevalModulePrototype):
         self.__cyclicdata['system'] = RunData('system',
                                               'system', self.__priority,
                                               logfnc=self._log)
-        self.__cyclicdata['system'].description = ("(%d cores) " % self.__numcores) + info['0']['model name']
+        self.__cyclicdata['system'].description = (f"({self.__numcores} cores) ") + info['0']['model name']
 
         if self.__sparse:
-            self._log(Log.DEBUG, "system using %d cpu cores" % self.__numcores)
+            self._log(Log.DEBUG, f"system using {self.__numcores} cpu cores")
         else:
-            self._log(Log.DEBUG, "system has %d cpu cores" % self.__numcores)
+            self._log(Log.DEBUG, f"system has {self.__numcores} cpu cores")
         self.__started = False
         self.__cyclicoutput = None
         self.__breaktraceval = None
@@ -273,30 +280,30 @@ class Cyclictest(rtevalModulePrototype):
 
 
     def _WorkloadPrepare(self):
-        self.__interval = 'interval' in self.__cfg and '-i%d' % int(self.__cfg.interval) or ""
+        self.__interval = 'interval' in self.__cfg and f'-i{int(self.__cfg.interval)}' or ""
 
         self.__cmd = ['cyclictest',
                       self.__interval,
                       '-qmu',
-                      '-h %d' % self.__buckets,
-                      "-p%d" % int(self.__priority),
+                      f'-h {self.__buckets}',
+                      f"-p{int(self.__priority)}",
                       ]
         if self.__sparse:
-            self.__cmd.append('-t%d' % self.__numcores)
-            self.__cmd.append('-a%s' % self.__cpulist)
+            self.__cmd.append(f'-t{self.__numcores}')
+            self.__cmd.append(f'-a{self.__cpulist}')
         else:
             self.__cmd.append('-t')
             self.__cmd.append('-a')
 
         if 'threads' in self.__cfg and self.__cfg.threads:
-            self.__cmd.append("-t%d" % int(self.__cfg.threads))
+            self.__cmd.append(f"-t{int(self.__cfg.threads)}")
 
         # Should have either breaktrace or threshold, not both
         if 'breaktrace' in self.__cfg and self.__cfg.breaktrace:
-            self.__cmd.append("-b%d" % int(self.__cfg.breaktrace))
+            self.__cmd.append(f"-b{int(self.__cfg.breaktrace)}")
             self.__cmd.append("--tracemark")
         elif self.__cfg.threshold:
-            self.__cmd.append("-b%d" % int(self.__cfg.threshold))
+            self.__cmd.append(f"-b{int(self.__cfg.threshold)}")
 
         # Buffer for cyclictest data written to stdout
         self.__cyclicoutput = tempfile.SpooledTemporaryFile(mode='w+b')
@@ -307,17 +314,16 @@ class Cyclictest(rtevalModulePrototype):
             # Don't restart cyclictest if it is already runing
             return
 
-        self._log(Log.DEBUG, "starting with cmd: %s" % " ".join(self.__cmd))
+        self._log(Log.DEBUG, f'starting with cmd: {" ".join(self.__cmd)}')
         self.__nullfp = os.open('/dev/null', os.O_RDWR)
 
         debugdir = self.__get_debugfs_mount()
         if 'breaktrace' in self.__cfg and self.__cfg.breaktrace and debugdir:
             # Ensure that the trace log is clean
             trace = os.path.join(debugdir, 'tracing', 'trace')
-            fp = open(os.path.join(trace), "w")
-            fp.write("0")
-            fp.flush()
-            fp.close()
+            with open(os.path.join(trace), "w") as fp:
+                fp.write("0")
+                fp.flush()
 
         self.__cyclicoutput.seek(0)
         try:
@@ -380,7 +386,7 @@ class Cyclictest(rtevalModulePrototype):
             try:
                 index = int(vals[0])
             except:
-                self._log(Log.DEBUG, "cyclictest: unexpected output: %s" % line)
+                self._log(Log.DEBUG, f"cyclictest: unexpected output: {line}")
                 continue
 
             for i, core in enumerate(self.__cpus):
@@ -420,8 +426,7 @@ class Cyclictest(rtevalModulePrototype):
 
         # Let the user know if max latency overshot the number of buckets
         if self.__cyclicdata["system"].get_max() > self.__buckets:
-            self._log(Log.ERR, "Max latency(%dus) exceeded histogram range(%dus). Skipping statistics" %
-                      (self.__cyclicdata["system"].get_max(), self.__buckets))
+            self._log(Log.ERR, f'Max latency({self.__cyclicdata["system"].get_max()}us) exceeded histogram range({self.__buckets}us). Skipping statistics')
             self._log(Log.ERR, "Increase number of buckets to avoid lost samples")
             return rep_n
 
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/6] rteval: Create common  functions in CpuList and SysTopology
  2022-07-26 13:35 ` [PATCH 1/6] rteval: Create common functions in CpuList and SysTopology John Kacur
@ 2022-07-27 20:32   ` Leah Leshchinsky
  0 siblings, 0 replies; 8+ messages in thread
From: Leah Leshchinsky @ 2022-07-27 20:32 UTC (permalink / raw)
  To: John Kacur; +Cc: RT, Clark Williams, Manasi Godse

On Tue, Jul 26, 2022 at 09:35:30AM -0400, John Kacur wrote:
> The purpose is to remove functions out of misc and use the ones in the
> file systopolgy.py
> 
> - Add function collapse_cpulist(cpulist)  outside of the CpuList
>   class
> - Make methods longest_sequence and expand_cpulist accesible outside of
>   their class
> - Add function online_cpus(self) to class SysTopology
> - Add function online_cpus_str(self) to class SysTopology
> - Add function invert_cpulist(self, cpulist) to class SysTopology
> - Convert strings to f-strings for better readability
> - Add a few missing docstrings to methods / functions, module etc
> - Add a few more tests to the unit test
> 
> TODO: CpuList is suited for use by SysTopology, but is not ideal for a
> generic CpuList. A more generally usable CpuList should be created
> 
> Signed-off-by: John Kacur <jkacur@redhat.com>
> ---
>  rteval/systopology.py | 90 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 74 insertions(+), 16 deletions(-)
> 
> diff --git a/rteval/systopology.py b/rteval/systopology.py
> index e852f86e450f..ce8d02cf7318 100644
> --- a/rteval/systopology.py
> +++ b/rteval/systopology.py
> @@ -23,11 +23,32 @@
>  #   including keys needed to generate an equivalently functional executable
>  #   are deemed to be part of the source code.
>  #
> +""" Module for querying cpu cores and nodes """
>  
>  import os
>  import os.path
>  import glob
>  
> +# Utility version of collapse_cpulist that doesn't require a CpuList object
> +def collapse_cpulist(cpulist):
> +    """ Collapse a list of cpu numbers into a string range
> +        of cpus (e.g. 0-5, 7, 9) """
> +    if len(cpulist) == 0:
> +        return ""
> +    idx = CpuList.longest_sequence(cpulist)
> +    if idx == 0:
> +        seq = str(cpulist[0])
> +    else:
> +        if idx == 1:
> +            seq = f"{cpulist[0]},{cpulist[idx]}"
> +        else:
> +            seq = f"{cpulist[0]}-{cpulist[idx]}"
> +
> +    rest = collapse_cpulist(cpulist[idx+1:])
> +    if rest == "":
> +        return seq
> +    return ",".join((seq, rest))
> +
>  def sysread(path, obj):
>      """ Helper function for reading system files """
>      with open(os.path.join(path, obj), "r") as fp:
> @@ -46,7 +67,7 @@ class CpuList:
>          if isinstance(cpulist, list):
>              self.cpulist = cpulist
>          elif isinstance(cpulist, str):
> -            self.cpulist = self.__expand_cpulist(cpulist)
> +            self.cpulist = self.expand_cpulist(cpulist)
>          self.cpulist = self.online_cpulist(self.cpulist)
>          self.cpulist.sort()
>  
> @@ -67,7 +88,7 @@ class CpuList:
>          return False
>  
>      @staticmethod
> -    def __longest_sequence(cpulist):
> +    def longest_sequence(cpulist):
>          """ return index of last element of a sequence that steps by one """
>          lim = len(cpulist)
>          for idx, _ in enumerate(cpulist):
> @@ -83,14 +104,14 @@ class CpuList:
>          """
>          if len(cpulist) == 0:
>              return ""
> -        idx = self.__longest_sequence(cpulist)
> +        idx = self.longest_sequence(cpulist)
>          if idx == 0:
>              seq = str(cpulist[0])
>          else:
>              if idx == 1:
> -                seq = "%d,%d" % (cpulist[0], cpulist[idx])
> +                seq = f"{cpulist[0]},{cpulist[idx]}"
>              else:
> -                seq = "%d-%d" % (cpulist[0], cpulist[idx])
> +                seq = f"{cpulist[0]}-{cpulist[idx]}"
>  
>          rest = self.__collapse_cpulist(cpulist[idx+1:])
>          if rest == "":
> @@ -98,7 +119,14 @@ class CpuList:
>          return ",".join((seq, rest))
>  
>      @staticmethod
> -    def __expand_cpulist(cpulist):
> +    def compress_cpulist(cpulist):
> +        """ return a string representation of cpulist """
> +        if isinstance(cpulist[0], int):
> +            return ",".join(str(e) for e in cpulist)
> +        return ",".join(cpulist)
> +
> +    @staticmethod
> +    def expand_cpulist(cpulist):
>          """ expand a range string into an array of cpu numbers
>          don't error check against online cpus
>          """
> @@ -124,8 +152,8 @@ class CpuList:
>      def is_online(self, n):
>          """ check whether cpu n is online """
>          if n not in self.cpulist:
> -            raise RuntimeError("invalid cpu number %d" % n)
> -        path = os.path.join(CpuList.cpupath, 'cpu%d' % n)
> +            raise RuntimeError(f"invalid cpu number {n}")
> +        path = os.path.join(CpuList.cpupath, f'cpu{n}')
>  
>          # Some hardware doesn't allow cpu0 to be turned off
>          if not os.path.exists(path + '/online') and n == 0:
> @@ -240,8 +268,8 @@ class SysTopology:
>          return len(list(self.nodes.keys()))
>  
>      def __str__(self):
> -        s = "%d node system" % len(list(self.nodes.keys()))
> -        s += " (%d cores per node)" % (len(self.nodes[list(self.nodes.keys())[0]]))
> +        s = f"{len(list(self.nodes.keys()))} node system "
> +        s += f"({(len(self.nodes[list(self.nodes.keys())[0]]))} cores per node)"
>          return s
>  
>      def __contains__(self, node):
> @@ -268,6 +296,7 @@ class SysTopology:
>          return n
>  
>      def getinfo(self):
> +        """ Initialize class Systopology """
>          nodes = glob.glob(os.path.join(SysTopology.nodepath, 'node[0-9]*'))
>          if nodes:
>              nodes.sort()
> @@ -278,27 +307,56 @@ class SysTopology:
>              self.nodes[0] = SimNumaNode()
>  
>      def getnodes(self):
> +        """ return a list of nodes """
>          return list(self.nodes.keys())
>  
>      def getcpus(self, node):
> +        """ return a dictionary of cpus keyed with the node """
>          return self.nodes[node].getcpulist()
>  
> +    def online_cpus(self):
> +        """ return a list of integers of all online cpus """
> +        cpulist = []
> +        for n in self.nodes:
> +            cpulist += self.getcpus(n)
> +        cpulist.sort()
> +        return cpulist
> +
> +    def online_cpus_str(self):
> +        """ return a list of strings of numbers of all online cpus """
> +        cpulist = [str(cpu) for cpu in self.online_cpus()]
> +        return cpulist
> +
> +    def invert_cpulist(self, cpulist):
> +        """ return a list of online cpus not in cpulist """
> +        return [c for c in self.online_cpus_str() if c not in cpulist]
>  
>  if __name__ == "__main__":
>  
>      def unit_test():
> +        """ unit test, run python rteval/systopology.py """
>          s = SysTopology()
>          print(s)
> -        print("number of nodes: %d" % len(s))
> +        print(f"number of nodes: {len(s)}")
>          for n in s:
> -            print("node[%d]: %s" % (n.nodeid, n))
> -        print("system has numa node 0: %s" % (0 in s))
> -        print("system has numa node 2: %s" % (2 in s))
> -        print("system has numa node 24: %s" % (24 in s))
> +            print(f"node[{n.nodeid}]: {n}")
> +        print(f"system has numa node 0: {0 in s}")
> +        print(f"system has numa node 2: {2 in s}")
> +        print(f"system has numa node 24: {24 in s}")
>  
>          cpus = {}
> +        print(f"nodes = {s.getnodes()}")
>          for node in s.getnodes():
>              cpus[node] = s.getcpus(int(node))
> -        print(f'cpus = {cpus}')
> +            print(f'cpus = {cpus}')
> +
> +        onlcpus = s.online_cpus()
> +        print(f'onlcpus = {onlcpus}')
> +        onlcpus = collapse_cpulist(onlcpus)
> +        print(f'onlcpus = {onlcpus}')
> +
> +        onlcpus_str = s.online_cpus_str()
> +        print(f'onlcpus_str = {onlcpus_str}')
>  
> +        print(f"invert of [ 2, 4, 5 ] = {s.invert_cpulist([2, 3, 4])}")
>      unit_test()
> -- 
> 2.37.1
> 

collapse_cpulist():

- Ordered numerical lists work properly [1,2,3,4,5,10,11,12,15,18,19,20] => '1-5,10-12,15,18-20'
- Empty list works as expected [] => ''
- Non-numerical elements result in ValueError ["a", 2, 3] => ValueError
- Mixture of numerical interger and string elements works properly ["0",2,3,4] => '0,2-4'

- Unordered list does not produce range. [0,10,11,12,2,5,4,3] => '0,10-12,2,5,4,3'
  Consider sorting the list before operating on it.
- Providing string to function that expects a list results in strange behavior '123' => '1-3'
  Perhaps check for whether a list was provided prior to operating on input

CpuList.compress_cpulist():

- Does not accept empty list [] => IndexError.
  Consider handling this case.
- Compress_cpulist is a bit misleading for a function that returns a string representation, and could be confused with collapse_cpulist.
  Consider renaming to something the indicates str output.

SysTopology.online_cpus():

- Tested by intializing SysTopology object, calling online_cpus showed [0,1,2,3,4,5,6,7],
then disabled cpu 4 and tested again by calling online_cpus(), which still showed cpu 4 as active.
Reinitializing the class and calling online_cpus() worked properly and showed [1,2,3,5,6,7] where cpu 4 was disabled.
Consider adding a comment to indicate that this function displays the state of the system at class initialization.
In the future, a function may be added to display the state of the system at function call time.

>>> top = st.SysTopology()
>>> top.online_cpus()
[0, 1, 2, 3, 4, 5, 6, 7]

$ su -c 'echo 0 > /sys/devices/system/cpu/cpu4/online '

>>> top.online_cpus()
[0, 1, 2, 3, 4, 5, 6, 7]
>>> top = st.SysTopology()
>>> top.online_cpus()
[0, 1, 2, 3, 5, 6, 7]


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-07-27 20:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-26 13:35 [PATCH 0/6] Create more common interfaces in misc and use them John Kacur
2022-07-26 13:35 ` [PATCH 1/6] rteval: Create common functions in CpuList and SysTopology John Kacur
2022-07-27 20:32   ` Leah Leshchinsky
2022-07-26 13:35 ` [PATCH 2/6] rteval: Make use of systopology instead of misc in rteval-cmd John Kacur
2022-07-26 13:35 ` [PATCH 3/6] rteval: Make use of systopology instead of misc in hackbench John Kacur
2022-07-26 13:35 ` [PATCH 4/6] rteval: Make use of systopology instead of misc in kcompile John Kacur
2022-07-26 13:35 ` [PATCH 5/6] rteval: Make use of systopology instead of misc in stressng module John Kacur
2022-07-26 13:35 ` [PATCH 6/6] rteval: Make use of systopology instead of misc in cyclictest John Kacur

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.