From: Joe Lawrence <joe.lawrence@redhat.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Michal Marek <michal.lkml@markovi.net>,
Nicholas Piggin <npiggin@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>
Subject: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring
Date: Tue, 26 Mar 2019 13:33:08 -0400 [thread overview]
Message-ID: <20190326173308.GA26546@redhat.com> (raw)
In-Reply-To: <CAK7LNARyD=tG0JhwN_mjdyDRSPAF8UoZKvqAT3Ah_v7GfymCPg@mail.gmail.com>
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
WARNING: multiple messages have this Message-ID (diff)
From: Joe Lawrence <joe.lawrence@redhat.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>,
Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
Nicholas Piggin <npiggin@gmail.com>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: [PATCH v2] kbuild: strip whitespace in cmd_record_mcount findstring
Date: Tue, 26 Mar 2019 13:33:08 -0400 [thread overview]
Message-ID: <20190326173308.GA26546@redhat.com> (raw)
In-Reply-To: <CAK7LNARyD=tG0JhwN_mjdyDRSPAF8UoZKvqAT3Ah_v7GfymCPg@mail.gmail.com>
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
next prev parent reply other threads:[~2019-03-26 17:33 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-25 16:04 [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring Joe Lawrence
2019-03-25 16:04 ` Joe Lawrence
2019-03-25 16:21 ` Steven Rostedt
2019-03-25 16:21 ` Steven Rostedt
2019-03-26 5:29 ` Masahiro Yamada
2019-03-26 5:29 ` Masahiro Yamada
2019-03-26 17:33 ` Joe Lawrence [this message]
2019-03-26 17:33 ` [PATCH v2] " Joe Lawrence
2019-03-27 6:17 ` Nicholas Piggin
2019-03-27 6:17 ` Nicholas Piggin
2019-03-28 12:57 ` Masahiro Yamada
2019-03-28 12:57 ` Masahiro Yamada
2019-03-28 13:36 ` Joe Lawrence
2019-03-28 13:36 ` Joe Lawrence
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190326173308.GA26546@redhat.com \
--to=joe.lawrence@redhat.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=michal.lkml@markovi.net \
--cc=npiggin@gmail.com \
--cc=rostedt@goodmis.org \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.