mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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	[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

all inboxes | Powered by JetHome®