[RFC][PATCH 0/2] kbuild: create force-deps for fixdep for recordmcount

[RFC][PATCH 0/2] kbuild: create force-deps for fixdep for recordmcount

[Steven Rostedt]

While doing a ktest patchcheck on several patches I've pulled,
an update to recordmcount caused ftrace to fail. This is because
ktest does a make mrproper for the first patch, but just uses the
dependencies for the patches after that. But one of the patches
modified both the way recordmcount worked, and what ftrace expected
in the kernel. Since modification of recordmcount does not force
updates to all object files, the expectation that ftrace had on
the object files was incorrect and caused ftrace to fail.

This is an RFC to modify fixdep to allow addition of files that would
be added as dependencies to all objects in the kernel.

Thoughts?

Steven Rostedt (2):
      kbuild: Add force-deps to fixdep
      ftrace/kbuild: Add recordmcount files to force full build

----
 scripts/Kbuild.include |    3 ++-
 scripts/Makefile.build |    6 ++++--
 scripts/basic/fixdep.c |   18 ++++++++++++++++--
 3 files changed, 22 insertions(+), 5 deletions(-)

[RFC][PATCH 1/2] kbuild: Add force-deps to fixdep

[Steven Rostedt]

[-- Attachment #1: 0001-kbuild-Add-force-deps-to-fixdep.patch --]
[-- Type: text/plain, Size: 3687 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Allow some files to cause a full recompile of the kernel.
There are some cases where all objects in the kernel may indirectly
depend on a tool or script. A force-deps option has been added
to fixdep to allow a list of files to be passed in to add them
to the dependency of kernel files that are not found by gcc -MD
option.

Cc: Michal Marek <mmarek@suse.cz>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/Kbuild.include |    3 ++-
 scripts/Makefile.build |    4 ++--
 scripts/basic/fixdep.c |   18 ++++++++++++++++--
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index ed2773e..219f2c1 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -210,7 +210,8 @@ if_changed = $(if $(strip $(any-prereq) $(arg-check)),                       \
 if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \
 	@set -e;                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
-	scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).tmp;\
+	scripts/basic/fixdep $(depfile) $@ '$(make-cmd)'		     \
+			      "$(force-deps)" > $(dot-target).tmp;	     \
 	rm -f $(depfile);                                                    \
 	mv -f $(dot-target).tmp $(dot-target).cmd)
 
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d5f925a..0ff5a58 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -272,8 +272,8 @@ define rule_cc_o_c
 	$(cmd_modversions)						  \
 	$(call echo-cmd,record_mcount)					  \
 	$(cmd_record_mcount)						  \
-	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
-	                                              $(dot-target).tmp;  \
+	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)'	  \
+	                           "$(force-deps)" >  $(dot-target).tmp;  \
 	rm -f $(depfile);						  \
 	mv -f $(dot-target).tmp $(dot-target).cmd
 endef
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 291228e..2b5045d 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -62,7 +62,7 @@
  *
  * It is invoked as
  *
- *   fixdep <depfile> <target> <cmdline>
+ *   fixdep <depfile> <target> <cmdline> [<force-deps>]
  *
  * and will read the dependency file <depfile>
  *
@@ -91,6 +91,9 @@
  * unrelated CONFIG_ options all over the place, it's not an
  * efficiency problem either.
  *
+ * An optional <force-deps> may also be added to include a list of
+ * files that are forced dependencies.
+ *
  * (Note: it'd be easy to port over the complete mkdep state machine,
  *  but I don't think the added complexity is worth it)
  */
@@ -123,6 +126,7 @@
 char *target;
 char *depfile;
 char *cmdline;
+char *forced;
 
 static void usage(void)
 {
@@ -333,6 +337,7 @@ static void parse_dep_file(void *map, size_t len)
 	clear_config();
 
 	first = 1;
+ again:
 	while (m < end) {
 		while (m < end && (*m == ' ' || *m == '\\' || *m == '\n'))
 			m++;
@@ -362,6 +367,13 @@ static void parse_dep_file(void *map, size_t len)
 		first = 0;
 		m = p + 1;
 	}
+	if (forced) {
+		m = forced;
+		end = m + strlen(m);
+		forced = NULL;
+		goto again;
+	}
+
 	printf("\n%s: $(deps_%s)\n\n", target, target);
 	printf("$(deps_%s):\n", target);
 }
@@ -418,12 +430,14 @@ int main(int argc, char *argv[])
 {
 	traps();
 
-	if (argc != 4)
+	if (argc != 4 && argc != 5)
 		usage();
 
 	depfile = argv[1];
 	target = argv[2];
 	cmdline = argv[3];
+	if (argc == 5)
+		forced = argv[4];
 
 	print_cmdline();
 	print_deps();
-- 
1.7.2.3

[RFC][PATCH 2/2] ftrace/kbuild: Add recordmcount files to force full build

[Steven Rostedt]

[-- Attachment #1: 0002-ftrace-kbuild-Add-recordmcount-files-to-force-full-b.patch --]
[-- Type: text/plain, Size: 1322 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Modifications to recordmcount must be performed on all object
files to stay consistent with what the kernel code may expect.
Add the recordmcount files to the force-deps to make sure
any change to them causes a full recompile.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/Makefile.build |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 0ff5a58..057d6e9 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -252,6 +252,7 @@ sub_cmd_record_mcount =					\
 	if [ $(@) != "scripts/mod/empty.o" ]; then	\
 		$(objtree)/scripts/recordmcount "$(@)";	\
 	fi;
+force-deps += $(srctree)/scripts/recordmcount.c $(srctree)/scripts/recordmcount.h
 else
 sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
 	"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
@@ -259,6 +260,7 @@ sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH
 	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CFLAGS)" \
 	"$(LD)" "$(NM)" "$(RM)" "$(MV)" \
 	"$(if $(part-of-module),1,0)" "$(@)";
+force-deps += $(srctree)/scripts/recordmcount.pl
 endif
 cmd_record_mcount = 						\
 	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\
-- 
1.7.2.3

Re: [RFC][PATCH 2/2] ftrace/kbuild: Add recordmcount files to force full build

[Michal Marek]

On Mon, May 16, 2011 at 10:10:29PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Modifications to recordmcount must be performed on all object
> files to stay consistent with what the kernel code may expect.
> Add the recordmcount files to the force-deps to make sure
> any change to them causes a full recompile.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  scripts/Makefile.build |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 0ff5a58..057d6e9 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -252,6 +252,7 @@ sub_cmd_record_mcount =					\
>  	if [ $(@) != "scripts/mod/empty.o" ]; then	\
>  		$(objtree)/scripts/recordmcount "$(@)";	\
>  	fi;
> +force-deps += $(srctree)/scripts/recordmcount.c $(srctree)/scripts/recordmcount.h
>  else
>  sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
>  	"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
> @@ -259,6 +260,7 @@ sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH
>  	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CFLAGS)" \
>  	"$(LD)" "$(NM)" "$(RM)" "$(MV)" \
>  	"$(if $(part-of-module),1,0)" "$(@)";
> +force-deps += $(srctree)/scripts/recordmcount.pl
>  endif
>  cmd_record_mcount = 						\
>  	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\

We could add recordmcount.c as a dependency in scripts/Makefile.build
directly, without playing with fixdep, like this (untested):

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index f133641..bc2c8d3 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -268,6 +268,7 @@ sub_cmd_record_mcount =					\
 	if [ $(@) != "scripts/mod/empty.o" ]; then	\
 		$(objtree)/scripts/recordmcount "$(@)";	\
 	fi;
+recordmcount_source := $(srctree)/scripts/recordmcount.c
 else
 sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
 	"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
@@ -275,6 +276,7 @@ sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH
 	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CFLAGS)" \
 	"$(LD)" "$(NM)" "$(RM)" "$(MV)" \
 	"$(if $(part-of-module),1,0)" "$(@)";
+recordmcount_source := $(srctree)/scripts/recordmcount.pl
 endif
 cmd_record_mcount = 						\
 	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\
@@ -295,13 +297,13 @@ define rule_cc_o_c
 endef
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)

Michal

Re: [RFC][PATCH 2/2] ftrace/kbuild: Add recordmcount files to force full build

[Steven Rostedt]

On Tue, 2011-05-17 at 15:36 +0200, Michal Marek wrote:

> We could add recordmcount.c as a dependency in scripts/Makefile.build
> directly, without playing with fixdep, like this (untested):

I actually tried this first, and it didn't seem to work. I'll go ahead
and give it another try (maybe I did something wrong). But I do prefer
this over my fixdep change.

Thanks, I'll let you know the outcome.

-- Steve

> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index f133641..bc2c8d3 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -268,6 +268,7 @@ sub_cmd_record_mcount =					\
>  	if [ $(@) != "scripts/mod/empty.o" ]; then	\
>  		$(objtree)/scripts/recordmcount "$(@)";	\
>  	fi;
> +recordmcount_source := $(srctree)/scripts/recordmcount.c
>  else
>  sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
>  	"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
> @@ -275,6 +276,7 @@ sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH
>  	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CFLAGS)" \
>  	"$(LD)" "$(NM)" "$(RM)" "$(MV)" \
>  	"$(if $(part-of-module),1,0)" "$(@)";
> +recordmcount_source := $(srctree)/scripts/recordmcount.pl
>  endif
>  cmd_record_mcount = 						\
>  	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\
> @@ -295,13 +297,13 @@ define rule_cc_o_c
>  endef
>  
>  # Built-in and composite module parts
> -$(obj)/%.o: $(src)/%.c FORCE
> +$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
>  	$(call cmd,force_checksrc)
>  	$(call if_changed_rule,cc_o_c)
>  
>  # Single-part modules are special since we need to mark them in $(MODVERDIR)
>  
> -$(single-used-m): $(obj)/%.o: $(src)/%.c FORCE
> +$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
>  	$(call cmd,force_checksrc)
>  	$(call if_changed_rule,cc_o_c)
>  	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)
> 
> Michal

Re: [RFC][PATCH 2/2] ftrace/kbuild: Add recordmcount files to force full build

[Steven Rostedt]

On Tue, 2011-05-17 at 09:47 -0400, Steven Rostedt wrote:
> On Tue, 2011-05-17 at 15:36 +0200, Michal Marek wrote:
> 
> > We could add recordmcount.c as a dependency in scripts/Makefile.build
> > directly, without playing with fixdep, like this (untested):
> 
> I actually tried this first, and it didn't seem to work. I'll go ahead
> and give it another try (maybe I did something wrong). But I do prefer
> this over my fixdep change.
> 
> Thanks, I'll let you know the outcome.

Awesome! This worked. I wonder what screw up I did to keep mine from
working? Anyway, I made a single change to your patch:

-recordmcount_source := $(srctree)/scripts/recordmcount.c
+recordmcount_source := $(srctree)/scripts/recordmcount.c \
+                   $(srctree)/scripts/recordmcount.h


Could you send me your Signed-off-by, and I'll add this to the beginning
of my patch queue.

Thanks!

-- Steve

Re: [RFC][PATCH 2/2] ftrace/kbuild: Add recordmcount files to force full build

[Steven Rostedt]

On Tue, 2011-05-17 at 10:20 -0400, Steven Rostedt wrote:

> Awesome! This worked. I wonder what screw up I did to keep mine from
> working? 

Bah! I forgot to add the $() around the variable that I used.

/me is embarrassed.

-- Steve

Re: [RFC][PATCH 2/2] ftrace/kbuild: Add recordmcount files to force full build

[Michal Marek]

On 17.5.2011 16:20, Steven Rostedt wrote:
> On Tue, 2011-05-17 at 09:47 -0400, Steven Rostedt wrote:
>> On Tue, 2011-05-17 at 15:36 +0200, Michal Marek wrote:
>>
>>> We could add recordmcount.c as a dependency in scripts/Makefile.build
>>> directly, without playing with fixdep, like this (untested):
>>
>> I actually tried this first, and it didn't seem to work. I'll go ahead
>> and give it another try (maybe I did something wrong). But I do prefer
>> this over my fixdep change.
>>
>> Thanks, I'll let you know the outcome.
>
> Awesome! This worked.

Great.


> I wonder what screw up I did to keep mine from
> working? Anyway, I made a single change to your patch:
>
> -recordmcount_source := $(srctree)/scripts/recordmcount.c
> +recordmcount_source := $(srctree)/scripts/recordmcount.c \
> +                   $(srctree)/scripts/recordmcount.h
>
>
> Could you send me your Signed-off-by, and I'll add this to the beginning
> of my patch queue.

Signed-off-by: Michal Marek <mmarek@suse.cz>

Michal

[tip:perf/core] ftrace/kbuild: Add recordmcount files to force full build

[tip-bot for Michal Marek]

Commit-ID:  d6971822c288ce5547190c6f812cc978d6520777
Gitweb:     http://git.kernel.org/tip/d6971822c288ce5547190c6f812cc978d6520777
Author:     Michal Marek <mmarek@suse.cz>
AuthorDate: Tue, 17 May 2011 15:36:46 +0200
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 19 May 2011 07:58:28 -0400

ftrace/kbuild: Add recordmcount files to force full build

Modifications to recordmcount must be performed on all object
files to stay consistent with what the kernel code may expect.
Add the recordmcount files to the main dependencies to make sure
any change to them causes a full recompile.

Signed-off-by: Michal Marek <mmarek@suse.cz>
Link: http://lkml.kernel.org/r/20110517133646.GP13293@sepie.suse.cz
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/Makefile.build |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index fdca952..6165622 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -255,6 +255,8 @@ sub_cmd_record_mcount =					\
 	if [ $(@) != "scripts/mod/empty.o" ]; then	\
 		$(objtree)/scripts/recordmcount $(RECORDMCOUNT_FLAGS) "$(@)";	\
 	fi;
+recordmcount_source := $(srctree)/scripts/recordmcount.c \
+		    $(srctree)/scripts/recordmcount.h
 else
 sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
 	"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
@@ -262,6 +264,7 @@ sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH
 	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CFLAGS)" \
 	"$(LD)" "$(NM)" "$(RM)" "$(MV)" \
 	"$(if $(part-of-module),1,0)" "$(@)";
+recordmcount_source := $(srctree)/scripts/recordmcount.pl
 endif
 cmd_record_mcount = 						\
 	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\
@@ -282,13 +285,13 @@ define rule_cc_o_c
 endef
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)

[tip:perf/core] ftrace/kbuild: Add recordmcount files to force full build

[tip-bot for Michal Marek]

Commit-ID:  44e6a2069122dbec29d25b8ced3f59f5626a97d7
Gitweb:     http://git.kernel.org/tip/44e6a2069122dbec29d25b8ced3f59f5626a97d7
Author:     Michal Marek <mmarek@suse.cz>
AuthorDate: Tue, 17 May 2011 15:36:46 +0200
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Tue, 17 May 2011 10:41:08 -0400

ftrace/kbuild: Add recordmcount files to force full build

Modifications to recordmcount must be performed on all object
files to stay consistent with what the kernel code may expect.
Add the recordmcount files to the main dependencies to make sure
any change to them causes a full recompile.

Signed-off-by: Michal Marek <mmarek@suse.cz>
Link: http://lkml.kernel.org/r/20110517133646.GP13293@sepie.suse.cz
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/Makefile.build |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d5f925a..7d3f903 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -252,6 +252,8 @@ sub_cmd_record_mcount =					\
 	if [ $(@) != "scripts/mod/empty.o" ]; then	\
 		$(objtree)/scripts/recordmcount "$(@)";	\
 	fi;
+recordmcount_source := $(srctree)/scripts/recordmcount.c \
+		    $(srctree)/scripts/recordmcount.h
 else
 sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
 	"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
@@ -259,6 +261,7 @@ sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH
 	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CFLAGS)" \
 	"$(LD)" "$(NM)" "$(RM)" "$(MV)" \
 	"$(if $(part-of-module),1,0)" "$(@)";
+recordmcount_source := $(srctree)/scripts/recordmcount.pl
 endif
 cmd_record_mcount = 						\
 	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\
@@ -279,13 +282,13 @@ define rule_cc_o_c
 endef
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)