All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] rteval: Add measurement and load location to run report
@ 2022-08-04 15:48 Leah Leshchinsky
  2022-08-04 23:56 ` John Kacur
  2022-08-05 13:37 ` Leah Leshchinsky
  0 siblings, 2 replies; 6+ messages in thread
From: Leah Leshchinsky @ 2022-08-04 15:48 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users

The run report produced at the end of a run does not contain information
on load and measurement thread locations.

Adjust MakeReport() functions of LoadModules and MeasurementModules
class so that new properties with number of loads and cpu information
are added to the XML report and can be read by rteval_text.xsl.

Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>

diff --git a/rteval/modules/loads/__init__.py b/rteval/modules/loads/__init__.py
index 2c2105efa964..5e5a5d85b616 100644
--- a/rteval/modules/loads/__init__.py
+++ b/rteval/modules/loads/__init__.py
@@ -30,6 +30,7 @@ import libxml2
 from rteval.Log import Log
 from rteval.rtevalConfig import rtevalCfgSection
 from rteval.modules import RtEvalModules, rtevalModulePrototype
+from rteval.systopology import collapse_cpulist, SysTopology
 
 class LoadThread(rtevalModulePrototype):
     def __init__(self, name, config, logger=None):
@@ -131,6 +132,12 @@ class LoadModules(RtEvalModules):
     def MakeReport(self):
         rep_n = RtEvalModules.MakeReport(self)
         rep_n.newProp("load_average", str(self.GetLoadAvg()))
+        rep_n.newProp("loads", str(self.ModulesLoaded()))
+        cpulist = self._cfg.GetSection(self._module_config).cpulist
+        if not cpulist:
+            st = SysTopology()
+            cpulist = collapse_cpulist(st.online_cpus())
+        rep_n.newProp("loadcpus", cpulist)
 
         return rep_n
 
diff --git a/rteval/modules/measurement/__init__.py b/rteval/modules/measurement/__init__.py
index 318248bd7e35..371fc5d08f44 100644
--- a/rteval/modules/measurement/__init__.py
+++ b/rteval/modules/measurement/__init__.py
@@ -24,7 +24,7 @@
 
 import libxml2
 from rteval.modules import RtEvalModules, ModuleContainer
-
+from rteval.systopology import collapse_cpulist, SysTopology
 
 class MeasurementProfile(RtEvalModules):
     """Keeps and controls all the measurement modules with the same measurement profile"""
@@ -189,6 +189,12 @@ measurement profiles, based on their characteristics"""
 
         # Get the reports from all meaurement modules in all measurement profiles
         rep_n = libxml2.newNode("Measurements")
+        cpulist = self.__cfg.GetSection("measurement").cpulist
+        if not cpulist:
+            st = SysTopology()
+            cpulist = collapse_cpulist(st.online_cpus())
+        rep_n.newProp("measurecpus", cpulist)
+
         for mp in self.__measureprofiles:
             mprep_n = mp.MakeReport()
             if mprep_n:
diff --git a/rteval/rteval_text.xsl b/rteval/rteval_text.xsl
index c40063e3dd19..7ecfac6b6140 100644
--- a/rteval/rteval_text.xsl
+++ b/rteval/rteval_text.xsl
@@ -13,6 +13,14 @@
     <xsl:value-of select="run_info/date"/><xsl:text> </xsl:text><xsl:value-of select="run_info/time"/>
     <xsl:text>&#10;</xsl:text>
 
+    <xsl:text>   Loads:        </xsl:text>
+    <xsl:value-of select="loads/@loads"/><xsl:text> loads run on cores </xsl:text><xsl:value-of select="loads/@loadcpus"/>
+    <xsl:text>&#10;</xsl:text>
+
+    <xsl:text>   Measurement:  </xsl:text>
+    <xsl:text>measurement threads run on cores </xsl:text><xsl:value-of select="Measurements/@measurecpus"/>
+    <xsl:text>&#10;</xsl:text>
+
     <xsl:text>   Run time:     </xsl:text>
     <xsl:value-of select="run_info/@days"/><xsl:text> days </xsl:text>
     <xsl:value-of select="run_info/@hours"/><xsl:text>h </xsl:text>
-- 
2.31.1


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

* Re: [PATCH v2] rteval: Add measurement and load location to run report
  2022-08-04 15:48 [PATCH v2] rteval: Add measurement and load location to run report Leah Leshchinsky
@ 2022-08-04 23:56 ` John Kacur
  2022-08-05 13:37 ` Leah Leshchinsky
  1 sibling, 0 replies; 6+ messages in thread
From: John Kacur @ 2022-08-04 23:56 UTC (permalink / raw)
  To: Leah Leshchinsky; +Cc: linux-rt-users



On Thu, 4 Aug 2022, Leah Leshchinsky wrote:

> The run report produced at the end of a run does not contain information
> on load and measurement thread locations.
> 
> Adjust MakeReport() functions of LoadModules and MeasurementModules
> class so that new properties with number of loads and cpu information
> are added to the XML report and can be read by rteval_text.xsl.
> 
> Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
> 
> diff --git a/rteval/modules/loads/__init__.py b/rteval/modules/loads/__init__.py
> index 2c2105efa964..5e5a5d85b616 100644
> --- a/rteval/modules/loads/__init__.py
> +++ b/rteval/modules/loads/__init__.py
> @@ -30,6 +30,7 @@ import libxml2
>  from rteval.Log import Log
>  from rteval.rtevalConfig import rtevalCfgSection
>  from rteval.modules import RtEvalModules, rtevalModulePrototype
> +from rteval.systopology import collapse_cpulist, SysTopology

add CpuList, and you might not need collapse_cpulist, and SysTopology

>  
>  class LoadThread(rtevalModulePrototype):
>      def __init__(self, name, config, logger=None):
> @@ -131,6 +132,12 @@ class LoadModules(RtEvalModules):
>      def MakeReport(self):
>          rep_n = RtEvalModules.MakeReport(self)
>          rep_n.newProp("load_average", str(self.GetLoadAvg()))
> +        rep_n.newProp("loads", str(self.ModulesLoaded()))
> +        cpulist = self._cfg.GetSection(self._module_config).cpulist

The above value is either going to be all cpus, or the value of 
--loads-cpulist=LIST or the inverse of --measurement-cpulist=LIST
I don't think it can be empty, so the following code after if not cpulist
is deadcode

> +        if not cpulist:
> +            st = SysTopology()
> +            cpulist = collapse_cpulist(st.online_cpus())

If the above code were to run it would just be a list of all online cpus

Admitedly the systopology api is a bit confusing but since it does a lot, 
I think all you really need is
cpulist = CpuList(self._cfg.GetSection(self._module_config).cpulist)
 rep_n.newProp("loadcpus", str(cpulist))

And that's it. You need to do something similar to the measurement section 
too.

> +        rep_n.newProp("loadcpus", cpulist)
>  
>          return rep_n
>  
> diff --git a/rteval/modules/measurement/__init__.py b/rteval/modules/measurement/__init__.py
> index 318248bd7e35..371fc5d08f44 100644
> --- a/rteval/modules/measurement/__init__.py
> +++ b/rteval/modules/measurement/__init__.py
> @@ -24,7 +24,7 @@
>  
>  import libxml2
>  from rteval.modules import RtEvalModules, ModuleContainer
> -
> +from rteval.systopology import collapse_cpulist, SysTopology
>  
>  class MeasurementProfile(RtEvalModules):
>      """Keeps and controls all the measurement modules with the same measurement profile"""
> @@ -189,6 +189,12 @@ measurement profiles, based on their characteristics"""
>  
>          # Get the reports from all meaurement modules in all measurement profiles
>          rep_n = libxml2.newNode("Measurements")
> +        cpulist = self.__cfg.GetSection("measurement").cpulist
> +        if not cpulist:
> +            st = SysTopology()
> +            cpulist = collapse_cpulist(st.online_cpus())
> +        rep_n.newProp("measurecpus", cpulist)
> +
>          for mp in self.__measureprofiles:
>              mprep_n = mp.MakeReport()
>              if mprep_n:
> diff --git a/rteval/rteval_text.xsl b/rteval/rteval_text.xsl
> index c40063e3dd19..7ecfac6b6140 100644
> --- a/rteval/rteval_text.xsl
> +++ b/rteval/rteval_text.xsl
> @@ -13,6 +13,14 @@
>      <xsl:value-of select="run_info/date"/><xsl:text> </xsl:text><xsl:value-of select="run_info/time"/>
>      <xsl:text>&#10;</xsl:text>
>  
> +    <xsl:text>   Loads:        </xsl:text>
> +    <xsl:value-of select="loads/@loads"/><xsl:text> loads run on cores </xsl:text><xsl:value-of select="loads/@loadcpus"/>
> +    <xsl:text>&#10;</xsl:text>
> +
> +    <xsl:text>   Measurement:  </xsl:text>
> +    <xsl:text>measurement threads run on cores </xsl:text><xsl:value-of select="Measurements/@measurecpus"/>
> +    <xsl:text>&#10;</xsl:text>
> +
>      <xsl:text>   Run time:     </xsl:text>
>      <xsl:value-of select="run_info/@days"/><xsl:text> days </xsl:text>
>      <xsl:value-of select="run_info/@hours"/><xsl:text>h </xsl:text>
> -- 
> 2.31.1
> 
> 

Something in the xsl code is not working, I'm not picking up any values in 
the reports for "Measurements/@measurecpus" for example.

Thanks


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

* Re: [PATCH v2] rteval: Add measurement and load location to run report
  2022-08-04 15:48 [PATCH v2] rteval: Add measurement and load location to run report Leah Leshchinsky
  2022-08-04 23:56 ` John Kacur
@ 2022-08-05 13:37 ` Leah Leshchinsky
  2022-08-05 16:32   ` John Kacur
  1 sibling, 1 reply; 6+ messages in thread
From: Leah Leshchinsky @ 2022-08-05 13:37 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users

On Thu, Aug 04, 2022 at 11:48:07AM -0400, Leah Leshchinsky wrote:
> The run report produced at the end of a run does not contain information
> on load and measurement thread locations.
> 
> Adjust MakeReport() functions of LoadModules and MeasurementModules
> class so that new properties with number of loads and cpu information
> are added to the XML report and can be read by rteval_text.xsl.
> 
> Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
> 
> diff --git a/rteval/modules/loads/__init__.py b/rteval/modules/loads/__init__.py
> index 2c2105efa964..5e5a5d85b616 100644
> --- a/rteval/modules/loads/__init__.py
> +++ b/rteval/modules/loads/__init__.py
> @@ -30,6 +30,7 @@ import libxml2
>  from rteval.Log import Log
>  from rteval.rtevalConfig import rtevalCfgSection
>  from rteval.modules import RtEvalModules, rtevalModulePrototype
> +from rteval.systopology import collapse_cpulist, SysTopology
>  
>  class LoadThread(rtevalModulePrototype):
>      def __init__(self, name, config, logger=None):
> @@ -131,6 +132,12 @@ class LoadModules(RtEvalModules):
>      def MakeReport(self):
>          rep_n = RtEvalModules.MakeReport(self)
>          rep_n.newProp("load_average", str(self.GetLoadAvg()))
> +        rep_n.newProp("loads", str(self.ModulesLoaded()))
> +        cpulist = self._cfg.GetSection(self._module_config).cpulist
> +        if not cpulist:
> +            st = SysTopology()
> +            cpulist = collapse_cpulist(st.online_cpus())
> +        rep_n.newProp("loadcpus", cpulist)
>  
>          return rep_n
>  
> diff --git a/rteval/modules/measurement/__init__.py b/rteval/modules/measurement/__init__.py
> index 318248bd7e35..371fc5d08f44 100644
> --- a/rteval/modules/measurement/__init__.py
> +++ b/rteval/modules/measurement/__init__.py
> @@ -24,7 +24,7 @@
>  
>  import libxml2
>  from rteval.modules import RtEvalModules, ModuleContainer
> -
> +from rteval.systopology import collapse_cpulist, SysTopology
>  
>  class MeasurementProfile(RtEvalModules):
>      """Keeps and controls all the measurement modules with the same measurement profile"""
> @@ -189,6 +189,12 @@ measurement profiles, based on their characteristics"""
>  
>          # Get the reports from all meaurement modules in all measurement profiles
>          rep_n = libxml2.newNode("Measurements")
> +        cpulist = self.__cfg.GetSection("measurement").cpulist
> +        if not cpulist:
> +            st = SysTopology()
> +            cpulist = collapse_cpulist(st.online_cpus())
> +        rep_n.newProp("measurecpus", cpulist)
> +
>          for mp in self.__measureprofiles:
>              mprep_n = mp.MakeReport()
>              if mprep_n:
> diff --git a/rteval/rteval_text.xsl b/rteval/rteval_text.xsl
> index c40063e3dd19..7ecfac6b6140 100644
> --- a/rteval/rteval_text.xsl
> +++ b/rteval/rteval_text.xsl
> @@ -13,6 +13,14 @@
>      <xsl:value-of select="run_info/date"/><xsl:text> </xsl:text><xsl:value-of select="run_info/time"/>
>      <xsl:text>&#10;</xsl:text>
>  
> +    <xsl:text>   Loads:        </xsl:text>
> +    <xsl:value-of select="loads/@loads"/><xsl:text> loads run on cores </xsl:text><xsl:value-of select="loads/@loadcpus"/>
> +    <xsl:text>&#10;</xsl:text>
> +
> +    <xsl:text>   Measurement:  </xsl:text>
> +    <xsl:text>measurement threads run on cores </xsl:text><xsl:value-of select="Measurements/@measurecpus"/>
> +    <xsl:text>&#10;</xsl:text>
> +
>      <xsl:text>   Run time:     </xsl:text>
>      <xsl:value-of select="run_info/@days"/><xsl:text> days </xsl:text>
>      <xsl:value-of select="run_info/@hours"/><xsl:text>h </xsl:text>
> -- 
> 2.31.1
> 

Hi John,

I do believe the cpulist variable can be empty. I was able to test this by running rteval without specifying and load and measurement cpus.

This patch is attempting to replicate the behavior found in rteval/__init.py lines 187-201, which also finds the cpulist using
            cpulist = self._loadmods._cfg.GetSection("loads").cpulist
and if the cpulist is empty, defaults to reporting the online cpus. I assume this is because the rteval program uses all available cpus
to run measurement and load threads if no specific cpus are specified.

Please let me know if I am misunderstanding any of the above.

I will investigate why the xsl code is not working for you. It seems to be working for me at the moment.

Thank you for the review,
Leah


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

* Re: [PATCH v2] rteval: Add measurement and load location to run report
  2022-08-05 13:37 ` Leah Leshchinsky
@ 2022-08-05 16:32   ` John Kacur
  2022-08-05 17:03     ` Leah Leshchinsky
  0 siblings, 1 reply; 6+ messages in thread
From: John Kacur @ 2022-08-05 16:32 UTC (permalink / raw)
  To: Leah Leshchinsky; +Cc: linux-rt-users



On Fri, 5 Aug 2022, Leah Leshchinsky wrote:

> On Thu, Aug 04, 2022 at 11:48:07AM -0400, Leah Leshchinsky wrote:
> > The run report produced at the end of a run does not contain information
> > on load and measurement thread locations.
> > 
> > Adjust MakeReport() functions of LoadModules and MeasurementModules
> > class so that new properties with number of loads and cpu information
> > are added to the XML report and can be read by rteval_text.xsl.
> > 
> > Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
> > 
> > diff --git a/rteval/modules/loads/__init__.py b/rteval/modules/loads/__init__.py
> > index 2c2105efa964..5e5a5d85b616 100644
> > --- a/rteval/modules/loads/__init__.py
> > +++ b/rteval/modules/loads/__init__.py
> > @@ -30,6 +30,7 @@ import libxml2
> >  from rteval.Log import Log
> >  from rteval.rtevalConfig import rtevalCfgSection
> >  from rteval.modules import RtEvalModules, rtevalModulePrototype
> > +from rteval.systopology import collapse_cpulist, SysTopology
> >  
> >  class LoadThread(rtevalModulePrototype):
> >      def __init__(self, name, config, logger=None):
> > @@ -131,6 +132,12 @@ class LoadModules(RtEvalModules):
> >      def MakeReport(self):
> >          rep_n = RtEvalModules.MakeReport(self)
> >          rep_n.newProp("load_average", str(self.GetLoadAvg()))
> > +        rep_n.newProp("loads", str(self.ModulesLoaded()))
> > +        cpulist = self._cfg.GetSection(self._module_config).cpulist
> > +        if not cpulist:
> > +            st = SysTopology()
> > +            cpulist = collapse_cpulist(st.online_cpus())
> > +        rep_n.newProp("loadcpus", cpulist)
> >  
> >          return rep_n
> >  
> > diff --git a/rteval/modules/measurement/__init__.py b/rteval/modules/measurement/__init__.py
> > index 318248bd7e35..371fc5d08f44 100644
> > --- a/rteval/modules/measurement/__init__.py
> > +++ b/rteval/modules/measurement/__init__.py
> > @@ -24,7 +24,7 @@
> >  
> >  import libxml2
> >  from rteval.modules import RtEvalModules, ModuleContainer
> > -
> > +from rteval.systopology import collapse_cpulist, SysTopology
> >  
> >  class MeasurementProfile(RtEvalModules):
> >      """Keeps and controls all the measurement modules with the same measurement profile"""
> > @@ -189,6 +189,12 @@ measurement profiles, based on their characteristics"""
> >  
> >          # Get the reports from all meaurement modules in all measurement profiles
> >          rep_n = libxml2.newNode("Measurements")
> > +        cpulist = self.__cfg.GetSection("measurement").cpulist
> > +        if not cpulist:
> > +            st = SysTopology()
> > +            cpulist = collapse_cpulist(st.online_cpus())
> > +        rep_n.newProp("measurecpus", cpulist)
> > +
> >          for mp in self.__measureprofiles:
> >              mprep_n = mp.MakeReport()
> >              if mprep_n:
> > diff --git a/rteval/rteval_text.xsl b/rteval/rteval_text.xsl
> > index c40063e3dd19..7ecfac6b6140 100644
> > --- a/rteval/rteval_text.xsl
> > +++ b/rteval/rteval_text.xsl
> > @@ -13,6 +13,14 @@
> >      <xsl:value-of select="run_info/date"/><xsl:text> </xsl:text><xsl:value-of select="run_info/time"/>
> >      <xsl:text>&#10;</xsl:text>
> >  
> > +    <xsl:text>   Loads:        </xsl:text>
> > +    <xsl:value-of select="loads/@loads"/><xsl:text> loads run on cores </xsl:text><xsl:value-of select="loads/@loadcpus"/>
> > +    <xsl:text>&#10;</xsl:text>
> > +
> > +    <xsl:text>   Measurement:  </xsl:text>
> > +    <xsl:text>measurement threads run on cores </xsl:text><xsl:value-of select="Measurements/@measurecpus"/>
> > +    <xsl:text>&#10;</xsl:text>
> > +
> >      <xsl:text>   Run time:     </xsl:text>
> >      <xsl:value-of select="run_info/@days"/><xsl:text> days </xsl:text>
> >      <xsl:value-of select="run_info/@hours"/><xsl:text>h </xsl:text>
> > -- 
> > 2.31.1
> > 
> 
> Hi John,
> 
> I do believe the cpulist variable can be empty. I was able to test this by running rteval without specifying and load and measurement cpus.

Ok, you are correct, cpulist can be empty if the following are not set
--loads-cpulist=LIST
--measurement-cpulist=LIST

So you do need some code to initialize the list of it is empty.
But there are still problems

For example if you shut off cpu 3
su -c 'echo 0 > /sys/devices/system/cpu/cpu3/online '

And then request measurement on cpus 0-5
su -c './rteval-cmd -d5s --measurement-cpulist=0-5'

The report should say 
Measurement:  measurement threads run on cores  0-2,4,5
Loads:        3 loads run on cores 6-11 (the inverse)
but instead with your patch it says
Loads:        3 loads run on cores 1,2,4,6,7,8,9,10,11
Measurement:  measurement threads run on cores 0-5

Now there are problems that exist even before your patch, so for example, 
the initial info to the screen not included in the report doesn't consider 
offline cpus. (We should fix that too of course, but we can do this 
stepwise)

And it's also possible that I have introduced some new problems with the 
changes to systopology that will hopefully make things simpler in the long 
run but may be interferring with your code. So, there are many things to 
untangle here.

> 
> This patch is attempting to replicate the behavior found in rteval/__init.py lines 187-201, which also finds the cpulist using
>             cpulist = self._loadmods._cfg.GetSection("loads").cpulist
> and if the cpulist is empty, defaults to reporting the online cpus. I assume this is because the rteval program uses all available cpus
> to run measurement and load threads if no specific cpus are specified.
> 
> Please let me know if I am misunderstanding any of the above.
> 
> I will investigate why the xsl code is not working for you. It seems to be working for me at the moment.


> 
> Thank you for the review,
> Leah
> 
> 
Thanks Leah

John


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

* Re: [PATCH v2] rteval: Add measurement and load location to run report
  2022-08-05 16:32   ` John Kacur
@ 2022-08-05 17:03     ` Leah Leshchinsky
  2022-08-05 17:14       ` John Kacur
  0 siblings, 1 reply; 6+ messages in thread
From: Leah Leshchinsky @ 2022-08-05 17:03 UTC (permalink / raw)
  To: John Kacur; +Cc: linux-rt-users

On Fri, Aug 05, 2022 at 12:32:32PM -0400, John Kacur wrote:
> 
> 
> On Fri, 5 Aug 2022, Leah Leshchinsky wrote:
> 
> > On Thu, Aug 04, 2022 at 11:48:07AM -0400, Leah Leshchinsky wrote:
> > > The run report produced at the end of a run does not contain information
> > > on load and measurement thread locations.
> > > 
> > > Adjust MakeReport() functions of LoadModules and MeasurementModules
> > > class so that new properties with number of loads and cpu information
> > > are added to the XML report and can be read by rteval_text.xsl.
> > > 
> > > Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
> > > 
> > > diff --git a/rteval/modules/loads/__init__.py b/rteval/modules/loads/__init__.py
> > > index 2c2105efa964..5e5a5d85b616 100644
> > > --- a/rteval/modules/loads/__init__.py
> > > +++ b/rteval/modules/loads/__init__.py
> > > @@ -30,6 +30,7 @@ import libxml2
> > >  from rteval.Log import Log
> > >  from rteval.rtevalConfig import rtevalCfgSection
> > >  from rteval.modules import RtEvalModules, rtevalModulePrototype
> > > +from rteval.systopology import collapse_cpulist, SysTopology
> > >  
> > >  class LoadThread(rtevalModulePrototype):
> > >      def __init__(self, name, config, logger=None):
> > > @@ -131,6 +132,12 @@ class LoadModules(RtEvalModules):
> > >      def MakeReport(self):
> > >          rep_n = RtEvalModules.MakeReport(self)
> > >          rep_n.newProp("load_average", str(self.GetLoadAvg()))
> > > +        rep_n.newProp("loads", str(self.ModulesLoaded()))
> > > +        cpulist = self._cfg.GetSection(self._module_config).cpulist
> > > +        if not cpulist:
> > > +            st = SysTopology()
> > > +            cpulist = collapse_cpulist(st.online_cpus())
> > > +        rep_n.newProp("loadcpus", cpulist)
> > >  
> > >          return rep_n
> > >  
> > > diff --git a/rteval/modules/measurement/__init__.py b/rteval/modules/measurement/__init__.py
> > > index 318248bd7e35..371fc5d08f44 100644
> > > --- a/rteval/modules/measurement/__init__.py
> > > +++ b/rteval/modules/measurement/__init__.py
> > > @@ -24,7 +24,7 @@
> > >  
> > >  import libxml2
> > >  from rteval.modules import RtEvalModules, ModuleContainer
> > > -
> > > +from rteval.systopology import collapse_cpulist, SysTopology
> > >  
> > >  class MeasurementProfile(RtEvalModules):
> > >      """Keeps and controls all the measurement modules with the same measurement profile"""
> > > @@ -189,6 +189,12 @@ measurement profiles, based on their characteristics"""
> > >  
> > >          # Get the reports from all meaurement modules in all measurement profiles
> > >          rep_n = libxml2.newNode("Measurements")
> > > +        cpulist = self.__cfg.GetSection("measurement").cpulist
> > > +        if not cpulist:
> > > +            st = SysTopology()
> > > +            cpulist = collapse_cpulist(st.online_cpus())
> > > +        rep_n.newProp("measurecpus", cpulist)
> > > +
> > >          for mp in self.__measureprofiles:
> > >              mprep_n = mp.MakeReport()
> > >              if mprep_n:
> > > diff --git a/rteval/rteval_text.xsl b/rteval/rteval_text.xsl
> > > index c40063e3dd19..7ecfac6b6140 100644
> > > --- a/rteval/rteval_text.xsl
> > > +++ b/rteval/rteval_text.xsl
> > > @@ -13,6 +13,14 @@
> > >      <xsl:value-of select="run_info/date"/><xsl:text> </xsl:text><xsl:value-of select="run_info/time"/>
> > >      <xsl:text>&#10;</xsl:text>
> > >  
> > > +    <xsl:text>   Loads:        </xsl:text>
> > > +    <xsl:value-of select="loads/@loads"/><xsl:text> loads run on cores </xsl:text><xsl:value-of select="loads/@loadcpus"/>
> > > +    <xsl:text>&#10;</xsl:text>
> > > +
> > > +    <xsl:text>   Measurement:  </xsl:text>
> > > +    <xsl:text>measurement threads run on cores </xsl:text><xsl:value-of select="Measurements/@measurecpus"/>
> > > +    <xsl:text>&#10;</xsl:text>
> > > +
> > >      <xsl:text>   Run time:     </xsl:text>
> > >      <xsl:value-of select="run_info/@days"/><xsl:text> days </xsl:text>
> > >      <xsl:value-of select="run_info/@hours"/><xsl:text>h </xsl:text>
> > > -- 
> > > 2.31.1
> > > 
> > 
> > Hi John,
> > 
> > I do believe the cpulist variable can be empty. I was able to test this by running rteval without specifying and load and measurement cpus.
> 
> Ok, you are correct, cpulist can be empty if the following are not set
> --loads-cpulist=LIST
> --measurement-cpulist=LIST
> 
> So you do need some code to initialize the list of it is empty.
> But there are still problems
> 
> For example if you shut off cpu 3
> su -c 'echo 0 > /sys/devices/system/cpu/cpu3/online '
> 
> And then request measurement on cpus 0-5
> su -c './rteval-cmd -d5s --measurement-cpulist=0-5'
> 
> The report should say 
> Measurement:  measurement threads run on cores  0-2,4,5
> Loads:        3 loads run on cores 6-11 (the inverse)
> but instead with your patch it says
> Loads:        3 loads run on cores 1,2,4,6,7,8,9,10,11
> Measurement:  measurement threads run on cores 0-5
> 
> Now there are problems that exist even before your patch, so for example, 
> the initial info to the screen not included in the report doesn't consider 
> offline cpus. (We should fix that too of course, but we can do this 
> stepwise)
> 
> And it's also possible that I have introduced some new problems with the 
> changes to systopology that will hopefully make things simpler in the long 
> run but may be interferring with your code. So, there are many things to 
> untangle here.
> 
> > 
> > This patch is attempting to replicate the behavior found in rteval/__init.py lines 187-201, which also finds the cpulist using
> >             cpulist = self._loadmods._cfg.GetSection("loads").cpulist
> > and if the cpulist is empty, defaults to reporting the online cpus. I assume this is because the rteval program uses all available cpus
> > to run measurement and load threads if no specific cpus are specified.
> > 
> > Please let me know if I am misunderstanding any of the above.
> > 
> > I will investigate why the xsl code is not working for you. It seems to be working for me at the moment.
> 
> 
> > 
> > Thank you for the review,
> > Leah
> > 
> > 
> Thanks Leah
> 
> John
> 

Hi John,

> The report should say
> Measurement:  measurement threads run on cores  0-2,4,5
> Loads:        3 loads run on cores 6-11 (the inverse)
> but instead with your patch it says
> Loads:        3 loads run on cores 1,2,4,6,7,8,9,10,11
> Measurement:  measurement threads run on cores 0-5

Now that I'm looking at the rteval program prior to my changes, I see that this issue existed in the initial pre-report posted to stdout.
This seems to be a separate bug in the way that 'self._loadmods._cfg.GetSection("loads").cpulist' and self.__cfg.GetSection("measurement").cpulist are calculated.

When I test your example on the rteval main branch, this is the result:

su -c 'echo 0 > /sys/devices/system/cpu/cpu3/online '
su -c './rteval-cmd -d5s --measurement-cpulist=0-5'

started 3 loads on cores 1,2,4,6,7
started measurement threads on cores 0-5

Should we file this as a separate bug, since the scope of it goes beyond my current patch?

Thanks,
Leah


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

* Re: [PATCH v2] rteval: Add measurement and load location to run report
  2022-08-05 17:03     ` Leah Leshchinsky
@ 2022-08-05 17:14       ` John Kacur
  0 siblings, 0 replies; 6+ messages in thread
From: John Kacur @ 2022-08-05 17:14 UTC (permalink / raw)
  To: Leah Leshchinsky; +Cc: linux-rt-users



On Fri, 5 Aug 2022, Leah Leshchinsky wrote:

> On Fri, Aug 05, 2022 at 12:32:32PM -0400, John Kacur wrote:
> > 
> > 
> > On Fri, 5 Aug 2022, Leah Leshchinsky wrote:
> > 
> > > On Thu, Aug 04, 2022 at 11:48:07AM -0400, Leah Leshchinsky wrote:
> > > > The run report produced at the end of a run does not contain information
> > > > on load and measurement thread locations.
> > > > 
> > > > Adjust MakeReport() functions of LoadModules and MeasurementModules
> > > > class so that new properties with number of loads and cpu information
> > > > are added to the XML report and can be read by rteval_text.xsl.
> > > > 
> > > > Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
> > > > 
> > > > diff --git a/rteval/modules/loads/__init__.py b/rteval/modules/loads/__init__.py
> > > > index 2c2105efa964..5e5a5d85b616 100644
> > > > --- a/rteval/modules/loads/__init__.py
> > > > +++ b/rteval/modules/loads/__init__.py
> > > > @@ -30,6 +30,7 @@ import libxml2
> > > >  from rteval.Log import Log
> > > >  from rteval.rtevalConfig import rtevalCfgSection
> > > >  from rteval.modules import RtEvalModules, rtevalModulePrototype
> > > > +from rteval.systopology import collapse_cpulist, SysTopology
> > > >  
> > > >  class LoadThread(rtevalModulePrototype):
> > > >      def __init__(self, name, config, logger=None):
> > > > @@ -131,6 +132,12 @@ class LoadModules(RtEvalModules):
> > > >      def MakeReport(self):
> > > >          rep_n = RtEvalModules.MakeReport(self)
> > > >          rep_n.newProp("load_average", str(self.GetLoadAvg()))
> > > > +        rep_n.newProp("loads", str(self.ModulesLoaded()))
> > > > +        cpulist = self._cfg.GetSection(self._module_config).cpulist
> > > > +        if not cpulist:
> > > > +            st = SysTopology()
> > > > +            cpulist = collapse_cpulist(st.online_cpus())
> > > > +        rep_n.newProp("loadcpus", cpulist)
> > > >  
> > > >          return rep_n
> > > >  
> > > > diff --git a/rteval/modules/measurement/__init__.py b/rteval/modules/measurement/__init__.py
> > > > index 318248bd7e35..371fc5d08f44 100644
> > > > --- a/rteval/modules/measurement/__init__.py
> > > > +++ b/rteval/modules/measurement/__init__.py
> > > > @@ -24,7 +24,7 @@
> > > >  
> > > >  import libxml2
> > > >  from rteval.modules import RtEvalModules, ModuleContainer
> > > > -
> > > > +from rteval.systopology import collapse_cpulist, SysTopology
> > > >  
> > > >  class MeasurementProfile(RtEvalModules):
> > > >      """Keeps and controls all the measurement modules with the same measurement profile"""
> > > > @@ -189,6 +189,12 @@ measurement profiles, based on their characteristics"""
> > > >  
> > > >          # Get the reports from all meaurement modules in all measurement profiles
> > > >          rep_n = libxml2.newNode("Measurements")
> > > > +        cpulist = self.__cfg.GetSection("measurement").cpulist
> > > > +        if not cpulist:
> > > > +            st = SysTopology()
> > > > +            cpulist = collapse_cpulist(st.online_cpus())
> > > > +        rep_n.newProp("measurecpus", cpulist)
> > > > +
> > > >          for mp in self.__measureprofiles:
> > > >              mprep_n = mp.MakeReport()
> > > >              if mprep_n:
> > > > diff --git a/rteval/rteval_text.xsl b/rteval/rteval_text.xsl
> > > > index c40063e3dd19..7ecfac6b6140 100644
> > > > --- a/rteval/rteval_text.xsl
> > > > +++ b/rteval/rteval_text.xsl
> > > > @@ -13,6 +13,14 @@
> > > >      <xsl:value-of select="run_info/date"/><xsl:text> </xsl:text><xsl:value-of select="run_info/time"/>
> > > >      <xsl:text>&#10;</xsl:text>
> > > >  
> > > > +    <xsl:text>   Loads:        </xsl:text>
> > > > +    <xsl:value-of select="loads/@loads"/><xsl:text> loads run on cores </xsl:text><xsl:value-of select="loads/@loadcpus"/>
> > > > +    <xsl:text>&#10;</xsl:text>
> > > > +
> > > > +    <xsl:text>   Measurement:  </xsl:text>
> > > > +    <xsl:text>measurement threads run on cores </xsl:text><xsl:value-of select="Measurements/@measurecpus"/>
> > > > +    <xsl:text>&#10;</xsl:text>
> > > > +
> > > >      <xsl:text>   Run time:     </xsl:text>
> > > >      <xsl:value-of select="run_info/@days"/><xsl:text> days </xsl:text>
> > > >      <xsl:value-of select="run_info/@hours"/><xsl:text>h </xsl:text>
> > > > -- 
> > > > 2.31.1
> > > > 
> > > 
> > > Hi John,
> > > 
> > > I do believe the cpulist variable can be empty. I was able to test this by running rteval without specifying and load and measurement cpus.
> > 
> > Ok, you are correct, cpulist can be empty if the following are not set
> > --loads-cpulist=LIST
> > --measurement-cpulist=LIST
> > 
> > So you do need some code to initialize the list of it is empty.
> > But there are still problems
> > 
> > For example if you shut off cpu 3
> > su -c 'echo 0 > /sys/devices/system/cpu/cpu3/online '
> > 
> > And then request measurement on cpus 0-5
> > su -c './rteval-cmd -d5s --measurement-cpulist=0-5'
> > 
> > The report should say 
> > Measurement:  measurement threads run on cores  0-2,4,5
> > Loads:        3 loads run on cores 6-11 (the inverse)
> > but instead with your patch it says
> > Loads:        3 loads run on cores 1,2,4,6,7,8,9,10,11
> > Measurement:  measurement threads run on cores 0-5
> > 
> > Now there are problems that exist even before your patch, so for example, 
> > the initial info to the screen not included in the report doesn't consider 
> > offline cpus. (We should fix that too of course, but we can do this 
> > stepwise)
> > 
> > And it's also possible that I have introduced some new problems with the 
> > changes to systopology that will hopefully make things simpler in the long 
> > run but may be interferring with your code. So, there are many things to 
> > untangle here.
> > 
> > > 
> > > This patch is attempting to replicate the behavior found in rteval/__init.py lines 187-201, which also finds the cpulist using
> > >             cpulist = self._loadmods._cfg.GetSection("loads").cpulist
> > > and if the cpulist is empty, defaults to reporting the online cpus. I assume this is because the rteval program uses all available cpus
> > > to run measurement and load threads if no specific cpus are specified.
> > > 
> > > Please let me know if I am misunderstanding any of the above.
> > > 
> > > I will investigate why the xsl code is not working for you. It seems to be working for me at the moment.
> > 
> > 
> > > 
> > > Thank you for the review,
> > > Leah
> > > 
> > > 
> > Thanks Leah
> > 
> > John
> > 
> 
> Hi John,
> 
> > The report should say
> > Measurement:  measurement threads run on cores  0-2,4,5
> > Loads:        3 loads run on cores 6-11 (the inverse)
> > but instead with your patch it says
> > Loads:        3 loads run on cores 1,2,4,6,7,8,9,10,11
> > Measurement:  measurement threads run on cores 0-5
> 
> Now that I'm looking at the rteval program prior to my changes, I see that this issue existed in the initial pre-report posted to stdout.
> This seems to be a separate bug in the way that 'self._loadmods._cfg.GetSection("loads").cpulist' and self.__cfg.GetSection("measurement").cpulist are calculated.
> 
> When I test your example on the rteval main branch, this is the result:
> 
> su -c 'echo 0 > /sys/devices/system/cpu/cpu3/online '
> su -c './rteval-cmd -d5s --measurement-cpulist=0-5'
> 
> started 3 loads on cores 1,2,4,6,7
> started measurement threads on cores 0-5
> 
> Should we file this as a separate bug, since the scope of it goes beyond my current patch?
> 
> Thanks,
> Leah
> 

Please send a separate patch to fix this.

John


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

end of thread, other threads:[~2022-08-05 17:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-04 15:48 [PATCH v2] rteval: Add measurement and load location to run report Leah Leshchinsky
2022-08-04 23:56 ` John Kacur
2022-08-05 13:37 ` Leah Leshchinsky
2022-08-05 16:32   ` John Kacur
2022-08-05 17:03     ` Leah Leshchinsky
2022-08-05 17:14       ` 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.