[v5,1/2] rteval: Added functionality to allow user to set the cstate of specified cpus when running rteval

We would like to be able to set the idle states of CPUs while running
rteval.

This patch adds the file cpupower.py and option '--idle-set' within
rteval to use cpupower. The set idle state is applied to the cpulist
given by '--measurement-cpulist'.

cpupower.py provides the infrastructure to interface and execute the cpupower
command, and the options in rteval-cmd let the user specify the idle state
to be set and the CPUs to set it on.

Signed-off-by: Anubhav Shelat <ashelat@redhat.com>
---
 rteval-cmd                 | 22 ++++++++-
 rteval/cpupower.py         | 94 ++++++++++++++++++++++++++++++++++++++
 rteval/modules/__init__.py |  2 +-
 3 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 rteval/cpupower.py

On Thu, 25 Jul 2024, Anubhav Shelat wrote:

> We would like to be able to set the idle states of CPUs while running
> rteval.
> 
> This patch adds the file cpupower.py and option '--idle-set' within
> rteval to use cpupower. The set idle state is applied to the cpulist
> given by '--measurement-cpulist'.
> 
> cpupower.py provides the infrastructure to interface and execute the cpupower
> command, and the options in rteval-cmd let the user specify the idle state
> to be set and the CPUs to set it on.
>

Ok, this time the patch applied cleanly, but there are a number of 
problems with it, please see below.


 
> Signed-off-by: Anubhav Shelat <ashelat@redhat.com>
> ---
>  rteval-cmd                 | 22 ++++++++-
>  rteval/cpupower.py         | 94 ++++++++++++++++++++++++++++++++++++++
>  rteval/modules/__init__.py |  2 +-
>  3 files changed, 116 insertions(+), 2 deletions(-)
>  create mode 100644 rteval/cpupower.py
> 
> diff --git a/rteval-cmd b/rteval-cmd
> index 19c82a0b64b3..9d87328302df 100755
> --- a/rteval-cmd
> +++ b/rteval-cmd
> @@ -29,6 +29,7 @@ from rteval.Log import Log
>  from rteval import RtEval, rtevalConfig
>  from rteval.modules.loads import LoadModules
>  from rteval.modules.measurement import MeasurementModules
> +from rteval import cpupower
>  from rteval.version import RTEVAL_VERSION
>  from rteval.systopology import SysTopology, parse_cpulist_from_config
>  from rteval.modules.loads.kcompile import ModuleParameters
> @@ -149,7 +150,8 @@ def parse_options(cfg, parser, cmdargs):
>      parser.add_argument("--noload", dest="rteval___noload",
>                          action="store_true", default=False,
>                          help="only run the measurements (don't run loads)")
> -
> +    parser.add_argument('--idle-set', dest='rteval___idlestate', metavar='IDLESTATE',
> +                        default=None, help='Idle state to enable for this rteval run')
>  
>      if not cmdargs:
>          cmdargs = ["--help"]
> @@ -172,6 +174,12 @@ def parse_options(cfg, parser, cmdargs):
>          cmd_args = cmdargs[ind+1:]
>          cmdargs = cmdargs[:ind+1]
>  
> +    # if --cpulist is specified, make sure --idle-set is too
> +    if ((sys.argv.count('-c')+sys.argv.count('--cpulist')) > 0) and \
> +    (sys.argv.count('--idle-set') == 0):
> +        print('--cpulist must be used with --idle-set')
> +        cmdargs = ["--help"]
> +
>      cmd_opts = parser.parse_args(args=cmdargs)
>  
>      # if no kernel version was provided for --source-download, set version to default
> @@ -265,6 +273,9 @@ if __name__ == '__main__':
>                  | (rtevcfg.debugging and Log.DEBUG)
>          logger.SetLogVerbosity(loglev)
>  
> +        # check if cpupower is being used
> +        if sys.argv.count('--idle-set') > 0:
> +            rtevcfg.update({'usingCpupower': True})
>          # Load modules
>          loadmods = LoadModules(config, logger=logger)
>          measuremods = MeasurementModules(config, logger=logger)
> @@ -402,6 +413,11 @@ if __name__ == '__main__':
>          if not os.path.isdir(rtevcfg.workdir):
>              raise RuntimeError(f"work directory {rtevcfg.workdir} does not exist")
>  
> +        # if idle-set has been specified, enable idlestate in rtecfg.cpulist
> +        cpupower_controller = None
> +        if rtevcfg.idlestate:
> +            cpupower_controller = cpupower.Cpupower(msrcfg.cpulist, rtevcfg.idlestate)
> +            cpupower_controller.enable_idle_state()
>  
>          rteval = RtEval(config, loadmods, measuremods, logger)
>          rteval.Prepare(rtevcfg.onlyload)
> @@ -421,6 +437,10 @@ if __name__ == '__main__':
>              ec = rteval.Measure()
>              logger.log(Log.DEBUG, f"exiting with exit code: {ec}")
>  
> +        # reset idlestate
> +        if rtevcfg.idlestate:
> +            cpupower_controller.restore_cstate()
> +
>          sys.exit(ec)
>      except KeyboardInterrupt:
>          sys.exit(0)
> diff --git a/rteval/cpupower.py b/rteval/cpupower.py
> new file mode 100644
> index 000000000000..9437b2451b54
> --- /dev/null
> +++ b/rteval/cpupower.py
> @@ -0,0 +1,94 @@
> +#! /user/bin/python3

If I run
su -c "./rteval/cpupower"
I get
bash: line 1: ./rteval/cpupower: No such file or directory
and this is because you spelled "user", when it should be "usr"

If I work around that problem like this, then I get a new problem.
su -c "python  ./rteval/cpupower.py "
Password: 
Namespace(cpu_list=None, idle_set='', info=False)
Traceback (most recent call last):
  File "/home/jkacur/src/rteval/./rteval/cpupower.py", line 85, in 
<module>
    idlestate = args.cstate
                ^^^^^^^^^^^
AttributeError: 'Namespace' object has no attribute 'cstate'

I assume that you meant args.idle_set
which, I understand we are translating a subcommand from cpupower into an 
option, so
idle_set is fine, but you probably want a metavar of "state" so the help
output looks nicer.

Instead of time.sleep(10), if you use DEBUG log statements in the code
that print out the state, then there is no need to sleep
This is also probably what you want instead of raw print statements in the 
code.

Please consistently change references of cstate to idle_state, or 
idlestate.

You should probably check the output after Subprocess.run
(check=True) and handle subprocess.CalledProcessError

The code doesn't look too bad but we can use some existing structures
and some more pythonesque code. For example, you probably don't need
a cpu count, you can just loop through the cpu list

We can discuss offline if you wish.

Thanks

John



> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +# Object to execute cpupower tool
> +# Copyright 2024    Anubhav Shelat <ashelat@redhat.com>
> +
> +import subprocess
> +import argparse
> +import os
> +import multiprocessing
> +import time
> +
> +
> +class Cpupower:
> +    def __init__(self, cpulist, idlestate):
> +        self.cpulist = cpulist
> +        self.idlestate = idlestate
> +        self.nstates = len(os.listdir('/sys/devices/system/cpu/cpu0/cpuidle/'))    # number of cstates
> +        self.cpu_count = multiprocessing.cpu_count()
> +        self.cstate_cfg = []
> +
> +
> +    def enable_idle_state(self):
> +        ''' Enable a specific cstate, while disabling all other cstates, and save the current cstate configuration '''
> +        self.cstate_cfg = self.get_cstate_cfg()
> +
> +        # ensure that cstate is in range of available cstates
> +        if int(self.idlestate) > self.nstates-1 or int(self.idlestate) < 0:
> +            print(f'Idle state {self.idlestate} is out of range, using POLL')
> +            self.idlestate = 0
> +
> +        # enable cstate and disable the rest
> +        if (self.cpulist):
> +            subprocess.run(['sudo', 'cpupower', '-c', self.cpulist,'idle-set', '-e', str(self.idlestate)], stdout=open(os.devnull, 'wb'))
> +            for cs in range(self.nstates):
> +                if str(cs) != self.idlestate:
> +                    subprocess.run(['sudo', 'cpupower', '-c', self.cpulist,'idle-set', '-d', str(cs)], stdout=open(os.devnull, 'wb'))
> +        else:
> +            subprocess.run(['sudo', 'cpupower', 'idle-set', '-e', str(self.idlestate)], stdout=open(os.devnull, 'wb'))
> +            for cs in range(self.nstates):
> +                if str(cs) != self.idlestate:
> +                    subprocess.run(['sudo', 'cpupower', 'idle-set', '-d', str(cs)], stdout=open(os.devnull, 'wb'))
> +
> +        if self.cpulist: print(f'Idlestate {self.idlestate} enabled on CPUs {self.cpulist}')
> +        else: print(f'Idlestate {self.idlestate} enabled on all CPUs')
> +
> +
> +    def get_cstate_cfg(self):
> +        ''' Store the current cstate config '''
> +        # cstate [m] configuration on cpu [n] can be found in '/sys/devices/system/cpu/cpu*/cpuidle/state*/disable'
> +        cfg = []
> +        for cpu in range(self.cpu_count):
> +            cfg.append([])
> +            for cs in range(self.nstates):
> +                f = open('/sys/devices/system/cpu/cpu'+str(cpu)+'/cpuidle/state'+str(cs)+'/disable', 'r')
> +                d = f.read(1)
> +                cfg[cpu].append(d)   # cstate_cfg[n][m] stores the m-th idle state on the n-th cpu
> +
> +        return cfg
> +
> +
> +    def restore_cstate(self):
> +        for cpu in range(self.cpu_count):
> +            for cs in range(self.nstates):
> +                f = open('/sys/devices/system/cpu/cpu'+str(cpu)+'/cpuidle/state'+str(cs)+'/disable', 'w')
> +                f.write(self.cstate_cfg[cpu][cs])
> +                f.close()
> +        print('Idle state configuration restored')
> +
> +    def get_idle_info():
> +        subprocess.call(['cpupower', 'idle-info'])
> +
> +
> +if __name__ == '__main__':
> +    parser = argparse.ArgumentParser()
> +    parser.add_argument('-c', '--cpu-list', required=False, default=None,
> +                        help='List of cpus to perform cpupower-idle-set operation on.')
> +    parser.add_argument('-s', '--idle-set', default='',
> +                        help='Specify idle state to enable/disable')
> +    parser.add_argument('--info', default=False, action='store_true', help='Get idle state information')
> +
> +    args = parser.parse_args()
> +    print(args)
> +    cpulist = args.cpu_list
> +    idlestate = args.cstate
> +    if (idlestate):
> +        cpupower = Cpupower(cpulist, idlestate)
> +        cpupower.enable_idle_state()
> +        time.sleep(10)
> +        cpupower.restore_cstate()
> +    elif (args.info):
> +        Cpupower.get_idle_info()
> +
> +
> diff --git a/rteval/modules/__init__.py b/rteval/modules/__init__.py
> index 2a4eafae71c7..10cbd50ddfd8 100644
> --- a/rteval/modules/__init__.py
> +++ b/rteval/modules/__init__.py
> @@ -282,7 +282,7 @@ reference from the first import"""
>          grparser.add_argument(f'--{self.__modtype}-cpulist',
>                              dest=f'{self.__modtype}___cpulist', action='store', default="",
>                              help=f'CPU list where {self.__modtype} modules will run',
> -                            metavar='LIST')
> +                            metavar='CPULIST')
>  
>          for (modname, mod) in list(self.__modsloaded.items()):
>              opts = mod.ModuleParameters()
> -- 
> 2.45.2
> 
> 
>