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@xxxxxxxxxx> 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@xxxxxxxxxx> >> > --- >> > >> > 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@xxxxxxxxxx> > 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@xxxxxxxxxxxxx> (refactoring) > Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer"). > Signed-off-by: Joe Lawrence <joe.lawrence@xxxxxxxxxx> > --- > 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 > >