Re: [PATCH 2/5] rteval: cyclictest.py: Update logic to get core description

[John Kacur <jkacur@redhat.com>]

On Wed, 1 Sep 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> Certain architectures such as arm and arm64 don't have a "model name"
> in /proc/cpuinfo but provide other identifying information such as
> implementer, architecture, variant, part, revision, etc..
> 
> Add a function 'get_cpumodel()' that takes the per-core dictionary
> constructed from /proc/cpuinfo and uses the available data to
> construct the CPU description. Update the users of "model name" to use
> the newly added function.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  rteval/misc.py                           | 23 ++++++++++++++++++++++-
>  rteval/modules/measurement/cyclictest.py |  8 +++++---
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/rteval/misc.py b/rteval/misc.py
> index 0dd361ff19fd..537041b53d61 100644
> --- a/rteval/misc.py
> +++ b/rteval/misc.py
> @@ -79,6 +79,27 @@ def cpuinfo():
>          info[core][key] = val
>      return info
>  
> +# Pass the per-core dictionary that is part of the dictionary returned
> +# by cpuinfo()
> +def get_cpumodel(core_info):
> +    desc = core_info.get('model name', '')
> +    if not desc:
> +        # On Arm (both 32 and 64 bit), 'CPU Implementer' is always
> +        # present. Return 'unknown' otherwise
> +        if 'CPU implementer' not in core_info:
> +            desc = 'unknown'
> +        else:
> +            implementor = core_info.get('CPU implementer', '')
> +            architecture = core_info.get('CPU architecture', '')
> +            variant = core_info.get('CPU variant', '')
> +            part = core_info.get('CPU part', '')
> +            revision = core_info.get('CPU revision', '')
> +
> +            desc = 'Implementor: %s Architecture: %s Variant: %s Part: %s Revision: %s' \
> +                % (implementor, architecture, variant, part, revision)
> +
> +    return desc
> +
>  if __name__ == "__main__":
>  
>      info = cpuinfo()
> @@ -86,4 +107,4 @@ if __name__ == "__main__":
>      for i in idx:
>          print("%s: %s" % (i, info[i]))
>  
> -    print("0: %s" % (info['0']['model name']))
> +    print("0: %s" % (get_cpumodel(info['0'])))
> diff --git a/rteval/modules/measurement/cyclictest.py b/rteval/modules/measurement/cyclictest.py
> index b1755d4f4421..11cb45e711dd 100644
> --- a/rteval/modules/measurement/cyclictest.py
> +++ b/rteval/modules/measurement/cyclictest.py
> @@ -34,7 +34,7 @@ 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 expand_cpulist, online_cpus, cpuinfo, get_cpumodel
>  
>  class RunData:
>      '''class to keep instance data from a cyclictest run'''
> @@ -226,13 +226,15 @@ class Cyclictest(rtevalModulePrototype):
>          for core in self.__cpus:
>              self.__cyclicdata[core] = RunData(core, 'core', self.__priority,
>                                                logfnc=self._log)
> -            self.__cyclicdata[core].description = info[core]['model name']
> +            self.__cyclicdata[core].description = get_cpumodel(info[core])
> +            if self.__cyclicdata[core].description == 'unknown':
> +                self._log(Log.INFO, "Unknown model for core %d" % core)
>  
>          # Create a RunData object for the overall system
>          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 = ("(%d cores) " % self.__numcores) + get_cpumodel(info['0'])
>  
>          if self.__sparse:
>              self._log(Log.DEBUG, "system using %d cpu cores" % self.__numcores)
> -- 
> 2.32.0
> 
> 

I like the idea of constructing the 'model name' from other fields in 
/proc/cpuinfo available for arm.

There are a number of problems with the implementation.
This should be written in such away that the local changes in misc.py
work seamlessly without changes to the code that calls it. The code should 
also add the 'model name' to every core.
Finally, a lesser detail, but there is no need when using get to set a 
default of "" when the default is already None. Just make the code work 
with None.

I have rewritten this from scratch, attributing the idea to you, I hope 
that is okay. I will send the patch to the list shortly.

John