* [PATCH] tools/power: amd_pstate_trace: fix help path dependencies @ 2026-06-24 12:31 Yousef Alhouseen 2026-07-22 12:48 ` Rafael J. Wysocki (Intel) 0 siblings, 1 reply; 5+ messages in thread From: Yousef Alhouseen @ 2026-06-24 12:31 UTC (permalink / raw) To: ray.huang, mario.limonciello, perry.yuan Cc: kprateek.nayak, linux-pm, linux-kernel, Yousef Alhouseen amd_pstate_trace imports Gnuplot and numpy before parsing command-line options. As a result, even "-h" fails if those optional runtime modules are not installed. It also handles "-h" by printing a blank line and exiting. Use the existing help printer for "-h", and move the numpy import to the path that needs it. The direct Gnuplot import is unused, so remove it. Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com> --- tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 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 875b08655..b1b201b2e 100755 --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py @@ -27,8 +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")) import intel_pstate_tracer as ipt @@ -260,9 +258,6 @@ graph_data_present = False; valid1 = False valid2 = False -cpu_mask = zeros((MAX_CPUS,), dtype=int) - - try: opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="]) except getopt.GetoptError: @@ -270,7 +265,7 @@ except getopt.GetoptError: sys.exit(2) for opt, arg in opts: if opt == '-h': - print() + ipt.print_help('amd_pstate') sys.exit() elif opt in ("-t", "--trace_file"): valid1 = True @@ -291,6 +286,10 @@ if not (valid1 and valid2): ipt.print_help('amd_pstate') sys.exit() +from numpy import zeros + +cpu_mask = zeros((MAX_CPUS,), dtype=int) + if cpu_list: for p in re.split("[,]", cpu_list): if int(p) < MAX_CPUS : -- 2.54.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/power: amd_pstate_trace: fix help path dependencies 2026-06-24 12:31 [PATCH] tools/power: amd_pstate_trace: fix help path dependencies Yousef Alhouseen @ 2026-07-22 12:48 ` Rafael J. Wysocki (Intel) 2026-07-22 15:21 ` K Prateek Nayak 0 siblings, 1 reply; 5+ messages in thread From: Rafael J. Wysocki (Intel) @ 2026-07-22 12:48 UTC (permalink / raw) To: Yousef Alhouseen, mario.limonciello, kprateek.nayak Cc: ray.huang, perry.yuan, linux-pm, linux-kernel On Wed, Jun 24, 2026 at 2:31 PM Yousef Alhouseen <alhouseenyousef@gmail.com> wrote: > > amd_pstate_trace imports Gnuplot and numpy before parsing command-line > options. As a result, even "-h" fails if those optional runtime modules are > not installed. > > It also handles "-h" by printing a blank line and exiting. Use the existing > help printer for "-h", and move the numpy import to the path that needs it. > The direct Gnuplot import is unused, so remove it. > > Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com> AMD folks, if this change is fine with you, I can take it into pm-tools, but an ACK (or equivalent) is needed. Thanks! > --- > tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------ > 1 file changed, 5 insertions(+), 6 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 875b08655..b1b201b2e 100755 > --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py > +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py > @@ -27,8 +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")) > import intel_pstate_tracer as ipt > @@ -260,9 +258,6 @@ graph_data_present = False; > valid1 = False > valid2 = False > > -cpu_mask = zeros((MAX_CPUS,), dtype=int) > - > - > try: > opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="]) > except getopt.GetoptError: > @@ -270,7 +265,7 @@ except getopt.GetoptError: > sys.exit(2) > for opt, arg in opts: > if opt == '-h': > - print() > + ipt.print_help('amd_pstate') > sys.exit() > elif opt in ("-t", "--trace_file"): > valid1 = True > @@ -291,6 +286,10 @@ if not (valid1 and valid2): > ipt.print_help('amd_pstate') > sys.exit() > > +from numpy import zeros > + > +cpu_mask = zeros((MAX_CPUS,), dtype=int) > + > if cpu_list: > for p in re.split("[,]", cpu_list): > if int(p) < MAX_CPUS : > -- > 2.54.0 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/power: amd_pstate_trace: fix help path dependencies 2026-07-22 12:48 ` Rafael J. Wysocki (Intel) @ 2026-07-22 15:21 ` K Prateek Nayak 2026-07-22 15:32 ` Mario Limonciello 0 siblings, 1 reply; 5+ messages in thread From: K Prateek Nayak @ 2026-07-22 15:21 UTC (permalink / raw) To: Rafael J. Wysocki (Intel), Yousef Alhouseen, mario.limonciello Cc: ray.huang, perry.yuan, linux-pm, linux-kernel Hello Rafael, On 7/22/2026 6:18 PM, Rafael J. Wysocki (Intel) wrote: > On Wed, Jun 24, 2026 at 2:31 PM Yousef Alhouseen > <alhouseenyousef@gmail.com> wrote: >> >> amd_pstate_trace imports Gnuplot and numpy before parsing command-line >> options. As a result, even "-h" fails if those optional runtime modules are >> not installed. >> >> It also handles "-h" by printing a blank line and exiting. Use the existing >> help printer for "-h", and move the numpy import to the path that needs it. >> The direct Gnuplot import is unused, so remove it. >> >> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com> > > AMD folks, if this change is fine with you, I can take it into > pm-tools, but an ACK (or equivalent) is needed. Sorry for the delay in reviewing! I have some comments below for Yousef. > > Thanks! > >> --- >> tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------ >> 1 file changed, 5 insertions(+), 6 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 875b08655..b1b201b2e 100755 >> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py >> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py >> @@ -27,8 +27,6 @@ import re >> import signal >> import sys >> import getopt >> -import Gnuplot We don't use Gnuplot so removing that is fine ... >> -from numpy import * ... but we want to keep all the imports together at top to easily audit dependencies. Afacit, most python scripts in tools/power/x86/ follow that convention. There exists a big fat comment on top of the file reading: Prerequisites: Python version 2.7.x or higher gnuplot 5.0 or higher gnuplot-py 1.8 or higher which is a good indication of whatis actually required to run the script. If users prefer, we can add a requirements.txt that pip can consume to install the necessary dependencies and make it clear. >> from decimal import * >> sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer")) >> import intel_pstate_tracer as ipt >> @@ -260,9 +258,6 @@ graph_data_present = False; >> valid1 = False >> valid2 = False >> >> -cpu_mask = zeros((MAX_CPUS,), dtype=int) >> - >> - >> try: >> opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="]) >> except getopt.GetoptError: >> @@ -270,7 +265,7 @@ except getopt.GetoptError: >> sys.exit(2) >> for opt, arg in opts: >> if opt == '-h': >> - print() >> + ipt.print_help('amd_pstate') >> sys.exit() >> elif opt in ("-t", "--trace_file"): >> valid1 = True >> @@ -291,6 +286,10 @@ if not (valid1 and valid2): >> ipt.print_help('amd_pstate') >> sys.exit() >> >> +from numpy import zeros Having it here needs users to skim through the file to know what external packages the script needs and we don't want that. >> + >> +cpu_mask = zeros((MAX_CPUS,), dtype=int) >> + >> if cpu_list: >> for p in re.split("[,]", cpu_list): >> if int(p) < MAX_CPUS : >> -- >> 2.54.0 >> >> -- Thanks and Regards, Prateek ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/power: amd_pstate_trace: fix help path dependencies 2026-07-22 15:21 ` K Prateek Nayak @ 2026-07-22 15:32 ` Mario Limonciello 2026-07-30 22:08 ` Yousef Alhouseen 0 siblings, 1 reply; 5+ messages in thread From: Mario Limonciello @ 2026-07-22 15:32 UTC (permalink / raw) To: K Prateek Nayak, Rafael J. Wysocki (Intel), Yousef Alhouseen Cc: ray.huang, perry.yuan, linux-pm, linux-kernel On 7/22/26 10:21, K Prateek Nayak wrote: > Hello Rafael, > > On 7/22/2026 6:18 PM, Rafael J. Wysocki (Intel) wrote: >> On Wed, Jun 24, 2026 at 2:31 PM Yousef Alhouseen >> <alhouseenyousef@gmail.com> wrote: >>> >>> amd_pstate_trace imports Gnuplot and numpy before parsing command-line >>> options. As a result, even "-h" fails if those optional runtime modules are >>> not installed. >>> >>> It also handles "-h" by printing a blank line and exiting. Use the existing >>> help printer for "-h", and move the numpy import to the path that needs it. >>> The direct Gnuplot import is unused, so remove it. >>> >>> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com> >> >> AMD folks, if this change is fine with you, I can take it into >> pm-tools, but an ACK (or equivalent) is needed. > > Sorry for the delay in reviewing! I have some comments below for Yousef. Yes; sorry this got buried in my inbox when I was out and I still haven't all the way dug out. > >> >> Thanks! >> >>> --- >>> tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------ >>> 1 file changed, 5 insertions(+), 6 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 875b08655..b1b201b2e 100755 >>> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py >>> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py >>> @@ -27,8 +27,6 @@ import re >>> import signal >>> import sys >>> import getopt >>> -import Gnuplot > > We don't use Gnuplot so removing that is fine ... > >>> -from numpy import * > > ... but we want to keep all the imports together at top to easily audit > dependencies. Afacit, most python scripts in tools/power/x86/ follow > that convention. > > There exists a big fat comment on top of the file reading: > > Prerequisites: > Python version 2.7.x or higher > gnuplot 5.0 or higher > gnuplot-py 1.8 or higher > > which is a good indication of whatis actually required to run the > script. If users prefer, we can add a requirements.txt that pip can > consume to install the necessary dependencies and make it clear. I think a requirements.txt and a quick few steps of how to start a venv and use it is a great idea. > >>> from decimal import * >>> sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer")) >>> import intel_pstate_tracer as ipt >>> @@ -260,9 +258,6 @@ graph_data_present = False; >>> valid1 = False >>> valid2 = False >>> >>> -cpu_mask = zeros((MAX_CPUS,), dtype=int) >>> - >>> - >>> try: >>> opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="]) >>> except getopt.GetoptError: >>> @@ -270,7 +265,7 @@ except getopt.GetoptError: >>> sys.exit(2) >>> for opt, arg in opts: >>> if opt == '-h': >>> - print() >>> + ipt.print_help('amd_pstate') >>> sys.exit() >>> elif opt in ("-t", "--trace_file"): >>> valid1 = True >>> @@ -291,6 +286,10 @@ if not (valid1 and valid2): >>> ipt.print_help('amd_pstate') >>> sys.exit() >>> >>> +from numpy import zeros > > Having it here needs users to skim through the file to know what > external packages the script needs and we don't want that. > >>> + >>> +cpu_mask = zeros((MAX_CPUS,), dtype=int) >>> + >>> if cpu_list: >>> for p in re.split("[,]", cpu_list): >>> if int(p) < MAX_CPUS : >>> -- >>> 2.54.0 >>> >>> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/power: amd_pstate_trace: fix help path dependencies 2026-07-22 15:32 ` Mario Limonciello @ 2026-07-30 22:08 ` Yousef Alhouseen 0 siblings, 0 replies; 5+ messages in thread From: Yousef Alhouseen @ 2026-07-30 22:08 UTC (permalink / raw) To: kprateek.nayak, rafael, mario.limonciello Cc: ray.huang, perry.yuan, linux-pm, linux-kernel Hi Prateek, Mario, Agreed. Moving the NumPy import below argument parsing is not the right way to manage a required dependency. Please drop this version. I will keep the NumPy import at module scope, split out the narrow -h output fix, and handle dependency/venv documentation separately after verifying the shared requirements for both tracers. The unused Gnuplot import in amd_pstate_trace can still be removed as a separate cleanup. Thanks for the guidance. Regards, Yousef On Wed, 22 Jul 2026 10:32:41 -0500, Mario Limonciello <mario.limonciello@amd.com> wrote: > On 7/22/26 10:21, K Prateek Nayak wrote: > > Hello Rafael, > > > > On 7/22/2026 6:18 PM, Rafael J. Wysocki (Intel) wrote: > >> On Wed, Jun 24, 2026 at 2:31 PM Yousef Alhouseen > >> <alhouseenyousef@gmail.com> wrote: > >>> > >>> amd_pstate_trace imports Gnuplot and numpy before parsing command-line > >>> options. As a result, even "-h" fails if those optional runtime modules are > >>> not installed. > >>> > >>> It also handles "-h" by printing a blank line and exiting. Use the existing > >>> help printer for "-h", and move the numpy import to the path that needs it. > >>> The direct Gnuplot import is unused, so remove it. > >>> > >>> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com> > >> > >> AMD folks, if this change is fine with you, I can take it into > >> pm-tools, but an ACK (or equivalent) is needed. > > > > Sorry for the delay in reviewing! I have some comments below for Yousef. > > Yes; sorry this got buried in my inbox when I was out and I still > haven't all the way dug out. > > > > >> > >> Thanks! > >> > >>> --- > >>> tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------ > >>> 1 file changed, 5 insertions(+), 6 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 875b08655..b1b201b2e 100755 > >>> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py > >>> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py > >>> @@ -27,8 +27,6 @@ import re > >>> import signal > >>> import sys > >>> import getopt > >>> -import Gnuplot > > > > We don't use Gnuplot so removing that is fine ... > > > >>> -from numpy import * > > > > ... but we want to keep all the imports together at top to easily audit > > dependencies. Afacit, most python scripts in tools/power/x86/ follow > > that convention. > > > > There exists a big fat comment on top of the file reading: > > > > Prerequisites: > > Python version 2.7.x or higher > > gnuplot 5.0 or higher > > gnuplot-py 1.8 or higher > > > > which is a good indication of whatis actually required to run the > > script. If users prefer, we can add a requirements.txt that pip can > > consume to install the necessary dependencies and make it clear. > > I think a requirements.txt and a quick few steps of how to start a venv > and use it is a great idea. > > > > >>> from decimal import * > >>> sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer")) > >>> import intel_pstate_tracer as ipt > >>> @@ -260,9 +258,6 @@ graph_data_present = False; > >>> valid1 = False > >>> valid2 = False > >>> > >>> -cpu_mask = zeros((MAX_CPUS,), dtype=int) > >>> - > >>> - > >>> try: > >>> opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="]) > >>> except getopt.GetoptError: > >>> @@ -270,7 +265,7 @@ except getopt.GetoptError: > >>> sys.exit(2) > >>> for opt, arg in opts: > >>> if opt == '-h': > >>> - print() > >>> + ipt.print_help('amd_pstate') > >>> sys.exit() > >>> elif opt in ("-t", "--trace_file"): > >>> valid1 = True > >>> @@ -291,6 +286,10 @@ if not (valid1 and valid2): > >>> ipt.print_help('amd_pstate') > >>> sys.exit() > >>> > >>> +from numpy import zeros > > > > Having it here needs users to skim through the file to know what > > external packages the script needs and we don't want that. > > > >>> + > >>> +cpu_mask = zeros((MAX_CPUS,), dtype=int) > >>> + > >>> if cpu_list: > >>> for p in re.split("[,]", cpu_list): > >>> if int(p) < MAX_CPUS : > >>> -- > >>> 2.54.0 > >>> > >>> > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-30 22:09 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-24 12:31 [PATCH] tools/power: amd_pstate_trace: fix help path dependencies Yousef Alhouseen 2026-07-22 12:48 ` Rafael J. Wysocki (Intel) 2026-07-22 15:21 ` K Prateek Nayak 2026-07-22 15:32 ` Mario Limonciello 2026-07-30 22:08 ` Yousef Alhouseen
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).