* [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring
@ 2019-03-25 16:04 Joe Lawrence
2019-03-25 16:21 ` Steven Rostedt
2019-03-26 5:29 ` Masahiro Yamada
0 siblings, 2 replies; 7+ messages in thread
From: Joe Lawrence @ 2019-03-25 16:04 UTC (permalink / raw)
To: linux-kbuild, linuxppc-dev, linux-kernel
Cc: Joe Lawrence, Masahiro Yamada, Michal Marek, Nicholas Piggin,
Steven Rostedt
CC_FLAGS_FTRACE may contain trailing whitespace that interferes with
findstring.
For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on
GCC 4.9 and newer") introduced a change such that on my ppc64le box,
CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.)
When cmd_record_mcount is now invoked, findstring fails as the ftrace
flags were found at very end of _c_flags, without the trailing space.
_c_flags=" ... -pg -mprofile-kernel"
CC_FLAGS_FTRACE="-pg -mprofile-kernel "
^
findstring is looking for this extra space
Remove the redundant whitespaces from CC_FLAGS_FTRACE in
cmd_record_mcount to avoid this problem.
Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer").
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
Standard disclaimer: I'm not a kbuild expert, but this works around the
problem I reported where ftrace and livepatch self-tests were failing as
specified object files were not run through the recordmcount.pl script:
ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html
scripts/Makefile.build | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 2554a15ecf2b..74d402b5aa3c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
"$(if $(part-of-module),1,0)" "$(@)";
recordmcount_source := $(srctree)/scripts/recordmcount.pl
endif # BUILD_C_RECORDMCOUNT
-cmd_record_mcount = \
- if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \
- "$(CC_FLAGS_FTRACE)" ]; then \
- $(sub_cmd_record_mcount) \
+cmd_record_mcount = \
+ if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \
+ "$(strip $(CC_FLAGS_FTRACE))" ]; then \
+ $(sub_cmd_record_mcount) \
fi
endif # CC_USING_RECORD_MCOUNT
endif # CONFIG_FTRACE_MCOUNT_RECORD
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring 2019-03-25 16:04 [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring Joe Lawrence @ 2019-03-25 16:21 ` Steven Rostedt 2019-03-26 5:29 ` Masahiro Yamada 1 sibling, 0 replies; 7+ messages in thread From: Steven Rostedt @ 2019-03-25 16:21 UTC (permalink / raw) To: Joe Lawrence Cc: linux-kbuild, linuxppc-dev, linux-kernel, Masahiro Yamada, Michal Marek, Nicholas Piggin On Mon, 25 Mar 2019 12:04:38 -0400 Joe Lawrence <joe.lawrence@redhat.com> wrote: > CC_FLAGS_FTRACE may contain trailing whitespace that interferes with > findstring. > > For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on > GCC 4.9 and newer") introduced a change such that on my ppc64le box, > CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) > When cmd_record_mcount is now invoked, findstring fails as the ftrace > flags were found at very end of _c_flags, without the trailing space. > > _c_flags=" ... -pg -mprofile-kernel" > CC_FLAGS_FTRACE="-pg -mprofile-kernel " > ^ > findstring is looking for this extra space Wow, what a bug. > > Remove the redundant whitespaces from CC_FLAGS_FTRACE in > cmd_record_mcount to avoid this problem. > > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org> -- Steve > --- > > Standard disclaimer: I'm not a kbuild expert, but this works around the > problem I reported where ftrace and livepatch self-tests were failing as > specified object files were not run through the recordmcount.pl script: > > ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken? > https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html > > scripts/Makefile.build | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 2554a15ecf2b..74d402b5aa3c 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > "$(if $(part-of-module),1,0)" "$(@)"; > recordmcount_source := $(srctree)/scripts/recordmcount.pl > endif # BUILD_C_RECORDMCOUNT > -cmd_record_mcount = \ > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > - "$(CC_FLAGS_FTRACE)" ]; then \ > - $(sub_cmd_record_mcount) \ > +cmd_record_mcount = \ > + if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \ > + "$(strip $(CC_FLAGS_FTRACE))" ]; then \ > + $(sub_cmd_record_mcount) \ > fi > endif # CC_USING_RECORD_MCOUNT > endif # CONFIG_FTRACE_MCOUNT_RECORD ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring 2019-03-25 16:04 [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring Joe Lawrence 2019-03-25 16:21 ` Steven Rostedt @ 2019-03-26 5:29 ` Masahiro Yamada 2019-03-26 17:33 ` [PATCH v2] " Joe Lawrence 1 sibling, 1 reply; 7+ messages in thread From: Masahiro Yamada @ 2019-03-26 5:29 UTC (permalink / raw) To: Joe Lawrence Cc: Linux Kbuild mailing list, linuxppc-dev, Linux Kernel Mailing List, Michal Marek, Nicholas Piggin, Steven Rostedt On Tue, Mar 26, 2019 at 1:05 AM Joe Lawrence <joe.lawrence@redhat.com> wrote: > > CC_FLAGS_FTRACE may contain trailing whitespace that interferes with > findstring. > > For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on > GCC 4.9 and newer") introduced a change such that on my ppc64le box, > CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) > When cmd_record_mcount is now invoked, findstring fails as the ftrace > flags were found at very end of _c_flags, without the trailing space. > > _c_flags=" ... -pg -mprofile-kernel" > CC_FLAGS_FTRACE="-pg -mprofile-kernel " > ^ > findstring is looking for this extra space > > Remove the redundant whitespaces from CC_FLAGS_FTRACE in > cmd_record_mcount to avoid this problem. > > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> > --- > > Standard disclaimer: I'm not a kbuild expert, but this works around the > problem I reported where ftrace and livepatch self-tests were failing as > specified object files were not run through the recordmcount.pl script: > > ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken? > https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html > > scripts/Makefile.build | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 2554a15ecf2b..74d402b5aa3c 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > "$(if $(part-of-module),1,0)" "$(@)"; > recordmcount_source := $(srctree)/scripts/recordmcount.pl > endif # BUILD_C_RECORDMCOUNT > -cmd_record_mcount = \ > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > - "$(CC_FLAGS_FTRACE)" ]; then \ > - $(sub_cmd_record_mcount) \ > +cmd_record_mcount = \ > + if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \ > + "$(strip $(CC_FLAGS_FTRACE))" ]; then \ > + $(sub_cmd_record_mcount) \ > fi > endif # CC_USING_RECORD_MCOUNT > endif # CONFIG_FTRACE_MCOUNT_RECORD > -- > 2.20.1 > I do not see a point in using the shell command here in the first place. Instead of adding crappy workarounds, I guess the following simple code should work: index 2554a15..5f13021 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -199,11 +199,8 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ "$(if $(part-of-module),1,0)" "$(@)"; recordmcount_source := $(srctree)/scripts/recordmcount.pl endif # BUILD_C_RECORDMCOUNT -cmd_record_mcount = \ - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ - "$(CC_FLAGS_FTRACE)" ]; then \ - $(sub_cmd_record_mcount) \ - fi +cmd_record_mcount = $(if $(findstring $(CC_FLAGS_FTRACE),$(_c_flags)),\ + $(sub_cmd_record_mcount)) endif # CC_USING_RECORD_MCOUNT endif # CONFIG_FTRACE_MCOUNT_RECORD -- Best Regards Masahiro Yamada ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring 2019-03-26 5:29 ` Masahiro Yamada @ 2019-03-26 17:33 ` Joe Lawrence 2019-03-27 6:17 ` Nicholas Piggin 2019-03-28 12:57 ` Masahiro Yamada 0 siblings, 2 replies; 7+ messages in thread From: Joe Lawrence @ 2019-03-26 17:33 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, linuxppc-dev, Linux Kernel Mailing List, Michal Marek, Nicholas Piggin, Steven Rostedt On Tue, Mar 26, 2019 at 02:29:47PM +0900, Masahiro Yamada wrote: > On Tue, Mar 26, 2019 at 1:05 AM Joe Lawrence <joe.lawrence@redhat.com> wrote: > > > > CC_FLAGS_FTRACE may contain trailing whitespace that interferes with > > findstring. > > > > For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on > > GCC 4.9 and newer") introduced a change such that on my ppc64le box, > > CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) > > When cmd_record_mcount is now invoked, findstring fails as the ftrace > > flags were found at very end of _c_flags, without the trailing space. > > > > _c_flags=" ... -pg -mprofile-kernel" > > CC_FLAGS_FTRACE="-pg -mprofile-kernel " > > ^ > > findstring is looking for this extra space > > > > Remove the redundant whitespaces from CC_FLAGS_FTRACE in > > cmd_record_mcount to avoid this problem. > > > > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). > > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> > > --- > > > > Standard disclaimer: I'm not a kbuild expert, but this works around the > > problem I reported where ftrace and livepatch self-tests were failing as > > specified object files were not run through the recordmcount.pl script: > > > > ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken? > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html > > > > scripts/Makefile.build | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > > index 2554a15ecf2b..74d402b5aa3c 100644 > > --- a/scripts/Makefile.build > > +++ b/scripts/Makefile.build > > @@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > > "$(if $(part-of-module),1,0)" "$(@)"; > > recordmcount_source := $(srctree)/scripts/recordmcount.pl > > endif # BUILD_C_RECORDMCOUNT > > -cmd_record_mcount = \ > > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > > - "$(CC_FLAGS_FTRACE)" ]; then \ > > - $(sub_cmd_record_mcount) \ > > +cmd_record_mcount = \ > > + if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \ > > + "$(strip $(CC_FLAGS_FTRACE))" ]; then \ > > + $(sub_cmd_record_mcount) \ > > fi > > endif # CC_USING_RECORD_MCOUNT > > endif # CONFIG_FTRACE_MCOUNT_RECORD > > -- > > 2.20.1 > > > > > > I do not see a point in using the shell command here > in the first place. > > Instead of adding crappy workarounds, > I guess the following simple code should work: > > > index 2554a15..5f13021 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -199,11 +199,8 @@ sub_cmd_record_mcount = perl > $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > "$(if $(part-of-module),1,0)" "$(@)"; > recordmcount_source := $(srctree)/scripts/recordmcount.pl > endif # BUILD_C_RECORDMCOUNT > -cmd_record_mcount = \ > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > - "$(CC_FLAGS_FTRACE)" ]; then \ > - $(sub_cmd_record_mcount) \ > - fi > +cmd_record_mcount = $(if $(findstring $(CC_FLAGS_FTRACE),$(_c_flags)),\ > + $(sub_cmd_record_mcount)) > endif # CC_USING_RECORD_MCOUNT > endif # CONFIG_FTRACE_MCOUNT_RECORD > Hi Masahiro, Agreed on the shell command ugliness, however I still think we need to strip the search pattern here. With your suggestion: % rm -f kernel/trace/trace_selftest_dynamic.o % make kernel/trace/trace_selftest_dynamic.o CALL scripts/checksyscalls.sh CALL scripts/atomic/check-atomics.sh CC kernel/trace/trace_selftest_dynamic.o % eu-readelf --sections kernel/trace/trace_selftest_dynamic.o | grep mcount (nothing) Adding it back as, as below, restores those sections and the self tests work again. Regards, -- Joe -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- From 6a85e8ecf4179b3e80601a327ec43d8d49f0e3cd Mon Sep 17 00:00:00 2001 From: Joe Lawrence <joe.lawrence@redhat.com> Date: Tue, 26 Mar 2019 10:50:28 -0400 Subject: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring CC_FLAGS_FTRACE may contain trailing whitespace that interferes with findstring. For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer") introduced a change such that on my ppc64le box, CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) When cmd_record_mcount is now invoked, findstring fails as the ftrace flags were found at very end of _c_flags, without the trailing space. _c_flags=" ... -pg -mprofile-kernel" CC_FLAGS_FTRACE="-pg -mprofile-kernel " ^ findstring is looking for this extra space Remove the redundant whitespaces from CC_FLAGS_FTRACE in cmd_record_mcount to avoid this problem. Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> (refactoring) Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> --- scripts/Makefile.build | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 2554a15ecf2b..76ca30cc4791 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -199,11 +199,8 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ "$(if $(part-of-module),1,0)" "$(@)"; recordmcount_source := $(srctree)/scripts/recordmcount.pl endif # BUILD_C_RECORDMCOUNT -cmd_record_mcount = \ - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ - "$(CC_FLAGS_FTRACE)" ]; then \ - $(sub_cmd_record_mcount) \ - fi +cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)), \ + $(sub_cmd_record_mcount)) endif # CC_USING_RECORD_MCOUNT endif # CONFIG_FTRACE_MCOUNT_RECORD -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring 2019-03-26 17:33 ` [PATCH v2] " Joe Lawrence @ 2019-03-27 6:17 ` Nicholas Piggin 2019-03-28 12:57 ` Masahiro Yamada 1 sibling, 0 replies; 7+ messages in thread From: Nicholas Piggin @ 2019-03-27 6:17 UTC (permalink / raw) To: Joe Lawrence, Masahiro Yamada Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, linuxppc-dev, Michal Marek, Steven Rostedt Joe Lawrence's on March 27, 2019 3:33 am: > On Tue, Mar 26, 2019 at 02:29:47PM +0900, Masahiro Yamada wrote: >> On Tue, Mar 26, 2019 at 1:05 AM Joe Lawrence <joe.lawrence@redhat.com> wrote: >> > >> > CC_FLAGS_FTRACE may contain trailing whitespace that interferes with >> > findstring. >> > >> > For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on >> > GCC 4.9 and newer") introduced a change such that on my ppc64le box, >> > CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) >> > When cmd_record_mcount is now invoked, findstring fails as the ftrace >> > flags were found at very end of _c_flags, without the trailing space. >> > >> > _c_flags=" ... -pg -mprofile-kernel" >> > CC_FLAGS_FTRACE="-pg -mprofile-kernel " >> > ^ >> > findstring is looking for this extra space >> > >> > Remove the redundant whitespaces from CC_FLAGS_FTRACE in >> > cmd_record_mcount to avoid this problem. >> > >> > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). >> > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> >> > --- >> > >> > Standard disclaimer: I'm not a kbuild expert, but this works around the >> > problem I reported where ftrace and livepatch self-tests were failing as >> > specified object files were not run through the recordmcount.pl script: >> > >> > ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken? >> > https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html >> > >> > scripts/Makefile.build | 8 ++++---- >> > 1 file changed, 4 insertions(+), 4 deletions(-) >> > >> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build >> > index 2554a15ecf2b..74d402b5aa3c 100644 >> > --- a/scripts/Makefile.build >> > +++ b/scripts/Makefile.build >> > @@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ >> > "$(if $(part-of-module),1,0)" "$(@)"; >> > recordmcount_source := $(srctree)/scripts/recordmcount.pl >> > endif # BUILD_C_RECORDMCOUNT >> > -cmd_record_mcount = \ >> > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ >> > - "$(CC_FLAGS_FTRACE)" ]; then \ >> > - $(sub_cmd_record_mcount) \ >> > +cmd_record_mcount = \ >> > + if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \ >> > + "$(strip $(CC_FLAGS_FTRACE))" ]; then \ >> > + $(sub_cmd_record_mcount) \ >> > fi >> > endif # CC_USING_RECORD_MCOUNT >> > endif # CONFIG_FTRACE_MCOUNT_RECORD >> > -- >> > 2.20.1 >> > >> >> >> >> I do not see a point in using the shell command here >> in the first place. >> >> Instead of adding crappy workarounds, >> I guess the following simple code should work: >> >> >> index 2554a15..5f13021 100644 >> --- a/scripts/Makefile.build >> +++ b/scripts/Makefile.build >> @@ -199,11 +199,8 @@ sub_cmd_record_mcount = perl >> $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ >> "$(if $(part-of-module),1,0)" "$(@)"; >> recordmcount_source := $(srctree)/scripts/recordmcount.pl >> endif # BUILD_C_RECORDMCOUNT >> -cmd_record_mcount = \ >> - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ >> - "$(CC_FLAGS_FTRACE)" ]; then \ >> - $(sub_cmd_record_mcount) \ >> - fi >> +cmd_record_mcount = $(if $(findstring $(CC_FLAGS_FTRACE),$(_c_flags)),\ >> + $(sub_cmd_record_mcount)) >> endif # CC_USING_RECORD_MCOUNT >> endif # CONFIG_FTRACE_MCOUNT_RECORD >> > > Hi Masahiro, > > Agreed on the shell command ugliness, however I still think we need to > strip the search pattern here. With your suggestion: > > % rm -f kernel/trace/trace_selftest_dynamic.o > % make kernel/trace/trace_selftest_dynamic.o > CALL scripts/checksyscalls.sh > CALL scripts/atomic/check-atomics.sh > CC kernel/trace/trace_selftest_dynamic.o > > % eu-readelf --sections kernel/trace/trace_selftest_dynamic.o | grep mcount > (nothing) > > Adding it back as, as below, restores those sections and the self tests > work again. Thanks for finding and fixing this, nice work! Thanks, Nick > > Regards, > > -- Joe > > -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- > > From 6a85e8ecf4179b3e80601a327ec43d8d49f0e3cd Mon Sep 17 00:00:00 2001 > From: Joe Lawrence <joe.lawrence@redhat.com> > Date: Tue, 26 Mar 2019 10:50:28 -0400 > Subject: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring > > CC_FLAGS_FTRACE may contain trailing whitespace that interferes with > findstring. > > For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on > GCC 4.9 and newer") introduced a change such that on my ppc64le box, > CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) > When cmd_record_mcount is now invoked, findstring fails as the ftrace > flags were found at very end of _c_flags, without the trailing space. > > _c_flags=" ... -pg -mprofile-kernel" > CC_FLAGS_FTRACE="-pg -mprofile-kernel " > ^ > findstring is looking for this extra space > > Remove the redundant whitespaces from CC_FLAGS_FTRACE in > cmd_record_mcount to avoid this problem. > > Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> (refactoring) > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> > --- > scripts/Makefile.build | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 2554a15ecf2b..76ca30cc4791 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -199,11 +199,8 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > "$(if $(part-of-module),1,0)" "$(@)"; > recordmcount_source := $(srctree)/scripts/recordmcount.pl > endif # BUILD_C_RECORDMCOUNT > -cmd_record_mcount = \ > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > - "$(CC_FLAGS_FTRACE)" ]; then \ > - $(sub_cmd_record_mcount) \ > - fi > +cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)), \ > + $(sub_cmd_record_mcount)) > endif # CC_USING_RECORD_MCOUNT > endif # CONFIG_FTRACE_MCOUNT_RECORD > > -- > 2.20.1 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring 2019-03-26 17:33 ` [PATCH v2] " Joe Lawrence 2019-03-27 6:17 ` Nicholas Piggin @ 2019-03-28 12:57 ` Masahiro Yamada 2019-03-28 13:36 ` Joe Lawrence 1 sibling, 1 reply; 7+ messages in thread From: Masahiro Yamada @ 2019-03-28 12:57 UTC (permalink / raw) To: Joe Lawrence Cc: Linux Kbuild mailing list, linuxppc-dev, Linux Kernel Mailing List, Michal Marek, Nicholas Piggin, Steven Rostedt Hi Joe, On Wed, Mar 27, 2019 at 2:33 AM Joe Lawrence <joe.lawrence@redhat.com> wrote: > > On Tue, Mar 26, 2019 at 02:29:47PM +0900, Masahiro Yamada wrote: > > On Tue, Mar 26, 2019 at 1:05 AM Joe Lawrence <joe.lawrence@redhat.com> wrote: > > > > > > CC_FLAGS_FTRACE may contain trailing whitespace that interferes with > > > findstring. > > > > > > For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on > > > GCC 4.9 and newer") introduced a change such that on my ppc64le box, > > > CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) > > > When cmd_record_mcount is now invoked, findstring fails as the ftrace > > > flags were found at very end of _c_flags, without the trailing space. > > > > > > _c_flags=" ... -pg -mprofile-kernel" > > > CC_FLAGS_FTRACE="-pg -mprofile-kernel " > > > ^ > > > findstring is looking for this extra space > > > > > > Remove the redundant whitespaces from CC_FLAGS_FTRACE in > > > cmd_record_mcount to avoid this problem. > > > > > > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). > > > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> > > > --- > > > > > > Standard disclaimer: I'm not a kbuild expert, but this works around the > > > problem I reported where ftrace and livepatch self-tests were failing as > > > specified object files were not run through the recordmcount.pl script: > > > > > > ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken? > > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html > > > > > > scripts/Makefile.build | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > > > index 2554a15ecf2b..74d402b5aa3c 100644 > > > --- a/scripts/Makefile.build > > > +++ b/scripts/Makefile.build > > > @@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > > > "$(if $(part-of-module),1,0)" "$(@)"; > > > recordmcount_source := $(srctree)/scripts/recordmcount.pl > > > endif # BUILD_C_RECORDMCOUNT > > > -cmd_record_mcount = \ > > > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > > > - "$(CC_FLAGS_FTRACE)" ]; then \ > > > - $(sub_cmd_record_mcount) \ > > > +cmd_record_mcount = \ > > > + if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \ > > > + "$(strip $(CC_FLAGS_FTRACE))" ]; then \ > > > + $(sub_cmd_record_mcount) \ > > > fi > > > endif # CC_USING_RECORD_MCOUNT > > > endif # CONFIG_FTRACE_MCOUNT_RECORD > > > -- > > > 2.20.1 > > > > > > > > > > > I do not see a point in using the shell command here > > in the first place. > > > > Instead of adding crappy workarounds, > > I guess the following simple code should work: > > > > > > index 2554a15..5f13021 100644 > > --- a/scripts/Makefile.build > > +++ b/scripts/Makefile.build > > @@ -199,11 +199,8 @@ sub_cmd_record_mcount = perl > > $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > > "$(if $(part-of-module),1,0)" "$(@)"; > > recordmcount_source := $(srctree)/scripts/recordmcount.pl > > endif # BUILD_C_RECORDMCOUNT > > -cmd_record_mcount = \ > > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > > - "$(CC_FLAGS_FTRACE)" ]; then \ > > - $(sub_cmd_record_mcount) \ > > - fi > > +cmd_record_mcount = $(if $(findstring $(CC_FLAGS_FTRACE),$(_c_flags)),\ > > + $(sub_cmd_record_mcount)) > > endif # CC_USING_RECORD_MCOUNT > > endif # CONFIG_FTRACE_MCOUNT_RECORD > > > > Hi Masahiro, > > Agreed on the shell command ugliness, however I still think we need to > strip the search pattern here. With your suggestion: > > % rm -f kernel/trace/trace_selftest_dynamic.o > % make kernel/trace/trace_selftest_dynamic.o > CALL scripts/checksyscalls.sh > CALL scripts/atomic/check-atomics.sh > CC kernel/trace/trace_selftest_dynamic.o > > % eu-readelf --sections kernel/trace/trace_selftest_dynamic.o | grep mcount > (nothing) > > Adding it back as, as below, restores those sections and the self tests > work again. OK, confirmed. First, I was wondering why I could not reproduce this issue. Then, I found the reason was I was using the latest GNU Make compiled from the git source tree. I found the following commit: commit b90fabc8d6f34fb37d428dc0fb1b8b1951a9fbed Author: Paul Smith <psmith@gnu.org> Date: Sat May 27 20:07:30 2017 -0400 * NEWS: Do not insert a space during '+=' if the value is empty. * doc/make.texi (Appending): Document this behavior. * variable.c (do_variable_definition): Only add a space if the variable value is not empty. * tests/scripts/variables/flavors: Test this behavior. diff --git a/NEWS b/NEWS index e60644a..6e2c5c6 100644 --- a/NEWS +++ b/NEWS @@ -29,6 +29,12 @@ http://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=108&set This was claimed to be fixed in 3.81, but wasn't, for some reason. To detect this change search for 'nocomment' in the .FEATURES variable. +* WARNING: Backward-incompatibility! + Previously appending using '+=' to an empty variable would result in a value + starting with a space. Now the initial space is only added if the variable + already contains some value. Similarly, appending an empty string does not + add a trailing space. + * The previous limit of 63 jobs under -jN on MS-Windows is now increased to 4095. That limit includes the subprocess started by the $(shell) function. Applied to linux-kbuild/fixes with additional comments. [Additional info by masahiro.yamada: This issue only happens in the released versions of GNU Make. CC_FLAGS_FTRACE will not contain the trailing space if you use the latest GNU Make, which contains commit b90fabc8d6f3 ("* NEWS: Do not insert a space during '+=' if the value is empty.") ] > Regards, > > -- Joe > > -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- > > From 6a85e8ecf4179b3e80601a327ec43d8d49f0e3cd Mon Sep 17 00:00:00 2001 > From: Joe Lawrence <joe.lawrence@redhat.com> > Date: Tue, 26 Mar 2019 10:50:28 -0400 > Subject: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring > > CC_FLAGS_FTRACE may contain trailing whitespace that interferes with > findstring. > > For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on > GCC 4.9 and newer") introduced a change such that on my ppc64le box, > CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.) > When cmd_record_mcount is now invoked, findstring fails as the ftrace > flags were found at very end of _c_flags, without the trailing space. > > _c_flags=" ... -pg -mprofile-kernel" > CC_FLAGS_FTRACE="-pg -mprofile-kernel " > ^ > findstring is looking for this extra space > > Remove the redundant whitespaces from CC_FLAGS_FTRACE in > cmd_record_mcount to avoid this problem. > > Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> (refactoring) > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> > --- > scripts/Makefile.build | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 2554a15ecf2b..76ca30cc4791 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -199,11 +199,8 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \ > "$(if $(part-of-module),1,0)" "$(@)"; > recordmcount_source := $(srctree)/scripts/recordmcount.pl > endif # BUILD_C_RECORDMCOUNT > -cmd_record_mcount = \ > - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \ > - "$(CC_FLAGS_FTRACE)" ]; then \ > - $(sub_cmd_record_mcount) \ > - fi > +cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)), \ > + $(sub_cmd_record_mcount)) > endif # CC_USING_RECORD_MCOUNT > endif # CONFIG_FTRACE_MCOUNT_RECORD > > -- > 2.20.1 > -- Best Regards Masahiro Yamada ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring 2019-03-28 12:57 ` Masahiro Yamada @ 2019-03-28 13:36 ` Joe Lawrence 0 siblings, 0 replies; 7+ messages in thread From: Joe Lawrence @ 2019-03-28 13:36 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, linuxppc-dev, Linux Kernel Mailing List, Michal Marek, Nicholas Piggin, Steven Rostedt On 3/28/19 8:57 AM, Masahiro Yamada wrote: > Hi Joe, > > OK, confirmed. > > [ ... snip .... ] > > First, I was wondering why I could not reproduce this issue. > Then, I found the reason was I was using the latest GNU Make > compiled from the git source tree. > > I found the following commit: > > commit b90fabc8d6f34fb37d428dc0fb1b8b1951a9fbed > Author: Paul Smith <psmith@gnu.org> > Date: Sat May 27 20:07:30 2017 -0400 > > * NEWS: Do not insert a space during '+=' if the value is empty. > > * doc/make.texi (Appending): Document this behavior. > * variable.c (do_variable_definition): Only add a space if the variable > value is not empty. > * tests/scripts/variables/flavors: Test this behavior. > > diff --git a/NEWS b/NEWS > index e60644a..6e2c5c6 100644 > --- a/NEWS > +++ b/NEWS > @@ -29,6 +29,12 @@ > http://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=108&set > This was claimed to be fixed in 3.81, but wasn't, for some reason. > To detect this change search for 'nocomment' in the .FEATURES variable. > > +* WARNING: Backward-incompatibility! > + Previously appending using '+=' to an empty variable would result in a value > + starting with a space. Now the initial space is only added if the variable > + already contains some value. Similarly, appending an empty string does not > + add a trailing space. > + > * The previous limit of 63 jobs under -jN on MS-Windows is now > increased to 4095. That limit includes the subprocess started by > the $(shell) function. > > Applied to linux-kbuild/fixes with additional comments. > > [Additional info by masahiro.yamada: > This issue only happens in the released versions of GNU Make. > CC_FLAGS_FTRACE will not contain the trailing space if you use > the latest GNU Make, which contains commit b90fabc8d6f3 > ("* NEWS: Do not insert a space during '+=' if the value is empty.") > ] Wow, this one gets even more specific. I had gone down the rabbit hole on this one, I didn't care to learn how or why that extra space got there :) Now we know, thanks for running that down and adding the note about GNU Make. -- Joe ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-28 13:36 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-25 16:04 [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring Joe Lawrence 2019-03-25 16:21 ` Steven Rostedt 2019-03-26 5:29 ` Masahiro Yamada 2019-03-26 17:33 ` [PATCH v2] " Joe Lawrence 2019-03-27 6:17 ` Nicholas Piggin 2019-03-28 12:57 ` Masahiro Yamada 2019-03-28 13:36 ` Joe Lawrence
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).