* [PATCH 0/2] Fix issues observed with selftests/amd-pstate
@ 2023-09-15 10:40 Swapnil Sapkal
2023-09-15 10:40 ` [PATCH 1/2] selftests/amd-pstate: Fix broken paths to run workloads in amd-pstate-ut Swapnil Sapkal
2023-09-15 10:40 ` [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot Swapnil Sapkal
0 siblings, 2 replies; 8+ messages in thread
From: Swapnil Sapkal @ 2023-09-15 10:40 UTC (permalink / raw)
To: rafael.j.wysocki, Ray.Huang, li.meng, shuah
Cc: sukrut.bellary, gautham.shenoy, wyes.karny, Perry.Yuan,
Mario.Limonciello, zwisler, linux-pm, linux-kernel,
linux-kselftest, Swapnil Sapkal
This series fixes the issues observed with selftests/amd-pstate while
running performance comparison tests with different governors. First
patch changes relative paths with absolute path and also change it with
correct path wherever it is broken.
The second patch fixes error observed while importing the Gnuplot in
intel_pstate_tracer.py.
Swapnil Sapkal (2):
selftests/amd-pstate: Fix broken paths to run workloads in
amd-pstate-ut
tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot
.../x86/amd_pstate_tracer/amd_pstate_trace.py | 3 +--
.../intel_pstate_tracer.py | 4 ++--
.../testing/selftests/amd-pstate/gitsource.sh | 14 +++++++-----
tools/testing/selftests/amd-pstate/run.sh | 22 +++++++++++++------
tools/testing/selftests/amd-pstate/tbench.sh | 4 ++--
5 files changed, 29 insertions(+), 18 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] selftests/amd-pstate: Fix broken paths to run workloads in amd-pstate-ut
2023-09-15 10:40 [PATCH 0/2] Fix issues observed with selftests/amd-pstate Swapnil Sapkal
@ 2023-09-15 10:40 ` Swapnil Sapkal
2023-09-15 10:40 ` [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot Swapnil Sapkal
1 sibling, 0 replies; 8+ messages in thread
From: Swapnil Sapkal @ 2023-09-15 10:40 UTC (permalink / raw)
To: rafael.j.wysocki, Ray.Huang, li.meng, shuah
Cc: sukrut.bellary, gautham.shenoy, wyes.karny, Perry.Yuan,
Mario.Limonciello, zwisler, linux-pm, linux-kernel,
linux-kselftest, Swapnil Sapkal
In selftests/amd-pstate, tbench and gitsource microbenchmarks are used to
compare the performance with different governors. In Current
implementation relative path to run `amd_pstate_tracer.py`
broken. Fixed this by using absolute paths.
Also selftests/amd-pstate uses distro `perf` to capture stats while running
these microbenchmarks. Distro `perf` is not working with upstream
kernel. Fixed this by providing an option to give the perf binary path.
Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
---
.../x86/amd_pstate_tracer/amd_pstate_trace.py | 2 +-
.../testing/selftests/amd-pstate/gitsource.sh | 14 +++++++-----
tools/testing/selftests/amd-pstate/run.sh | 22 +++++++++++++------
tools/testing/selftests/amd-pstate/tbench.sh | 4 ++--
4 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
index 904df0ea0a1e..2448bb07973f 100755
--- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
+++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
@@ -30,7 +30,7 @@ import getopt
import Gnuplot
from numpy import *
from decimal import *
-sys.path.append('../intel_pstate_tracer')
+sys.path.append(os.path.join(os.path.dirname(__file__), '../intel_pstate_tracer'))
#import intel_pstate_tracer
import intel_pstate_tracer as ipt
diff --git a/tools/testing/selftests/amd-pstate/gitsource.sh b/tools/testing/selftests/amd-pstate/gitsource.sh
index 5f2171f0116d..c327444d3506 100755
--- a/tools/testing/selftests/amd-pstate/gitsource.sh
+++ b/tools/testing/selftests/amd-pstate/gitsource.sh
@@ -66,12 +66,15 @@ post_clear_gitsource()
install_gitsource()
{
- if [ ! -d $git_name ]; then
+ if [ ! -d $SCRIPTDIR/$git_name ]; then
+ BACKUP_DIR=$(pwd)
+ cd $SCRIPTDIR
printf "Download gitsource, please wait a moment ...\n\n"
wget -O $git_tar $gitsource_url > /dev/null 2>&1
printf "Tar gitsource ...\n\n"
tar -xzf $git_tar
+ cd $BACKUP_DIR
fi
}
@@ -79,12 +82,13 @@ install_gitsource()
run_gitsource()
{
echo "Launching amd pstate tracer for $1 #$2 tracer_interval: $TRACER_INTERVAL"
- ./amd_pstate_trace.py -n tracer-gitsource-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &
+ $SCRIPTDIR/../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py -n tracer-gitsource-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &
printf "Make and test gitsource for $1 #$2 make_cpus: $MAKE_CPUS\n"
- cd $git_name
- perf stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o ../$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > ../$OUTFILE_GIT-perf-$1-$2.log 2>&1
- cd ..
+ BACKUP_DIR=$(pwd)
+ cd $SCRIPTDIR/$git_name
+ $PERF stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o $BACKUP_DIR/$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > $BACKUP_DIR/$OUTFILE_GIT-perf-$1-$2.log 2>&1
+ cd $BACKUP_DIR
for job in `jobs -p`
do
diff --git a/tools/testing/selftests/amd-pstate/run.sh b/tools/testing/selftests/amd-pstate/run.sh
index de4d8e9c9565..0803e70b04da 100755
--- a/tools/testing/selftests/amd-pstate/run.sh
+++ b/tools/testing/selftests/amd-pstate/run.sh
@@ -8,9 +8,11 @@ else
FILE_MAIN=DONE
fi
-source basic.sh
-source tbench.sh
-source gitsource.sh
+SCRIPTDIR=`dirname "$0"`
+
+source $SCRIPTDIR/basic.sh
+source $SCRIPTDIR/tbench.sh
+source $SCRIPTDIR/gitsource.sh
# amd-pstate-ut only run on x86/x86_64 AMD systems.
ARCH=$(uname -m 2>/dev/null | sed -e 's/i.86/x86/' -e 's/x86_64/x86/')
@@ -22,6 +24,7 @@ OUTFILE=selftest
OUTFILE_TBENCH="$OUTFILE.tbench"
OUTFILE_GIT="$OUTFILE.gitsource"
+PERF=/usr/bin/perf
SYSFS=
CPUROOT=
CPUFREQROOT=
@@ -149,8 +152,9 @@ help()
gitsource: Gitsource testing.>]
[-t <tbench time limit>]
[-p <tbench process number>]
- [-l <loop times for tbench>]
+ [-l <loop times for tbench/gitsource>]
[-i <amd tracer interval>]
+ [-b <perf binary>]
[-m <comparative test: acpi-cpufreq>]
\n"
exit 2
@@ -158,7 +162,7 @@ help()
parse_arguments()
{
- while getopts ho:c:t:p:l:i:m: arg
+ while getopts ho:c:t:p:l:i:b:m: arg
do
case $arg in
h) # --help
@@ -189,6 +193,10 @@ parse_arguments()
TRACER_INTERVAL=$OPTARG
;;
+ b) # --perf-binary
+ PERF=`realpath $OPTARG`
+ ;;
+
m) # --comparative-test
COMPARATIVE_TEST=$OPTARG
;;
@@ -202,8 +210,8 @@ parse_arguments()
command_perf()
{
- if ! command -v perf > /dev/null; then
- echo $msg please install perf. >&2
+ if ! $PERF -v; then
+ echo $msg please install perf or provide perf binary path as argument >&2
exit $ksft_skip
fi
}
diff --git a/tools/testing/selftests/amd-pstate/tbench.sh b/tools/testing/selftests/amd-pstate/tbench.sh
index 49c9850341f6..70e5863e74ea 100755
--- a/tools/testing/selftests/amd-pstate/tbench.sh
+++ b/tools/testing/selftests/amd-pstate/tbench.sh
@@ -64,11 +64,11 @@ post_clear_tbench()
run_tbench()
{
echo "Launching amd pstate tracer for $1 #$2 tracer_interval: $TRACER_INTERVAL"
- ./amd_pstate_trace.py -n tracer-tbench-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &
+ $SCRIPTDIR/../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py -n tracer-tbench-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &
printf "Test tbench for $1 #$2 time_limit: $TIME_LIMIT procs_num: $PROCESS_NUM\n"
tbench_srv > /dev/null 2>&1 &
- perf stat -a --per-socket -I 1000 -e power/energy-pkg/ tbench -t $TIME_LIMIT $PROCESS_NUM > $OUTFILE_TBENCH-perf-$1-$2.log 2>&1
+ $PERF stat -a --per-socket -I 1000 -e power/energy-pkg/ tbench -t $TIME_LIMIT $PROCESS_NUM > $OUTFILE_TBENCH-perf-$1-$2.log 2>&1
pid=`pidof tbench_srv`
kill $pid
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot
2023-09-15 10:40 [PATCH 0/2] Fix issues observed with selftests/amd-pstate Swapnil Sapkal
2023-09-15 10:40 ` [PATCH 1/2] selftests/amd-pstate: Fix broken paths to run workloads in amd-pstate-ut Swapnil Sapkal
@ 2023-09-15 10:40 ` Swapnil Sapkal
2023-09-15 21:15 ` Doug Smythies
1 sibling, 1 reply; 8+ messages in thread
From: Swapnil Sapkal @ 2023-09-15 10:40 UTC (permalink / raw)
To: rafael.j.wysocki, Ray.Huang, li.meng, shuah
Cc: sukrut.bellary, gautham.shenoy, wyes.karny, Perry.Yuan,
Mario.Limonciello, zwisler, linux-pm, linux-kernel,
linux-kselftest, Swapnil Sapkal
In intel_pstate_tracer.py, Gnuplot is used to generate 2D plots.
In current implementation this tracer gives error while importing
the module because Gnuplot is imported from package Gnuplot-py which
does not support python 3.x. Fix this by using pygnuplot package to
import this module.
Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
---
tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 1 -
tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
index 2448bb07973f..14f8d81f91de 100755
--- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
+++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
@@ -27,7 +27,6 @@ import re
import signal
import sys
import getopt
-import Gnuplot
from numpy import *
from decimal import *
sys.path.append(os.path.join(os.path.dirname(__file__), '../intel_pstate_tracer'))
diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
index ec3323100e1a..68412abdd7d4 100755
--- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
+++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
@@ -32,7 +32,7 @@ import re
import signal
import sys
import getopt
-import Gnuplot
+from pygnuplot import gnuplot
from numpy import *
from decimal import *
@@ -295,7 +295,7 @@ def common_all_gnuplot_settings(output_png):
def common_gnuplot_settings():
""" common gnuplot settings. """
- g_plot = Gnuplot.Gnuplot(persist=1)
+ g_plot = gnuplot.Gnuplot(persist=1)
# The following line is for rigor only. It seems to be assumed for .csv files
g_plot('set datafile separator \",\"')
g_plot('set ytics nomirror')
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot
2023-09-15 10:40 ` [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot Swapnil Sapkal
@ 2023-09-15 21:15 ` Doug Smythies
2023-09-15 21:31 ` Mario Limonciello
0 siblings, 1 reply; 8+ messages in thread
From: Doug Smythies @ 2023-09-15 21:15 UTC (permalink / raw)
To: 'Swapnil Sapkal', rafael.j.wysocki, Ray.Huang, li.meng,
shuah
Cc: sukrut.bellary, gautham.shenoy, wyes.karny, Perry.Yuan,
Mario.Limonciello, zwisler, linux-pm, linux-kernel,
linux-kselftest, 'Srinivas Pandruvada', Doug Smythies
On 2023.09.15 03:41 Swapnil Sapkal wrote:
> In intel_pstate_tracer.py, Gnuplot is used to generate 2D plots.
> In current implementation this tracer gives error while importing
> the module because Gnuplot is imported from package Gnuplot-py which
> does not support python 3.x. Fix this by using pygnuplot package to
> import this module.
As described in the prerequisites section, the package name is distribution dependant.
On my distribution the original package name is phython3-gnuplot,
and it is working fine.
sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)
I don't currently have python3-pygnuplot installed, and so this patch breaks
the intel_pstate_tracer for me.
So, I installed the python3-pygnuplot package, and it still didn't work, as there
still wasn't a pygnuplot module to import.
So, I found something called PyGnuplot.py and so changed to that and got further.
But then it got upset with:
File "./intel_pstate_tracer.py.amd", line 298, in common_gnuplot_settings
g_plot = gnuplot.Gnuplot(persist=1)
NameError: name 'gnuplot' is not defined
I gave up and returned to the unpatched
intel_pstate_tracer.py
And checked that is still worked fine. It did.
So, I do not accept this proposed patch.
Not really related, but for a few years now I have been meaning to
change the minimum python version prerequisite to >= 3.0 and
to change the shebang line from this:
#!/usr/bin/env python
To this:
#!/usr/bin/env python3
I have to use the latter version on my distro.
Back when I looked into it, things were inconsistent,
so I didn't know what to do. The kernel tree has 52 .py files
of the latter shebang and 11 of the former.
... Doug
> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
> ---
> tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 1 -
> tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> index 2448bb07973f..14f8d81f91de 100755
> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> @@ -27,7 +27,6 @@ import re
> import signal
> import sys
> import getopt
> -import Gnuplot
> from numpy import *
> from decimal import *
> sys.path.append(os.path.join(os.path.dirname(__file__), '../intel_pstate_tracer'))
> diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> index ec3323100e1a..68412abdd7d4 100755
> --- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> +++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> @@ -32,7 +32,7 @@ import re
> import signal
> import sys
> import getopt
> -import Gnuplot
> +from pygnuplot import gnuplot
> from numpy import *
> from decimal import *
>
> @@ -295,7 +295,7 @@ def common_all_gnuplot_settings(output_png):
> def common_gnuplot_settings():
> """ common gnuplot settings. """
>
> - g_plot = Gnuplot.Gnuplot(persist=1)
> + g_plot = gnuplot.Gnuplot(persist=1)
> # The following line is for rigor only. It seems to be assumed for .csv files
> g_plot('set datafile separator \",\"')
> g_plot('set ytics nomirror')
> --
> 2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot
2023-09-15 21:15 ` Doug Smythies
@ 2023-09-15 21:31 ` Mario Limonciello
2023-09-15 22:16 ` Doug Smythies
0 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2023-09-15 21:31 UTC (permalink / raw)
To: Doug Smythies, 'Swapnil Sapkal', rafael.j.wysocki,
Ray.Huang, li.meng, shuah
Cc: sukrut.bellary, gautham.shenoy, wyes.karny, Perry.Yuan, zwisler,
linux-pm, linux-kernel, linux-kselftest,
'Srinivas Pandruvada'
On 9/15/2023 16:15, Doug Smythies wrote:
> On 2023.09.15 03:41 Swapnil Sapkal wrote:
>
>> In intel_pstate_tracer.py, Gnuplot is used to generate 2D plots.
>> In current implementation this tracer gives error while importing
>> the module because Gnuplot is imported from package Gnuplot-py which
>> does not support python 3.x. Fix this by using pygnuplot package to
>> import this module.
>
> As described in the prerequisites section, the package name is distribution dependant.
> On my distribution the original package name is phython3-gnuplot,
> and it is working fine.
>
> sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)
>
> I don't currently have python3-pygnuplot installed, and so this patch breaks
> the intel_pstate_tracer for me.
>
> So, I installed the python3-pygnuplot package, and it still didn't work, as there
> still wasn't a pygnuplot module to import.
> So, I found something called PyGnuplot.py and so changed to that and got further.
> But then it got upset with:
>
> File "./intel_pstate_tracer.py.amd", line 298, in common_gnuplot_settings
> g_plot = gnuplot.Gnuplot(persist=1)
> NameError: name 'gnuplot' is not defined
>
> I gave up and returned to the unpatched
> intel_pstate_tracer.py
> And checked that is still worked fine. It did.
>
> So, I do not accept this proposed patch.
>
> Not really related, but for a few years now I have been meaning to
> change the minimum python version prerequisite to >= 3.0 and
> to change the shebang line from this:
>
> #!/usr/bin/env python
>
> To this:
>
> #!/usr/bin/env python3
>
> I have to use the latter version on my distro.
> Back when I looked into it, things were inconsistent,
> so I didn't know what to do. The kernel tree has 52 .py files
> of the latter shebang and 11 of the former.
>
> ... Doug
Presumably this is the one that Swapnil intended:
https://pypi.org/project/py-gnuplot/
It requires python3, so I think if upgrading to this one the script does
need to be switched to python3. Besides the shebang, you should also
use a helper like 2to3 to look for any other changes.
There were 97 hits for 'gnuplot' at pypi. 2 stood out but at least in
the case of gnuplot based stuff, I think it's worth dropping
a comment that links back to pypi page for the intended package.
Another alternative is to include a 'requirements.txt' file that pip can
pick up.
https://pip.pypa.io/en/stable/reference/requirements-file-format/
>
>> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
>> ---
>> tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 1 -
>> tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
>> 2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> index 2448bb07973f..14f8d81f91de 100755
>> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> @@ -27,7 +27,6 @@ import re
>> import signal
>> import sys
>> import getopt
>> -import Gnuplot
>> from numpy import *
>> from decimal import *
>> sys.path.append(os.path.join(os.path.dirname(__file__), '../intel_pstate_tracer'))
>> diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
>> index ec3323100e1a..68412abdd7d4 100755
>> --- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
>> +++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
>> @@ -32,7 +32,7 @@ import re
>> import signal
>> import sys
>> import getopt
>> -import Gnuplot
>> +from pygnuplot import gnuplot
>> from numpy import *
>> from decimal import *
>>
>> @@ -295,7 +295,7 @@ def common_all_gnuplot_settings(output_png):
>> def common_gnuplot_settings():
>> """ common gnuplot settings. """
>>
>> - g_plot = Gnuplot.Gnuplot(persist=1)
>> + g_plot = gnuplot.Gnuplot(persist=1)
>> # The following line is for rigor only. It seems to be assumed for .csv files
>> g_plot('set datafile separator \",\"')
>> g_plot('set ytics nomirror')
>> --
>> 2.34.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot
2023-09-15 21:31 ` Mario Limonciello
@ 2023-09-15 22:16 ` Doug Smythies
2023-09-17 21:43 ` Doug Smythies
0 siblings, 1 reply; 8+ messages in thread
From: Doug Smythies @ 2023-09-15 22:16 UTC (permalink / raw)
To: Mario Limonciello, Swapnil Sapkal
Cc: rafael.j.wysocki, Ray.Huang, li.meng, shuah, sukrut.bellary,
gautham.shenoy, wyes.karny, Perry.Yuan, zwisler, linux-pm,
linux-kernel, linux-kselftest, Srinivas Pandruvada, Doug Smythies
On Fri, Sep 15, 2023 at 2:31 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
> On 9/15/2023 16:15, Doug Smythies wrote:
> > On 2023.09.15 03:41 Swapnil Sapkal wrote:
> >
> >> In intel_pstate_tracer.py, Gnuplot is used to generate 2D plots.
> >> In current implementation this tracer gives error while importing
> >> the module because Gnuplot is imported from package Gnuplot-py which
> >> does not support python 3.x. Fix this by using pygnuplot package to
> >> import this module.
> >
> > As described in the prerequisites section, the package name is distribution dependant.
> > On my distribution the original package name is phython3-gnuplot,
> > and it is working fine.
> >
> > sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)
> >
> > I don't currently have python3-pygnuplot installed, and so this patch breaks
> > the intel_pstate_tracer for me.
> >
> > So, I installed the python3-pygnuplot package, and it still didn't work, as there
> > still wasn't a pygnuplot module to import.
> > So, I found something called PyGnuplot.py and so changed to that and got further.
> > But then it got upset with:
> >
> > File "./intel_pstate_tracer.py.amd", line 298, in common_gnuplot_settings
> > g_plot = gnuplot.Gnuplot(persist=1)
> > NameError: name 'gnuplot' is not defined
> >
> > I gave up and returned to the unpatched
> > intel_pstate_tracer.py
> > And checked that is still worked fine. It did.
> >
> > So, I do not accept this proposed patch.
> >
> > Not really related, but for a few years now I have been meaning to
> > change the minimum python version prerequisite to >= 3.0 and
> > to change the shebang line from this:
> >
> > #!/usr/bin/env python
> >
> > To this:
> >
> > #!/usr/bin/env python3
> >
> > I have to use the latter version on my distro.
> > Back when I looked into it, things were inconsistent,
> > so I didn't know what to do. The kernel tree has 52 .py files
> > of the latter shebang and 11 of the former.
> >
> > ... Doug
>
> Presumably this is the one that Swapnil intended:
>
> https://pypi.org/project/py-gnuplot/
Yes, I found that earlier.
For my part of it, I do not want to use any out-of-distro package.
> It requires python3, so I think if upgrading to this one the script does
> need to be switched to python3. Besides the shebang, you should also
> use a helper like 2to3 to look for any other changes.
I already did the python 3 patch in January, 2020:
commit e749e09db30c38f1a275945814b0109e530a07b0
tools/power/x86/intel_pstate_tracer: changes for python 3 compatibility
I haven't had any issues since, shebang aside.
... Doug
> There were 97 hits for 'gnuplot' at pypi. 2 stood out but at least in
> the case of gnuplot based stuff, I think it's worth dropping
> a comment that links back to pypi page for the intended package.
>
> Another alternative is to include a 'requirements.txt' file that pip can
> pick up.
>
> https://pip.pypa.io/en/stable/reference/requirements-file-format/
>
> >> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
> >> ---
> >> tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 1 -
> >> tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
> >> 2 files changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> >> index 2448bb07973f..14f8d81f91de 100755
> >> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> >> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> >> @@ -27,7 +27,6 @@ import re
> >> import signal
> >> import sys
> >> import getopt
> >> -import Gnuplot
> >> from numpy import *
> >> from decimal import *
> >> sys.path.append(os.path.join(os.path.dirname(__file__), '../intel_pstate_tracer'))
> >> diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> >> index ec3323100e1a..68412abdd7d4 100755
> >> --- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> >> +++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> >> @@ -32,7 +32,7 @@ import re
> >> import signal
> >> import sys
> >> import getopt
> >> -import Gnuplot
> >> +from pygnuplot import gnuplot
> >> from numpy import *
> >> from decimal import *
> >>
> >> @@ -295,7 +295,7 @@ def common_all_gnuplot_settings(output_png):
> >> def common_gnuplot_settings():
> >> """ common gnuplot settings. """
> >>
> >> - g_plot = Gnuplot.Gnuplot(persist=1)
> >> + g_plot = gnuplot.Gnuplot(persist=1)
> >> # The following line is for rigor only. It seems to be assumed for .csv files
> >> g_plot('set datafile separator \",\"')
> >> g_plot('set ytics nomirror')
> >> --
> >> 2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot
2023-09-15 22:16 ` Doug Smythies
@ 2023-09-17 21:43 ` Doug Smythies
2023-09-19 7:36 ` Swapnil Sapkal
0 siblings, 1 reply; 8+ messages in thread
From: Doug Smythies @ 2023-09-17 21:43 UTC (permalink / raw)
To: 'Mario Limonciello', 'Swapnil Sapkal'
Cc: rafael.j.wysocki, Ray.Huang, li.meng, shuah, sukrut.bellary,
gautham.shenoy, wyes.karny, Perry.Yuan, zwisler, linux-pm,
linux-kernel, linux-kselftest, 'Srinivas Pandruvada',
Doug Smythies
On 2023.09.17 15:17 Doug wrote:
> On Fri, Sep 15, 2023 at 2:31 PM Mario Limonciello
>> On 9/15/2023 16:15, Doug Smythies wrote:
>>> On 2023.09.15 03:41 Swapnil Sapkal wrote:
...
>>> Not really related, but for a few years now I have been meaning to
>>> change the minimum python version prerequisite to >= 3.0 and
>>> to change the shebang line.
...
>> Besides the shebang, you should also
>> use a helper like 2to3 to look for any other changes.
Hi Mario,
I was not aware of the 2to3 helper.
Thank you mentioning it.
The 2to3 helper only changed one line,
which I included in the minimum python version
patch I just submitted.
> I already did the python 3 patch in January, 2020:
> commit e749e09db30c38f1a275945814b0109e530a07b0
> tools/power/x86/intel_pstate_tracer: changes for python 3 compatibility
>
> I haven't had any issues since, shebang aside.
... Doug
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot
2023-09-17 21:43 ` Doug Smythies
@ 2023-09-19 7:36 ` Swapnil Sapkal
0 siblings, 0 replies; 8+ messages in thread
From: Swapnil Sapkal @ 2023-09-19 7:36 UTC (permalink / raw)
To: Doug Smythies, 'Mario Limonciello'
Cc: rafael.j.wysocki, Ray.Huang, li.meng, shuah, sukrut.bellary,
gautham.shenoy, wyes.karny, Perry.Yuan, zwisler, linux-pm,
linux-kernel, linux-kselftest, 'Srinivas Pandruvada'
Hello Doug,
Sorry for delay in response.
On 9/18/2023 3:13 AM, Doug Smythies wrote:
> On 2023.09.17 15:17 Doug wrote:
>> On Fri, Sep 15, 2023 at 2:31 PM Mario Limonciello
>>> On 9/15/2023 16:15, Doug Smythies wrote:
>>>> On 2023.09.15 03:41 Swapnil Sapkal wrote:
> ...
>>>> Not really related, but for a few years now I have been meaning to
>>>> change the minimum python version prerequisite to >= 3.0 and
>>>> to change the shebang line.
> ...
>>> Besides the shebang, you should also
>>> use a helper like 2to3 to look for any other changes.
>
> Hi Mario,
>
> I was not aware of the 2to3 helper.
> Thank you mentioning it.
> The 2to3 helper only changed one line,
> which I included in the minimum python version
> patch I just submitted.
>
I tried with installing python3-gnuplot and it worked for me. Initially I
tried with other packages in prerequisites and it didn't worked for me.
I will send v2 removing this patch.
>> I already did the python 3 patch in January, 2020:
>> commit e749e09db30c38f1a275945814b0109e530a07b0
>> tools/power/x86/intel_pstate_tracer: changes for python 3 compatibility
>>
>> I haven't had any issues since, shebang aside.
>
> ... Doug
>
>
--
Thanks and Regards,
Swapnil
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-19 7:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-15 10:40 [PATCH 0/2] Fix issues observed with selftests/amd-pstate Swapnil Sapkal
2023-09-15 10:40 ` [PATCH 1/2] selftests/amd-pstate: Fix broken paths to run workloads in amd-pstate-ut Swapnil Sapkal
2023-09-15 10:40 ` [PATCH 2/2] tools/power/x86/intel_pstate_tracer: Use pygnuplot package for Gnuplot Swapnil Sapkal
2023-09-15 21:15 ` Doug Smythies
2023-09-15 21:31 ` Mario Limonciello
2023-09-15 22:16 ` Doug Smythies
2023-09-17 21:43 ` Doug Smythies
2023-09-19 7:36 ` Swapnil Sapkal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).