| Message ID | 20240229093711.581230-2-maddy@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] selftest/powerpc: Re-order *FLAGS to follow lib.mk | expand |
> On 29-Feb-2024, at 3:07 PM, Madhavan Srinivasan <maddy@linux.ibm.com> wrote: > > When running `make -C powerpc/pmu run_tests` from top level selftests > directory, currently this error is being reported > > make: Entering directory '/home/maddy/linux/tools/testing/selftests/powerpc/pmu' > Makefile:40: warning: overriding recipe for target 'emit_tests' > ../../lib.mk:111: warning: ignoring old recipe for target 'emit_tests' > gcc -m64 count_instructions.c ../harness.c event.c lib.c ../utils.c loop.S -o /home/maddy/selftest_output//count_instructions > In file included from count_instructions.c:13: > event.h:12:10: fatal error: utils.h: No such file or directory > 12 | #include "utils.h" > | ^~~~~~~~~ > compilation terminated. > > This is due to missing of include path in CFLAGS. That is, CFLAGS and > GIT_VERSION macros are defined in the powerpc/ folder Makefile which > in this case not involved. > > To address the failure incase of executing specific sub-folder test directly, > a new rule file has been addded by the patch called "flags.mk" under > selftest/powerpc/ folder and is linked to all the Makefile of powerpc/pmu > sub-folders. > > Reported-by: Sachin Sant <sachinp@linux.ibm.com> > Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> > --- Fixes the reported problem for me. Tested-by: Sachin Sant <sachinp@linux.ibm.com> While at it, FWIW I have also tested the remaining 2 patches and no problems were seen. For the other 2 patches in the series Tested-by: Sachin Sant <sachinp@linux.ibm.com> — Sachin
Madhavan Srinivasan <maddy@linux.ibm.com> writes: > When running `make -C powerpc/pmu run_tests` from top level selftests > directory, currently this error is being reported > > make: Entering directory '/home/maddy/linux/tools/testing/selftests/powerpc/pmu' > Makefile:40: warning: overriding recipe for target 'emit_tests' > ../../lib.mk:111: warning: ignoring old recipe for target 'emit_tests' > gcc -m64 count_instructions.c ../harness.c event.c lib.c ../utils.c loop.S -o /home/maddy/selftest_output//count_instructions > In file included from count_instructions.c:13: > event.h:12:10: fatal error: utils.h: No such file or directory > 12 | #include "utils.h" > | ^~~~~~~~~ > compilation terminated. > > This is due to missing of include path in CFLAGS. That is, CFLAGS and > GIT_VERSION macros are defined in the powerpc/ folder Makefile which > in this case not involved. > > To address the failure incase of executing specific sub-folder test directly, > a new rule file has been addded by the patch called "flags.mk" under > selftest/powerpc/ folder and is linked to all the Makefile of powerpc/pmu > sub-folders. This patch made my selftest build go from ~10s to ~50s ! I tracked it down to "git describe" being run hundreds of times. > diff --git a/tools/testing/selftests/powerpc/flags.mk b/tools/testing/selftests/powerpc/flags.mk > new file mode 100644 > index 000000000000..28374f470126 > --- /dev/null > +++ b/tools/testing/selftests/powerpc/flags.mk > @@ -0,0 +1,12 @@ > +#This checks for any ENV variables and add those. > + > +#ifeq ($(GIT_VERSION),) This isn't right, # is a comment in make syntax, so this line is just a comment. It needs to be "ifeq". > +GIT_VERSION = $(shell git describe --always --long --dirty || echo "unknown") Using '=' here means Make re-runs the command every time the variable is used. Previously that was OK because the variable was set once and then exported. But now that it's a Make variable in each file it leads to "git describe" being run a few hundred times. I've squashed in those fixes, no need to send a v2. cheers
On 4/29/24 7:39 PM, Michael Ellerman wrote: > Madhavan Srinivasan <maddy@linux.ibm.com> writes: >> When running `make -C powerpc/pmu run_tests` from top level selftests >> directory, currently this error is being reported >> >> make: Entering directory '/home/maddy/linux/tools/testing/selftests/powerpc/pmu' >> Makefile:40: warning: overriding recipe for target 'emit_tests' >> ../../lib.mk:111: warning: ignoring old recipe for target 'emit_tests' >> gcc -m64 count_instructions.c ../harness.c event.c lib.c ../utils.c loop.S -o /home/maddy/selftest_output//count_instructions >> In file included from count_instructions.c:13: >> event.h:12:10: fatal error: utils.h: No such file or directory >> 12 | #include "utils.h" >> | ^~~~~~~~~ >> compilation terminated. >> >> This is due to missing of include path in CFLAGS. That is, CFLAGS and >> GIT_VERSION macros are defined in the powerpc/ folder Makefile which >> in this case not involved. >> >> To address the failure incase of executing specific sub-folder test directly, >> a new rule file has been addded by the patch called "flags.mk" under >> selftest/powerpc/ folder and is linked to all the Makefile of powerpc/pmu >> sub-folders. > This patch made my selftest build go from ~10s to ~50s ! > > I tracked it down to "git describe" being run hundreds of times. > >> diff --git a/tools/testing/selftests/powerpc/flags.mk b/tools/testing/selftests/powerpc/flags.mk >> new file mode 100644 >> index 000000000000..28374f470126 >> --- /dev/null >> +++ b/tools/testing/selftests/powerpc/flags.mk >> @@ -0,0 +1,12 @@ >> +#This checks for any ENV variables and add those. >> + >> +#ifeq ($(GIT_VERSION),) > > This isn't right, # is a comment in make syntax, so this line is just a > comment. It needs to be "ifeq". oops, my bad :( But nice catch. Thanks Maddy > >> +GIT_VERSION = $(shell git describe --always --long --dirty || echo "unknown") > > Using '=' here means Make re-runs the command every time the variable is > used. Previously that was OK because the variable was set once and then > exported. But now that it's a Make variable in each file it leads to > "git describe" being run a few hundred times. > > I've squashed in those fixes, no need to send a v2. > > cheers
diff --git a/tools/testing/selftests/powerpc/flags.mk b/tools/testing/selftests/powerpc/flags.mk new file mode 100644 index 000000000000..28374f470126 --- /dev/null +++ b/tools/testing/selftests/powerpc/flags.mk @@ -0,0 +1,12 @@ +#This checks for any ENV variables and add those. + +#ifeq ($(GIT_VERSION),) +GIT_VERSION = $(shell git describe --always --long --dirty || echo "unknown") +export GIT_VERSION +#endif + +#ifeq ($(CFLAGS),) +CFLAGS := -std=gnu99 -O2 -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(selfdir)/powerpc/include $(CFLAGS) +export CFLAGS +#endif + diff --git a/tools/testing/selftests/powerpc/pmu/Makefile b/tools/testing/selftests/powerpc/pmu/Makefile index a284fa874a9f..1fcacae1b188 100644 --- a/tools/testing/selftests/powerpc/pmu/Makefile +++ b/tools/testing/selftests/powerpc/pmu/Makefile @@ -7,6 +7,7 @@ EXTRA_SOURCES := ../harness.c event.c lib.c ../utils.c top_srcdir = ../../../../.. include ../../lib.mk +include ../flags.mk all: $(TEST_GEN_PROGS) ebb sampling_tests event_code_tests diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile b/tools/testing/selftests/powerpc/pmu/ebb/Makefile index b3946ce17e0c..1b39af7c10db 100644 --- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile +++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile @@ -18,6 +18,7 @@ TEST_GEN_PROGS := reg_access_test event_attributes_test cycles_test \ top_srcdir = ../../../../../.. include ../../../lib.mk +include ../../flags.mk # The EBB handler is 64-bit code and everything links against it CFLAGS += -m64 diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/Makefile b/tools/testing/selftests/powerpc/pmu/event_code_tests/Makefile index 509d4b235b9e..fdb080b3fa65 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/Makefile +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/Makefile @@ -9,6 +9,7 @@ TEST_GEN_PROGS := group_constraint_pmc56_test group_pmc56_exclude_constraints_te top_srcdir = ../../../../../.. include ../../../lib.mk +include ../../flags.mk CFLAGS += -m64 diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile index d45892151e05..9f79bec5fce7 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/Makefile @@ -9,6 +9,7 @@ TEST_GEN_PROGS := mmcr0_exceptionbits_test mmcr0_cc56run_test mmcr0_pmccext_test top_srcdir = ../../../../../.. include ../../../lib.mk +include ../../flags.mk CFLAGS += -m64
When running `make -C powerpc/pmu run_tests` from top level selftests directory, currently this error is being reported make: Entering directory '/home/maddy/linux/tools/testing/selftests/powerpc/pmu' Makefile:40: warning: overriding recipe for target 'emit_tests' ../../lib.mk:111: warning: ignoring old recipe for target 'emit_tests' gcc -m64 count_instructions.c ../harness.c event.c lib.c ../utils.c loop.S -o /home/maddy/selftest_output//count_instructions In file included from count_instructions.c:13: event.h:12:10: fatal error: utils.h: No such file or directory 12 | #include "utils.h" | ^~~~~~~~~ compilation terminated. This is due to missing of include path in CFLAGS. That is, CFLAGS and GIT_VERSION macros are defined in the powerpc/ folder Makefile which in this case not involved. To address the failure incase of executing specific sub-folder test directly, a new rule file has been addded by the patch called "flags.mk" under selftest/powerpc/ folder and is linked to all the Makefile of powerpc/pmu sub-folders. Reported-by: Sachin Sant <sachinp@linux.ibm.com> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> --- Changelog RFC: - Rename the rule file as flags.mk - Added additional patches to support other sub-folders under powerpc/ to be buildable on it own. link to RFC: https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20240225163926.264286-1-maddy@linux.ibm.com/ tools/testing/selftests/powerpc/flags.mk | 12 ++++++++++++ tools/testing/selftests/powerpc/pmu/Makefile | 1 + tools/testing/selftests/powerpc/pmu/ebb/Makefile | 1 + .../selftests/powerpc/pmu/event_code_tests/Makefile | 1 + .../selftests/powerpc/pmu/sampling_tests/Makefile | 1 + 5 files changed, 16 insertions(+) create mode 100644 tools/testing/selftests/powerpc/flags.mk