All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE
@ 2018-09-05 22:35 Paulo Zanoni
  2018-09-05 22:36 ` [RFC 2/2] tracing/Makefile: reindent the CONFIG_FUNCTION_TRACER chunk Paulo Zanoni
  2018-09-06  2:10 ` [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE Steven Rostedt
  0 siblings, 2 replies; 7+ messages in thread
From: Paulo Zanoni @ 2018-09-05 22:35 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Paulo Zanoni, Vasily Gorbik, Michal Marek, Steven Rostedt,
	Ingo Molnar, Tvrtko Ursulin

As a Kernel developer, I make heavy use of "make targz-pkg" in order
to locally compile and remotely install my development Kernels. The
nice feature I rely on is that after a normal "make", "make targz-pkg"
only generates the tarball without having to recompile everything.

That was true until commit f28bc3c32c05 ("tracing: Handle
CC_FLAGS_FTRACE more accurately"). After it, running "make targz-pkg"
after "make" will recompile the whole Kernel tree, making my
development workflow much slower.

The Kernel is choosing to recompile everything because it claims the
command line has changed. A diff of the .cmd files show a repeated
-mfentry in one of the files. It seems that something on make
targz-pkg triggers the double -mfentry but not the double -pg.

So this patch attempts to deal with that problem by handling -mfentry
and others just like we handle -pg: don't append them if
CC_FLAGS_FTRACE is already defined. I'm not a Makefile expert and so I
can't claim this won't break anything else, hopefully if this patch
gets applied it will have a Reviewed-by tag from some actual Makefile
expert. I can claim this patch fixes the specific regression I
reported.

Fixes: commit f28bc3c32c05 ("tracing: Handle CC_FLAGS_FTRACE more
 accurately")
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 Makefile | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 19948e556941..2c8df481b4f1 100644
--- a/Makefile
+++ b/Makefile
@@ -755,28 +755,30 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-ifndef CC_FLAGS_FTRACE
-CC_FLAGS_FTRACE := -pg
-endif
+cc_flags_ftrace_ := -pg
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
   # gcc 5 supports generating the mcount tables directly
   ifeq ($(call cc-option-yn,-mrecord-mcount),y)
-    CC_FLAGS_FTRACE	+= -mrecord-mcount
+    cc_flags_ftrace_	+= -mrecord-mcount
     export CC_USING_RECORD_MCOUNT := 1
   endif
   ifdef CONFIG_HAVE_NOP_MCOUNT
     ifeq ($(call cc-option-yn, -mnop-mcount),y)
-      CC_FLAGS_FTRACE	+= -mnop-mcount
+      cc_flags_ftrace_	+= -mnop-mcount
       CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
     endif
   endif
 endif
 ifdef CONFIG_HAVE_FENTRY
   ifeq ($(call cc-option-yn, -mfentry),y)
-    CC_FLAGS_FTRACE	+= -mfentry
+    cc_flags_ftrace_	+= -mfentry
     CC_FLAGS_USING	+= -DCC_USING_FENTRY
   endif
 endif
+
+ifndef CC_FLAGS_FTRACE
+  CC_FLAGS_FTRACE := $(cc_flags_ftrace_)
+endif
 export CC_FLAGS_FTRACE
 KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
 KBUILD_AFLAGS	+= $(CC_FLAGS_USING)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC 2/2] tracing/Makefile: reindent the CONFIG_FUNCTION_TRACER chunk
  2018-09-05 22:35 [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE Paulo Zanoni
@ 2018-09-05 22:36 ` Paulo Zanoni
  2018-09-06  2:10 ` [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE Steven Rostedt
  1 sibling, 0 replies; 7+ messages in thread
From: Paulo Zanoni @ 2018-09-05 22:36 UTC (permalink / raw)
  To: linux-kbuild; +Cc: Paulo Zanoni, Steven Rostedt

The whole block was lacking an indent level for the
CONFIG_FUNCTION_TRACER ifdef, which made the code a little harder to
read. Add the missing indent level and also document the endif for
easier reading.

At the very end some tabs were replaced with spaces. My small
understanding of Makefiles suggests that spaces are the appropriate
characters here.

Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 Makefile | 60 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/Makefile b/Makefile
index 2c8df481b4f1..dce0f7330038 100644
--- a/Makefile
+++ b/Makefile
@@ -755,40 +755,40 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-cc_flags_ftrace_ := -pg
-ifdef CONFIG_FTRACE_MCOUNT_RECORD
-  # gcc 5 supports generating the mcount tables directly
-  ifeq ($(call cc-option-yn,-mrecord-mcount),y)
-    cc_flags_ftrace_	+= -mrecord-mcount
-    export CC_USING_RECORD_MCOUNT := 1
-  endif
-  ifdef CONFIG_HAVE_NOP_MCOUNT
-    ifeq ($(call cc-option-yn, -mnop-mcount),y)
-      cc_flags_ftrace_	+= -mnop-mcount
-      CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
+  cc_flags_ftrace_ := -pg
+  ifdef CONFIG_FTRACE_MCOUNT_RECORD
+    # gcc 5 supports generating the mcount tables directly
+    ifeq ($(call cc-option-yn,-mrecord-mcount),y)
+      cc_flags_ftrace_	+= -mrecord-mcount
+      export CC_USING_RECORD_MCOUNT := 1
+    endif
+    ifdef CONFIG_HAVE_NOP_MCOUNT
+      ifeq ($(call cc-option-yn, -mnop-mcount),y)
+        cc_flags_ftrace_	+= -mnop-mcount
+        CC_FLAGS_USING		+= -DCC_USING_NOP_MCOUNT
+      endif
     endif
   endif
-endif
-ifdef CONFIG_HAVE_FENTRY
-  ifeq ($(call cc-option-yn, -mfentry),y)
-    cc_flags_ftrace_	+= -mfentry
-    CC_FLAGS_USING	+= -DCC_USING_FENTRY
+  ifdef CONFIG_HAVE_FENTRY
+    ifeq ($(call cc-option-yn, -mfentry),y)
+      cc_flags_ftrace_	+= -mfentry
+      CC_FLAGS_USING	+= -DCC_USING_FENTRY
+    endif
   endif
-endif
 
-ifndef CC_FLAGS_FTRACE
-  CC_FLAGS_FTRACE := $(cc_flags_ftrace_)
-endif
-export CC_FLAGS_FTRACE
-KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
-KBUILD_AFLAGS	+= $(CC_FLAGS_USING)
-ifdef CONFIG_DYNAMIC_FTRACE
-	ifdef CONFIG_HAVE_C_RECORDMCOUNT
-		BUILD_C_RECORDMCOUNT := y
-		export BUILD_C_RECORDMCOUNT
-	endif
-endif
-endif
+  ifndef CC_FLAGS_FTRACE
+    CC_FLAGS_FTRACE := $(cc_flags_ftrace_)
+  endif
+  export CC_FLAGS_FTRACE
+  KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
+  KBUILD_AFLAGS	+= $(CC_FLAGS_USING)
+  ifdef CONFIG_DYNAMIC_FTRACE
+    ifdef CONFIG_HAVE_C_RECORDMCOUNT
+      BUILD_C_RECORDMCOUNT := y
+      export BUILD_C_RECORDMCOUNT
+    endif
+  endif
+endif # CONFIG_FUNCTION_TRACER
 
 # We trigger additional mismatches with less inlining
 ifdef CONFIG_DEBUG_SECTION_MISMATCH
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE
  2018-09-05 22:35 [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE Paulo Zanoni
  2018-09-05 22:36 ` [RFC 2/2] tracing/Makefile: reindent the CONFIG_FUNCTION_TRACER chunk Paulo Zanoni
@ 2018-09-06  2:10 ` Steven Rostedt
  2018-09-06 13:07   ` Vasily Gorbik
  1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2018-09-06  2:10 UTC (permalink / raw)
  To: Paulo Zanoni
  Cc: linux-kbuild, Vasily Gorbik, Michal Marek, Ingo Molnar, Tvrtko Ursulin

On Wed,  5 Sep 2018 15:35:59 -0700
Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:

> As a Kernel developer, I make heavy use of "make targz-pkg" in order
> to locally compile and remotely install my development Kernels. The
> nice feature I rely on is that after a normal "make", "make targz-pkg"
> only generates the tarball without having to recompile everything.
> 
> That was true until commit f28bc3c32c05 ("tracing: Handle
> CC_FLAGS_FTRACE more accurately"). After it, running "make targz-pkg"
> after "make" will recompile the whole Kernel tree, making my
> development workflow much slower.
> 
> The Kernel is choosing to recompile everything because it claims the
> command line has changed. A diff of the .cmd files show a repeated
> -mfentry in one of the files. It seems that something on make
> targz-pkg triggers the double -mfentry but not the double -pg.

It's probably because the addition of -mfentry isn't protected behind a
ifndef block (like you added in this patch).

> 
> So this patch attempts to deal with that problem by handling -mfentry
> and others just like we handle -pg: don't append them if
> CC_FLAGS_FTRACE is already defined. I'm not a Makefile expert and so I
> can't claim this won't break anything else, hopefully if this patch
> gets applied it will have a Reviewed-by tag from some actual Makefile
> expert. I can claim this patch fixes the specific regression I
> reported.
> 
> Fixes: commit f28bc3c32c05 ("tracing: Handle CC_FLAGS_FTRACE more
>  accurately")
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: linux-kbuild@vger.kernel.org
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  Makefile | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 19948e556941..2c8df481b4f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -755,28 +755,30 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
>  endif
>  
>  ifdef CONFIG_FUNCTION_TRACER
> -ifndef CC_FLAGS_FTRACE
> -CC_FLAGS_FTRACE := -pg
> -endif
> +cc_flags_ftrace_ := -pg

I have no problem with this patch, but why the extra underscore at the
end of cc_flags_ftrace_, and not just cc_flags_ftrace?

-- Steve

>  ifdef CONFIG_FTRACE_MCOUNT_RECORD
>    # gcc 5 supports generating the mcount tables directly
>    ifeq ($(call cc-option-yn,-mrecord-mcount),y)
> -    CC_FLAGS_FTRACE	+= -mrecord-mcount
> +    cc_flags_ftrace_	+= -mrecord-mcount
>      export CC_USING_RECORD_MCOUNT := 1
>    endif
>    ifdef CONFIG_HAVE_NOP_MCOUNT
>      ifeq ($(call cc-option-yn, -mnop-mcount),y)
> -      CC_FLAGS_FTRACE	+= -mnop-mcount
> +      cc_flags_ftrace_	+= -mnop-mcount
>        CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
>      endif
>    endif
>  endif
>  ifdef CONFIG_HAVE_FENTRY
>    ifeq ($(call cc-option-yn, -mfentry),y)
> -    CC_FLAGS_FTRACE	+= -mfentry
> +    cc_flags_ftrace_	+= -mfentry
>      CC_FLAGS_USING	+= -DCC_USING_FENTRY
>    endif
>  endif
> +
> +ifndef CC_FLAGS_FTRACE
> +  CC_FLAGS_FTRACE := $(cc_flags_ftrace_)
> +endif
>  export CC_FLAGS_FTRACE
>  KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
>  KBUILD_AFLAGS	+= $(CC_FLAGS_USING)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE
  2018-09-06  2:10 ` [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE Steven Rostedt
@ 2018-09-06 13:07   ` Vasily Gorbik
  2018-09-10 17:59     ` [PATCH v2] " Paulo Zanoni
  0 siblings, 1 reply; 7+ messages in thread
From: Vasily Gorbik @ 2018-09-06 13:07 UTC (permalink / raw)
  To: Steven Rostedt, Paulo Zanoni
  Cc: linux-kbuild, Michal Marek, Ingo Molnar, Tvrtko Ursulin

On Wed, Sep 05, 2018 at 10:10:59PM -0400, Steven Rostedt wrote:
> On Wed,  5 Sep 2018 15:35:59 -0700
> Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> 
> > As a Kernel developer, I make heavy use of "make targz-pkg" in order
> > to locally compile and remotely install my development Kernels. The
> > nice feature I rely on is that after a normal "make", "make targz-pkg"
> > only generates the tarball without having to recompile everything.
> > 
> > That was true until commit f28bc3c32c05 ("tracing: Handle
> > CC_FLAGS_FTRACE more accurately"). After it, running "make targz-pkg"
> > after "make" will recompile the whole Kernel tree, making my
> > development workflow much slower.
> > 
> > The Kernel is choosing to recompile everything because it claims the
> > command line has changed. A diff of the .cmd files show a repeated
> > -mfentry in one of the files. It seems that something on make
> > targz-pkg triggers the double -mfentry but not the double -pg.
> 
> It's probably because the addition of -mfentry isn't protected behind a
> ifndef block (like you added in this patch).

scripts/package/Makefile:
tar%pkg: FORCE
჻·······$(MAKE) KBUILD_SRC=
჻·······$(CONFIG_SHELL) $(srctree)/scripts/package/buildtar $@

scripts/package/buildtar:
if grep -q '^CONFIG_MODULES=y' "${KCONFIG_CONFIG}"; then
჻·······make ARCH="${ARCH}" O="${objtree}" KBUILD_SRC= INSTALL_MOD_PATH="${tmpdir}" modules_install

"make targz-pkg" target calls "make modules_install" and environment is
already populated with exported variables, CC_FLAGS_FTRACE being one
of them. In other words top Makefile is called recursively in
sub-make. I assume, to make it work in general exported variables should be
always recalculated to avoid side effects.

In this particular case:
ifndef CC_FLAGS_FTRACE
CC_FLAGS_FTRACE := -pg
endif
is bad, because we don't know if CC_FLAGS_FTRACE is defined in platform
specific arch/$(SRCARCH)/Makefile (included earlier), or coming from parent
make process as exported variable.

In my view one simple way to fix this particular case would be to move:
ifdef CONFIG_FUNCTION_TRACER
        CC_FLAGS_FTRACE := -pg
endif
before:
include arch/$(SRCARCH)/Makefile
where platforms set custom CC_FLAGS_FTRACE.

Or alternatively change the name of the variable that platforms have to set in
arch/$(SRCARCH)/Makefile (so that it does not conflict with exported variable).

> > 
> > So this patch attempts to deal with that problem by handling -mfentry
> > and others just like we handle -pg: don't append them if
> > CC_FLAGS_FTRACE is already defined. I'm not a Makefile expert and so I
> > can't claim this won't break anything else, hopefully if this patch
> > gets applied it will have a Reviewed-by tag from some actual Makefile
> > expert. I can claim this patch fixes the specific regression I
> > reported.
> > 
> > Fixes: commit f28bc3c32c05 ("tracing: Handle CC_FLAGS_FTRACE more
> >  accurately")
> > Cc: Vasily Gorbik <gor@linux.ibm.com>
> > Cc: Michal Marek <michal.lkml@markovi.net>
> > Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: linux-kbuild@vger.kernel.org
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  Makefile | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 19948e556941..2c8df481b4f1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -755,28 +755,30 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
> >  endif
> >  
> >  ifdef CONFIG_FUNCTION_TRACER
> > -ifndef CC_FLAGS_FTRACE
> > -CC_FLAGS_FTRACE := -pg
> > -endif
> > +cc_flags_ftrace_ := -pg
> 
> I have no problem with this patch, but why the extra underscore at the
> end of cc_flags_ftrace_, and not just cc_flags_ftrace?
> 
> -- Steve
> 
> >  ifdef CONFIG_FTRACE_MCOUNT_RECORD
> >    # gcc 5 supports generating the mcount tables directly
> >    ifeq ($(call cc-option-yn,-mrecord-mcount),y)
> > -    CC_FLAGS_FTRACE	+= -mrecord-mcount
> > +    cc_flags_ftrace_	+= -mrecord-mcount
> >      export CC_USING_RECORD_MCOUNT := 1
> >    endif
> >    ifdef CONFIG_HAVE_NOP_MCOUNT
> >      ifeq ($(call cc-option-yn, -mnop-mcount),y)
> > -      CC_FLAGS_FTRACE	+= -mnop-mcount
> > +      cc_flags_ftrace_	+= -mnop-mcount
> >        CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
> >      endif
> >    endif
> >  endif
> >  ifdef CONFIG_HAVE_FENTRY
> >    ifeq ($(call cc-option-yn, -mfentry),y)
> > -    CC_FLAGS_FTRACE	+= -mfentry
> > +    cc_flags_ftrace_	+= -mfentry
> >      CC_FLAGS_USING	+= -DCC_USING_FENTRY
> >    endif
> >  endif
> > +
> > +ifndef CC_FLAGS_FTRACE
> > +  CC_FLAGS_FTRACE := $(cc_flags_ftrace_)
> > +endif

Unfortunately this changes logic and inconsistent. With that change
platforms which define CC_FLAGS_FTRACE in arch/$(SRCARCH)/Makefile
would end up without extra -mrecord-mcount, -mnop-mcount and -mfentry
build flags, while CC_FLAGS_USING would be populated and
CC_USING_RECORD_MCOUNT set.

> >  export CC_FLAGS_FTRACE
> >  KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
> >  KBUILD_AFLAGS	+= $(CC_FLAGS_USING)
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE
  2018-09-06 13:07   ` Vasily Gorbik
@ 2018-09-10 17:59     ` Paulo Zanoni
  2018-09-12 10:18       ` Vasily Gorbik
  2018-09-12 18:37       ` Steven Rostedt
  0 siblings, 2 replies; 7+ messages in thread
From: Paulo Zanoni @ 2018-09-10 17:59 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Paulo Zanoni, Vasily Gorbik, Michal Marek, Steven Rostedt,
	Ingo Molnar, Tvrtko Ursulin

As a Kernel developer, I make heavy use of "make targz-pkg" in order
to locally compile and remotely install my development Kernels. The
nice feature I rely on is that after a normal "make", "make targz-pkg"
only generates the tarball without having to recompile everything.

That was true until commit f28bc3c32c05 ("tracing: Handle
CC_FLAGS_FTRACE more accurately"). After it, running "make targz-pkg"
after "make" will recompile the whole Kernel tree, making my
development workflow much slower.

The Kernel is choosing to recompile everything because it claims the
command line has changed. A diff of the .cmd files show a repeated
-mfentry in one of the files. That is because "make targz-pkg" calls
"make modules_install" and the environment is already populated with
the exported variables, CC_FLAGS_FTRACE being one of them. Then,
-mfentry gets duplicated because it is not protected behind an ifndef
block, like -pg.

To complicate the problem a little bit more, architectures can define
their own version CC_FLAGS_FTRACE, so our code not only has to
consider recursive Makefiles, but also architecture overrides.

So in this patch we move CC_FLAGFS_FTRACE up and unconditionally
define it to -pg. Then we let the architecture Makefiles possibly
override it, and finally append the extra options later. This ensures
the variable is always fully redefined at each invocation so recursive
Makefiles don't keep appending, and hopefully it maintains the
intended behavior on how architectures can override the defaults..

Thanks Steven Rostedt and Vasily Gorbik for the help on this
regression.

Fixes: commit f28bc3c32c05 ("tracing: Handle CC_FLAGS_FTRACE more
 accurately")
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 Makefile | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 4d5c883a98e5..a5ef6818157a 100644
--- a/Makefile
+++ b/Makefile
@@ -616,6 +616,11 @@ CFLAGS_GCOV	:= -fprofile-arcs -ftest-coverage \
 	$(call cc-disable-warning,maybe-uninitialized,)
 export CFLAGS_GCOV
 
+# The arch Makefiles can override CC_FLAGS_FTRACE. We may also append it later.
+ifdef CONFIG_FUNCTION_TRACER
+  CC_FLAGS_FTRACE := -pg
+endif
+
 # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default
 # values of the respective KBUILD_* variables
 ARCH_CPPFLAGS :=
@@ -755,9 +760,6 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-ifndef CC_FLAGS_FTRACE
-CC_FLAGS_FTRACE := -pg
-endif
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
   # gcc 5 supports generating the mcount tables directly
   ifeq ($(call cc-option-yn,-mrecord-mcount),y)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE
  2018-09-10 17:59     ` [PATCH v2] " Paulo Zanoni
@ 2018-09-12 10:18       ` Vasily Gorbik
  2018-09-12 18:37       ` Steven Rostedt
  1 sibling, 0 replies; 7+ messages in thread
From: Vasily Gorbik @ 2018-09-12 10:18 UTC (permalink / raw)
  To: Paulo Zanoni
  Cc: linux-kbuild, Michal Marek, Steven Rostedt, Ingo Molnar, Tvrtko Ursulin

On Mon, Sep 10, 2018 at 10:59:56AM -0700, Paulo Zanoni wrote:
> As a Kernel developer, I make heavy use of "make targz-pkg" in order
> to locally compile and remotely install my development Kernels. The
> nice feature I rely on is that after a normal "make", "make targz-pkg"
> only generates the tarball without having to recompile everything.
> 
> That was true until commit f28bc3c32c05 ("tracing: Handle
> CC_FLAGS_FTRACE more accurately"). After it, running "make targz-pkg"
> after "make" will recompile the whole Kernel tree, making my
> development workflow much slower.
> 
> The Kernel is choosing to recompile everything because it claims the
> command line has changed. A diff of the .cmd files show a repeated
> -mfentry in one of the files. That is because "make targz-pkg" calls
> "make modules_install" and the environment is already populated with
> the exported variables, CC_FLAGS_FTRACE being one of them. Then,
> -mfentry gets duplicated because it is not protected behind an ifndef
> block, like -pg.
> 
> To complicate the problem a little bit more, architectures can define
> their own version CC_FLAGS_FTRACE, so our code not only has to
> consider recursive Makefiles, but also architecture overrides.
> 
> So in this patch we move CC_FLAGFS_FTRACE up and unconditionally
                           CC_FLAGS_FTRACE
> define it to -pg. Then we let the architecture Makefiles possibly
> override it, and finally append the extra options later. This ensures
> the variable is always fully redefined at each invocation so recursive
> Makefiles don't keep appending, and hopefully it maintains the
> intended behavior on how architectures can override the defaults..
> 
> Thanks Steven Rostedt and Vasily Gorbik for the help on this
> regression.
> 
> Fixes: commit f28bc3c32c05 ("tracing: Handle CC_FLAGS_FTRACE more
>  accurately")
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: linux-kbuild@vger.kernel.org
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  Makefile | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 4d5c883a98e5..a5ef6818157a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -616,6 +616,11 @@ CFLAGS_GCOV	:= -fprofile-arcs -ftest-coverage \
>  	$(call cc-disable-warning,maybe-uninitialized,)
>  export CFLAGS_GCOV
>  
> +# The arch Makefiles can override CC_FLAGS_FTRACE. We may also append it later.
> +ifdef CONFIG_FUNCTION_TRACER
> +  CC_FLAGS_FTRACE := -pg
> +endif
> +
>  # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default
>  # values of the respective KBUILD_* variables
>  ARCH_CPPFLAGS :=
> @@ -755,9 +760,6 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
>  endif
>  
>  ifdef CONFIG_FUNCTION_TRACER
> -ifndef CC_FLAGS_FTRACE
> -CC_FLAGS_FTRACE := -pg
> -endif
>  ifdef CONFIG_FTRACE_MCOUNT_RECORD
>    # gcc 5 supports generating the mcount tables directly
>    ifeq ($(call cc-option-yn,-mrecord-mcount),y)
> -- 
> 2.17.1
> 

Acked-by: Vasily Gorbik <gor@linux.ibm.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE
  2018-09-10 17:59     ` [PATCH v2] " Paulo Zanoni
  2018-09-12 10:18       ` Vasily Gorbik
@ 2018-09-12 18:37       ` Steven Rostedt
  1 sibling, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2018-09-12 18:37 UTC (permalink / raw)
  To: Paulo Zanoni
  Cc: linux-kbuild, Vasily Gorbik, Michal Marek, Ingo Molnar,
	Tvrtko Ursulin, LKML

On Mon, 10 Sep 2018 10:59:56 -0700
Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:

> As a Kernel developer, I make heavy use of "make targz-pkg" in order
> to locally compile and remotely install my development Kernels. The
> nice feature I rely on is that after a normal "make", "make targz-pkg"
> only generates the tarball without having to recompile everything.
> 
> That was true until commit f28bc3c32c05 ("tracing: Handle
> CC_FLAGS_FTRACE more accurately"). After it, running "make targz-pkg"
> after "make" will recompile the whole Kernel tree, making my
> development workflow much slower.
> 
> The Kernel is choosing to recompile everything because it claims the
> command line has changed. A diff of the .cmd files show a repeated
> -mfentry in one of the files. That is because "make targz-pkg" calls
> "make modules_install" and the environment is already populated with
> the exported variables, CC_FLAGS_FTRACE being one of them. Then,
> -mfentry gets duplicated because it is not protected behind an ifndef
> block, like -pg.
> 
> To complicate the problem a little bit more, architectures can define
> their own version CC_FLAGS_FTRACE, so our code not only has to
> consider recursive Makefiles, but also architecture overrides.
> 
> So in this patch we move CC_FLAGFS_FTRACE up and unconditionally
> define it to -pg. Then we let the architecture Makefiles possibly
> override it, and finally append the extra options later. This ensures
> the variable is always fully redefined at each invocation so recursive
> Makefiles don't keep appending, and hopefully it maintains the
> intended behavior on how architectures can override the defaults..
> 
> Thanks Steven Rostedt and Vasily Gorbik for the help on this
> regression.
> 

I'll pull in this patch, but it's always a good idea to also Cc LKML
(which I added to this email).

I also updated the change log and added Vasily's acked-by.

-- Steve

> Fixes: commit f28bc3c32c05 ("tracing: Handle CC_FLAGS_FTRACE more
>  accurately")
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: linux-kbuild@vger.kernel.org
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  Makefile | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 4d5c883a98e5..a5ef6818157a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -616,6 +616,11 @@ CFLAGS_GCOV	:= -fprofile-arcs -ftest-coverage \
>  	$(call cc-disable-warning,maybe-uninitialized,)
>  export CFLAGS_GCOV
>  
> +# The arch Makefiles can override CC_FLAGS_FTRACE. We may also append it later.
> +ifdef CONFIG_FUNCTION_TRACER
> +  CC_FLAGS_FTRACE := -pg
> +endif
> +
>  # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default
>  # values of the respective KBUILD_* variables
>  ARCH_CPPFLAGS :=
> @@ -755,9 +760,6 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
>  endif
>  
>  ifdef CONFIG_FUNCTION_TRACER
> -ifndef CC_FLAGS_FTRACE
> -CC_FLAGS_FTRACE := -pg
> -endif
>  ifdef CONFIG_FTRACE_MCOUNT_RECORD
>    # gcc 5 supports generating the mcount tables directly
>    ifeq ($(call cc-option-yn,-mrecord-mcount),y)


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-09-12 18:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-05 22:35 [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE Paulo Zanoni
2018-09-05 22:36 ` [RFC 2/2] tracing/Makefile: reindent the CONFIG_FUNCTION_TRACER chunk Paulo Zanoni
2018-09-06  2:10 ` [RFC 1/2] tracing/Makefile: fix handling redefinition of CC_FLAGS_FTRACE Steven Rostedt
2018-09-06 13:07   ` Vasily Gorbik
2018-09-10 17:59     ` [PATCH v2] " Paulo Zanoni
2018-09-12 10:18       ` Vasily Gorbik
2018-09-12 18:37       ` Steven Rostedt

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.