Better way to force a rebuild

Better way to force a rebuild

[Steven Rostedt]

Hi,

In include/linux/kernel.h I currently have the following lines at the 
bottom of the file:

/* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
#ifdef CONFIG_FTRACE_MCOUNT_RECORD
# define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
#endif


This is only there to force a rebuild of all files when 
CONFIG_FTRACE_MCOUNT_RECORD is modified. Since the modification of that 
config causes the build to act different, we need to rebuild all C 
objects. I added the #ifdef in kernel.h as a hack to force the rebuild by 
the config dependencies used in kbuild.

My question is, is there a better way to force a full rebuild on a 
modification of a config?

Thanks,

-- Steve

Re: Better way to force a rebuild

[Sam Ravnborg]

On Wed, Dec 10, 2008 at 02:48:35PM -0500, Steven Rostedt wrote:
> 
> Hi,
> 
> In include/linux/kernel.h I currently have the following lines at the 
> bottom of the file:
> 
> /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
> #ifdef CONFIG_FTRACE_MCOUNT_RECORD
> # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
> #endif
> 
> 
> This is only there to force a rebuild of all files when 
> CONFIG_FTRACE_MCOUNT_RECORD is modified. Since the modification of that 
> config causes the build to act different, we need to rebuild all C 
> objects. I added the #ifdef in kernel.h as a hack to force the rebuild by 
> the config dependencies used in kbuild.
> 
> My question is, is there a better way to force a full rebuild on a 
> modification of a config?

Why is that hack required in the first place?
We rebuild if:
- target file is missing (not the case here)
- any prerequisite files are newer than target (not the case here)
- any CONFIG_* options used by any prerequisite has changed (not the case here)
  [It is due to your hack]
- gcc changed (not the case here)
- options are added/deleted to gcc (this should be the case for relevant files)


Looking a bit deeper into this.
I see that we have in Makefile.build:

define rule_cc_o_c
        $(call echo-cmd,checksrc) $(cmd_checksrc)                         \
        $(call echo-cmd,cc_o_c) $(cmd_cc_o_c);                            \
        $(cmd_modversions)                                                \
        $(cmd_record_mcount)   

So you want everyting to be rebuild if the command above changes.
That should be doable - and it is indeed a bug that we do not do so.
The integration of record_mcount needs to be improved.


	Sam

Re: Better way to force a rebuild

[Andrew Morton]

On Wed, 10 Dec 2008 21:02:42 +0100
Sam Ravnborg <sam@ravnborg.org> wrote:

> On Wed, Dec 10, 2008 at 02:48:35PM -0500, Steven Rostedt wrote:
> > 
> > Hi,
> > 
> > In include/linux/kernel.h I currently have the following lines at the 
> > bottom of the file:
> > 
> > /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
> > #ifdef CONFIG_FTRACE_MCOUNT_RECORD
> > # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
> > #endif
> > 
> > 
> > This is only there to force a rebuild of all files when 
> > CONFIG_FTRACE_MCOUNT_RECORD is modified. Since the modification of that 
> > config causes the build to act different, we need to rebuild all C 
> > objects. I added the #ifdef in kernel.h as a hack to force the rebuild by 
> > the config dependencies used in kbuild.
> > 
> > My question is, is there a better way to force a full rebuild on a 
> > modification of a config?
> 
> Why is that hack required in the first place?
> We rebuild if:
> - target file is missing (not the case here)
> - any prerequisite files are newer than target (not the case here)
> - any CONFIG_* options used by any prerequisite has changed (not the case here)
>   [It is due to your hack]
> - gcc changed (not the case here)
> - options are added/deleted to gcc (this should be the case for relevant files)
> 
> 
> Looking a bit deeper into this.
> I see that we have in Makefile.build:
> 
> define rule_cc_o_c
>         $(call echo-cmd,checksrc) $(cmd_checksrc)                         \
>         $(call echo-cmd,cc_o_c) $(cmd_cc_o_c);                            \
>         $(cmd_modversions)                                                \
>         $(cmd_record_mcount)   
> 
> So you want everyting to be rebuild if the command above changes.
> That should be doable - and it is indeed a bug that we do not do so.
> The integration of record_mcount needs to be improved.

Yes, changing the state of CONFIG_CC_OPTIMIZE_FOR_SIZE does exactly
what Steven wants.

But damn me it's hard to work out where and how this is implemented :(

Re: Better way to force a rebuild

[Sam Ravnborg]

On Wed, Dec 10, 2008 at 12:10:48PM -0800, Andrew Morton wrote:
> On Wed, 10 Dec 2008 21:02:42 +0100
> Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Wed, Dec 10, 2008 at 02:48:35PM -0500, Steven Rostedt wrote:
> > > 
> > > Hi,
> > > 
> > > In include/linux/kernel.h I currently have the following lines at the 
> > > bottom of the file:
> > > 
> > > /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
> > > #ifdef CONFIG_FTRACE_MCOUNT_RECORD
> > > # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
> > > #endif
> > > 
> > > 
> > > This is only there to force a rebuild of all files when 
> > > CONFIG_FTRACE_MCOUNT_RECORD is modified. Since the modification of that 
> > > config causes the build to act different, we need to rebuild all C 
> > > objects. I added the #ifdef in kernel.h as a hack to force the rebuild by 
> > > the config dependencies used in kbuild.
> > > 
> > > My question is, is there a better way to force a full rebuild on a 
> > > modification of a config?
> > 
> > Why is that hack required in the first place?
> > We rebuild if:
> > 1- target file is missing (not the case here)
> > 2- any prerequisite files are newer than target (not the case here)
> > 3- any CONFIG_* options used by any prerequisite has changed (not the case here)
> >   [It is due to your hack]
> > 4- gcc changed (not the case here)
> > 5- options are added/deleted to gcc (this should be the case for relevant files)
> > 
> 
> But damn me it's hard to work out where and how this is implemented :(

The magic is to be found in Kbuild.include:

# Usage: $(call if_changed_rule,foo)
# Will check if $(cmd_foo) or any of the prerequisites changed,
# and if so will execute $(rule_foo).
if_changed_rule = $(if $(strip $(any-prereq) $(arg-check) ),                 \
        @set -e;                                                             \
        $(rule_$(1)))


This parts takes care of:
$(any-prereq) takes care of 1 and 2 in the above list
$(arg-check) takes care of 4 and 5 in the above list.

And record_mcount is part of the following definition:
define rule_cc_o_c
        $(call echo-cmd,checksrc) $(cmd_checksrc)                         \
        $(call echo-cmd,cc_o_c) $(cmd_cc_o_c);                            \
        $(cmd_modversions)                                                \
        $(cmd_record_mcount)                                              \
        scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
                                                      $(dot-target).tmp;  \
        rm -f $(depfile);                                                 \
        mv -f $(dot-target).tmp $(dot-target).cmd
endef

Here we use fixdep to create a set of prerequisites - one for each
CONFIG_ symbol that is found in the prerequisites. And fixdep reads the
list of prerequisites in the $(dep-file) that is created by gcc in the
cc_o_c command:

cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $<


So after reading this you know almost as much about kbuild as I do.

I will fix the record_mcount stuff somehow.
But not tonight. Busy with work-work.

	Sam

Re: Better way to force a rebuild

[Steven Rostedt]

On Wed, 10 Dec 2008, Sam Ravnborg wrote:
> 
> I will fix the record_mcount stuff somehow.
> But not tonight. Busy with work-work.

Sam, Thanks!

No hurry, I was doing some clean ups when I came across that code, and 
said to myself "what an ugly hack!", and I wanted it fixed. The current 
hack works just fine, but it is an eye sore in the kernel.

I just wanted others to know that it needed to be cleaned up, and figured 
a post would help get it done.

Thanks,

-- Steve

Re: Better way to force a rebuild

[Steven Rostedt]

On Wed, 10 Dec 2008, Sam Ravnborg wrote:

> On Wed, Dec 10, 2008 at 02:48:35PM -0500, Steven Rostedt wrote:
> 
> Looking a bit deeper into this.
> I see that we have in Makefile.build:
> 
> define rule_cc_o_c
>         $(call echo-cmd,checksrc) $(cmd_checksrc)                         \
>         $(call echo-cmd,cc_o_c) $(cmd_cc_o_c);                            \
>         $(cmd_modversions)                                                \
>         $(cmd_record_mcount)   
> 
> So you want everyting to be rebuild if the command above changes.
> That should be doable - and it is indeed a bug that we do not do so.
> The integration of record_mcount needs to be improved.

Yes, that is exactly what I want. Now the next step is to implement this. 
I'm not that strong on the kbuild system to know how.

Thanks,

-- Steve

Re: Better way to force a rebuild

[Steven Rostedt]

> 
> 	Sam
> 

BTW, Sam, are you the new (or current) maintainer of kbuild/kconfig? If 
so, then we need to update MAINTAINERS.  I just notice that you reply to 
my kbuild questions more than anybody else (except for Andrew, but he 
doesn't count ;-)

-- Steve

Re: Better way to force a rebuild

[Steven Rostedt]

On Wed, 10 Dec 2008, Steven Rostedt wrote:

> > 
> > 	Sam
> > 
> 
> BTW, Sam, are you the new (or current) maintainer of kbuild/kconfig? If 
> so, then we need to update MAINTAINERS.  I just notice that you reply to 
> my kbuild questions more than anybody else (except for Andrew, but he 
> doesn't count ;-)

After writing this, I see...


KCONFIG
P:      Roman Zippel
M:      zippel@linux-m68k.org
L:      linux-kbuild@vger.kernel.org
S:      Maintained

[...]

KERNEL BUILD (kbuild: Makefile, scripts/Makefile.*)
P:      Sam Ravnborg
M:      sam@ravnborg.org
T:      git kernel.org:/pub/scm/linux/kernel/git/sam/kbuild-next.git
T:      git kernel.org:/pub/scm/linux/kernel/git/sam/kbuild-fixes.git
L:      linux-kbuild@vger.kernel.org
S:      Maintained


Bah, I need to make my vim search non-case-sensitive. My search for KBUILD 
came up with nothing, and then I searched for KCONFIG.

-- Steve

Re: Better way to force a rebuild

[Sam Ravnborg]

On Wed, Dec 10, 2008 at 03:26:14PM -0500, Steven Rostedt wrote:
> > 
> > 	Sam
> > 
> 
> BTW, Sam, are you the new (or current) maintainer of kbuild/kconfig? If 
> so, then we need to update MAINTAINERS.  I just notice that you reply to 
> my kbuild questions more than anybody else (except for Andrew, but he 
> doesn't count ;-)

From MAINTAINERS:
KERNEL BUILD (kbuild: Makefile, scripts/Makefile.*)
P:      Sam Ravnborg
M:      sam@ravnborg.org
T:      git kernel.org:/pub/scm/linux/kernel/git/sam/kbuild-next.git
T:      git kernel.org:/pub/scm/linux/kernel/git/sam/kbuild-fixes.git
L:      linux-kbuild@vger.kernel.org
S:      Maintained


Roman Zippel is the maintainer of kconfig and we pass the
patches via me.
Again from MAINTAINERS:
KCONFIG
P:      Roman Zippel
M:      zippel@linux-m68k.org
L:      linux-kbuild@vger.kernel.org
S:      Maintained

	Sam

Re: Better way to force a rebuild

[Sam Ravnborg]

On Wed, Dec 10, 2008 at 09:02:42PM +0100, Sam Ravnborg wrote:
> On Wed, Dec 10, 2008 at 02:48:35PM -0500, Steven Rostedt wrote:
> > 
> > Hi,
> > 
> > In include/linux/kernel.h I currently have the following lines at the 
> > bottom of the file:
> > 
> > /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
> > #ifdef CONFIG_FTRACE_MCOUNT_RECORD
> > # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
> > #endif
> > 
> > 
> > This is only there to force a rebuild of all files when 
> > CONFIG_FTRACE_MCOUNT_RECORD is modified. Since the modification of that 
> > config causes the build to act different, we need to rebuild all C 
> > objects. I added the #ifdef in kernel.h as a hack to force the rebuild by 
> > the config dependencies used in kbuild.
> > 
> > My question is, is there a better way to force a full rebuild on a 
> > modification of a config?
> 
> Why is that hack required in the first place?
> We rebuild if:
> - target file is missing (not the case here)
> - any prerequisite files are newer than target (not the case here)
> - any CONFIG_* options used by any prerequisite has changed (not the case here)
>   [It is due to your hack]
> - gcc changed (not the case here)
> - options are added/deleted to gcc (this should be the case for relevant files)
> 
> 
> Looking a bit deeper into this.
> I see that we have in Makefile.build:
> 
> define rule_cc_o_c
>         $(call echo-cmd,checksrc) $(cmd_checksrc)                         \
>         $(call echo-cmd,cc_o_c) $(cmd_cc_o_c);                            \
>         $(cmd_modversions)                                                \
>         $(cmd_record_mcount)   
> 
> So you want everyting to be rebuild if the command above changes.
> That should be doable - and it is indeed a bug that we do not do so.
> The integration of record_mcount needs to be improved.

I made the following quick hack.
I think it should do the trick.

	Sam

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 468fbc9..8b5bf43 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -215,7 +215,8 @@ define rule_cc_o_c
 	$(call echo-cmd,cc_o_c) $(cmd_cc_o_c);				  \
 	$(cmd_modversions)						  \
 	$(cmd_record_mcount)						  \
-	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
+	scripts/basic/fixdep $(depfile) $@                                \
+	'$(call make-cmd,cc_o_c) $(call make-cmd,record_mcount)' >        \
 	                                              $(dot-target).tmp;  \
 	rm -f $(depfile);						  \
 	mv -f $(dot-target).tmp $(dot-target).cmd

Re: Better way to force a rebuild

[Sam Ravnborg]

On Wed, Dec 10, 2008 at 10:28:21PM +0100, Sam Ravnborg wrote:
> 
> I made the following quick hack.
> I think it should do the trick.
> 
> 	Sam
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 468fbc9..8b5bf43 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -215,7 +215,8 @@ define rule_cc_o_c
>  	$(call echo-cmd,cc_o_c) $(cmd_cc_o_c);				  \
>  	$(cmd_modversions)						  \
>  	$(cmd_record_mcount)						  \
> -	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
> +	scripts/basic/fixdep $(depfile) $@                                \
> +	'$(call make-cmd,cc_o_c) $(call make-cmd,record_mcount)' >        \
>  	                                              $(dot-target).tmp;  \
>  	rm -f $(depfile);						  \
>  	mv -f $(dot-target).tmp $(dot-target).cmd

Hi Steven.

Did you try the above?
If it works I will add it to kbuild-next.git

	Sam

Re: Better way to force a rebuild

[Steven Rostedt]

On Sun, 14 Dec 2008, Sam Ravnborg wrote:

> On Wed, Dec 10, 2008 at 10:28:21PM +0100, Sam Ravnborg wrote:
> > 
> > I made the following quick hack.
> > I think it should do the trick.
> > 
> > 	Sam
> > 
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 468fbc9..8b5bf43 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -215,7 +215,8 @@ define rule_cc_o_c
> >  	$(call echo-cmd,cc_o_c) $(cmd_cc_o_c);				  \
> >  	$(cmd_modversions)						  \
> >  	$(cmd_record_mcount)						  \
> > -	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
> > +	scripts/basic/fixdep $(depfile) $@                                \
> > +	'$(call make-cmd,cc_o_c) $(call make-cmd,record_mcount)' >        \
> >  	                                              $(dot-target).tmp;  \
> >  	rm -f $(depfile);						  \
> >  	mv -f $(dot-target).tmp $(dot-target).cmd
> 
> Hi Steven.
> 
> Did you try the above?
> If it works I will add it to kbuild-next.git

I started to but got distracted :-/   I'll retry it on Monday.

-- Steve

Re: Better way to force a rebuild

[Steven Rostedt]

On Sun, 14 Dec 2008, Steven Rostedt wrote:
> 
> On Sun, 14 Dec 2008, Sam Ravnborg wrote:
> 
> > On Wed, Dec 10, 2008 at 10:28:21PM +0100, Sam Ravnborg wrote:
> > > 
> > > I made the following quick hack.
> > > I think it should do the trick.
> > > 
> > > 	Sam
> > > 
> > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > index 468fbc9..8b5bf43 100644
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -215,7 +215,8 @@ define rule_cc_o_c
> > >  	$(call echo-cmd,cc_o_c) $(cmd_cc_o_c);				  \
> > >  	$(cmd_modversions)						  \
> > >  	$(cmd_record_mcount)						  \
> > > -	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
> > > +	scripts/basic/fixdep $(depfile) $@                                \
> > > +	'$(call make-cmd,cc_o_c) $(call make-cmd,record_mcount)' >        \
> > >  	                                              $(dot-target).tmp;  \
> > >  	rm -f $(depfile);						  \
> > >  	mv -f $(dot-target).tmp $(dot-target).cmd
> > 
> > Hi Steven.
> > 
> > Did you try the above?
> > If it works I will add it to kbuild-next.git
> 
> I started to but got distracted :-/   I'll retry it on Monday.

Hi Sam,

Sorry for taking so long to get back to you, but my hard drive for my 
laptop is on the fritz, and I'm busy making sure everything is backed up.

I tried this patch and it does not seem to do what I want :-(

You can test it too, just compile the kernel with CONFIG_FUNCTION_TRACER
and CONFIG_DYNAMIC_FTRACE enabled, and then compile again with just 
disabling CONFIG_DYNAMIC_FTRACE (keep CONFIG_FUNCTION_TRACER enabled). If 
the entire kernel does not compile again, then it did not work.

-- Steve

Oh, you need to add this patch first:

---
 include/linux/kernel.h |    5 -----
 1 file changed, 5 deletions(-)

Index: linux-trace.git/include/linux/kernel.h
===================================================================
--- linux-trace.git.orig/include/linux/kernel.h	2008-11-20 14:36:55.000000000 -0500
+++ linux-trace.git/include/linux/kernel.h	2008-12-10 16:54:16.000000000 -0500
@@ -535,9 +535,4 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
-/* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
-#ifdef CONFIG_FTRACE_MCOUNT_RECORD
-# define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
-#endif
-
 #endif