diff mbox series

[v2] selftests: amd-pstate: Don't delete source files via Makefile

Message ID 000601d92db7$39e9d0b0$adbd7210$@telus.net (mailing list archive)
State New
Headers show
Series [v2] selftests: amd-pstate: Don't delete source files via Makefile | expand

Commit Message

Doug Smythies Jan. 21, 2023, 4:41 p.m. UTC 
Revert the portion of a recent Makefile change that incorrectly
deletes source files when doing "make clean".

Fixes: ba2d788aa873 ("selftests: amd-pstate: Trigger tbench benchmark and test cpus")
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>
Acked-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Doug Smythies <dsmythies@telus.net>
---
v2: fix address list.
---
 tools/testing/selftests/amd-pstate/Makefile | 5 -----
 1 file changed, 5 deletions(-)

--
2.25.1

Comments

Sedat Dilek Jan. 21, 2023, 4:58 p.m. UTC | #1 
On Sat, Jan 21, 2023 at 5:41 PM Doug Smythies <dsmythies@telus.net> wrote:
>
> Revert the portion of a recent Makefile change that incorrectly
> deletes source files when doing "make clean".
>
> Fixes: ba2d788aa873 ("selftests: amd-pstate: Trigger tbench benchmark and test cpus")
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Doug Smythies <dsmythies@telus.net>
> ---
> v2: fix address list.
> ---

Hi Doug,

thanks for the update.

You happen to know through which Git tree this will go?
linux-pm?
linux-kselftest?

https://lore.kernel.org/lkml/?q=Doug+Smythies

Hmm, I cannot download the patch with b4:

link="https://lore.kernel.org/lkml/000601d92db7$39e9d0b0$adbd7210$@telus.net/"

$ b4 -d am $link
Running git --no-pager config -z --get-regexp b4\..*
Running git --no-pager config -z --get-regexp get-lore-mbox\..*
Running git --no-pager config -z --get-regexp gpg\..*
t_mbx_url=https://lore.kernel.org/all/https%3A%2F%2Flore.kernel.org%2Flkml%2F000601d92db79e9d0b0telus.net%2F/t.mbox.gz
Grabbing thread from
lore.kernel.org/all/https%3A%2F%2Flore.kernel.org%2Flkml%2F000601d92db79e9d0b0telus.net%2F/t.mbox.gz
That message-id is not known.

Maybe be patient.

BR,
-Sedat-

>  tools/testing/selftests/amd-pstate/Makefile | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/tools/testing/selftests/amd-pstate/Makefile b/tools/testing/selftests/amd-pstate/Makefile
> index 5f195ee756d6..5fd1424db37d 100644
> --- a/tools/testing/selftests/amd-pstate/Makefile
> +++ b/tools/testing/selftests/amd-pstate/Makefile
> @@ -7,11 +7,6 @@ all:
>  uname_M := $(shell uname -m 2>/dev/null || echo not)
>  ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
>
> -ifeq (x86,$(ARCH))
> -TEST_GEN_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
> -TEST_GEN_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> -endif
> -
>  TEST_PROGS := run.sh
>  TEST_FILES := basic.sh tbench.sh gitsource.sh
>
> --
> 2.25.1
>
>
>
Shuah Khan Jan. 25, 2023, 5:03 p.m. UTC | #2 
On 1/21/23 09:41, Doug Smythies wrote:
> Revert the portion of a recent Makefile change that incorrectly
> deletes source files when doing "make clean".
> 
> Fixes: ba2d788aa873 ("selftests: amd-pstate: Trigger tbench benchmark and test cpus")
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Doug Smythies <dsmythies@telus.net>
> ---
> v2: fix address list.
> ---
>   tools/testing/selftests/amd-pstate/Makefile | 5 -----
>   1 file changed, 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/amd-pstate/Makefile b/tools/testing/selftests/amd-pstate/Makefile
> index 5f195ee756d6..5fd1424db37d 100644
> --- a/tools/testing/selftests/amd-pstate/Makefile
> +++ b/tools/testing/selftests/amd-pstate/Makefile
> @@ -7,11 +7,6 @@ all:
>   uname_M := $(shell uname -m 2>/dev/null || echo not)
>   ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
> 
> -ifeq (x86,$(ARCH))
> -TEST_GEN_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
> -TEST_GEN_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> -endif
> -

This looks good - Do you need these files to run this test and if so
do these need to installed when the test is run on a test system?

Now applied to linux-kselftest fixes. I will send this up for the next
rc.

thanks,
-- Shuah
Doug Smythies Jan. 26, 2023, 10:43 p.m. UTC | #3 
On 2023.01.25 09:03 Shuah Khan wrote:
> On 1/21/23 09:41, Doug Smythies wrote:
>> Revert the portion of a recent Makefile change that incorrectly
>> deletes source files when doing "make clean".
>> 
>> Fixes: ba2d788aa873 ("selftests: amd-pstate: Trigger tbench benchmark and test cpus")
>> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
>> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
>> Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>
>> Acked-by: Huang Rui <ray.huang@amd.com>
>> Signed-off-by: Doug Smythies <dsmythies@telus.net>
>> ---
>> v2: fix address list.
>> ---
>>   tools/testing/selftests/amd-pstate/Makefile | 5 -----
>>   1 file changed, 5 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/amd-pstate/Makefile b/tools/testing/selftests/amd-pstate/Makefile
>> index 5f195ee756d6..5fd1424db37d 100644
>> --- a/tools/testing/selftests/amd-pstate/Makefile
>> +++ b/tools/testing/selftests/amd-pstate/Makefile
>> @@ -7,11 +7,6 @@ all:
>>   uname_M := $(shell uname -m 2>/dev/null || echo not)
>>   ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
>> 
>> -ifeq (x86,$(ARCH))
>> -TEST_GEN_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> -TEST_GEN_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
>> -endif
>> -
>
> This looks good - Do you need these files to run this test and if so
> do these need to installed when the test is run on a test system?

I do not know.
It is a question for the AMD people.

> Now applied to linux-kselftest fixes. I will send this up for the next
> rc.

Thank you
... Doug
Guillaume Tucker Feb. 3, 2023, 3:15 p.m. UTC | #4 
On 21/01/2023 17:41, Doug Smythies wrote:
> Revert the portion of a recent Makefile change that incorrectly
> deletes source files when doing "make clean".
> 
> Fixes: ba2d788aa873 ("selftests: amd-pstate: Trigger tbench benchmark and test cpus")
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Doug Smythies <dsmythies@telus.net>
> ---
> v2: fix address list.
> ---
>  tools/testing/selftests/amd-pstate/Makefile | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/amd-pstate/Makefile b/tools/testing/selftests/amd-pstate/Makefile
> index 5f195ee756d6..5fd1424db37d 100644
> --- a/tools/testing/selftests/amd-pstate/Makefile
> +++ b/tools/testing/selftests/amd-pstate/Makefile
> @@ -7,11 +7,6 @@ all:
>  uname_M := $(shell uname -m 2>/dev/null || echo not)
>  ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
> 
> -ifeq (x86,$(ARCH))
> -TEST_GEN_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
> -TEST_GEN_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> -endif
> -


The two lines above should also be removed as they're only used
in the condition to include the Python files.

I've also hit this issue and independently came up with an
alternative solution, let me know if you want me to submit it as
a patch on top of the previous fix; see below.

The Python scripts appear to be used in gitsource.sh and
tbench.sh so I guess they are needed for these kselftests
although I haven't tried to run them or do anything with them.
This is just to fix make clean.



Author: Guillaume Tucker <guillaume.tucker@collabora.com>
Date:   Fri Feb 3 11:23:50 2023 +0100

    selftests: amd-pstate: fix TEST_FILES
    
    Replace TEST_GEN_FILES with TEST_FILES to avoid having them deleted
    when doing a clean.  Also fix the way the architecture is being
    determined as they should also be installed when ARCH=x86_64 is
    provided explicitly.  Then also append extra files to TEST_FILES and
    TEST_PROGS with += so they don't get discarded.
    
    Fixes: a2d788aa873 ("selftests: amd-pstate: Trigger tbench benchmark and test cpus")
    Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>

diff --git a/tools/testing/selftests/amd-pstate/Makefile b/tools/testing/selftests/amd-pstate/Makefile
index 5f195ee756d6..c382f579fe94 100644
--- a/tools/testing/selftests/amd-pstate/Makefile
+++ b/tools/testing/selftests/amd-pstate/Makefile
@@ -4,15 +4,15 @@
 # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
 all:
 
-uname_M := $(shell uname -m 2>/dev/null || echo not)
-ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
+ARCH ?= $(shell uname -m 2>/dev/null || echo not)
+ARCH := $(shell echo $(ARCH) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
 
 ifeq (x86,$(ARCH))
-TEST_GEN_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
-TEST_GEN_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
+TEST_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
+TEST_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
 endif
 
-TEST_PROGS := run.sh
-TEST_FILES := basic.sh tbench.sh gitsource.sh
+TEST_PROGS += run.sh
+TEST_FILES += basic.sh tbench.sh gitsource.sh
 
 include ../lib.mk
Shuah Khan Feb. 3, 2023, 4:26 p.m. UTC | #5 
On 2/3/23 08:15, Guillaume Tucker wrote:
> On 21/01/2023 17:41, Doug Smythies wrote:
>> Revert the portion of a recent Makefile change that incorrectly
>> deletes source files when doing "make clean".
>>
>> Fixes: ba2d788aa873 ("selftests: amd-pstate: Trigger tbench benchmark and test cpus")
>> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
>> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
>> Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>
>> Acked-by: Huang Rui <ray.huang@amd.com>
>> Signed-off-by: Doug Smythies <dsmythies@telus.net>
>> ---
>> v2: fix address list.
>> ---
>>   tools/testing/selftests/amd-pstate/Makefile | 5 -----
>>   1 file changed, 5 deletions(-)
>>
>> diff --git a/tools/testing/selftests/amd-pstate/Makefile b/tools/testing/selftests/amd-pstate/Makefile
>> index 5f195ee756d6..5fd1424db37d 100644
>> --- a/tools/testing/selftests/amd-pstate/Makefile
>> +++ b/tools/testing/selftests/amd-pstate/Makefile
>> @@ -7,11 +7,6 @@ all:
>>   uname_M := $(shell uname -m 2>/dev/null || echo not)
>>   ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
>>
>> -ifeq (x86,$(ARCH))
>> -TEST_GEN_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
>> -TEST_GEN_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
>> -endif
>> -
> 
> 
> The two lines above should also be removed as they're only used
> in the condition to include the Python files.
> 
> I've also hit this issue and independently came up with an
> alternative solution, let me know if you want me to submit it as
> a patch on top of the previous fix; see below.
> 
> The Python scripts appear to be used in gitsource.sh and
> tbench.sh so I guess they are needed for these kselftests
> although I haven't tried to run them or do anything with them.
> This is just to fix make clean.
> 
> 

Please send me the patch.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/amd-pstate/Makefile b/tools/testing/selftests/amd-pstate/Makefile
index 5f195ee756d6..5fd1424db37d 100644
--- a/tools/testing/selftests/amd-pstate/Makefile
+++ b/tools/testing/selftests/amd-pstate/Makefile
@@ -7,11 +7,6 @@  all:
 uname_M := $(shell uname -m 2>/dev/null || echo not)
 ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)

-ifeq (x86,$(ARCH))
-TEST_GEN_FILES += ../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py
-TEST_GEN_FILES += ../../../power/x86/intel_pstate_tracer/intel_pstate_tracer.py
-endif
-
 TEST_PROGS := run.sh
 TEST_FILES := basic.sh tbench.sh gitsource.sh