"build-id" changes break sparc64

"build-id" changes break sparc64

[David Miller]

(This is in reference to commit 18991197b4b588255ccabf472ebc84db7b66a19c)

When actually used, the build-id linker option causes problems for the
sparc64 kernel in two ways:

1) When building modules we get tons of warnings from the linker
   such as:

  LD [M]  drivers/scsi/sr_mod.o
ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.

2) The resulting kernel image will not boot.  The SILO boot loader on
   sparc aborts if there are multiple PT_LOAD segments in the ELF
   program header of the kernel image.  The boot loader can only
   handle allocating memory for and setting up mappings via the
   firmware for one one linear PT_LOAD segment.

   build-id puts a second PT_LOAD into the kernel image (albeit a tiny
   one) and as a result it cannot be booted

This is all with the current binutils in Ubuntu gutsy which
is version 2.17.50.20070718

Whilst #1 could be some kind of binutils bug and the warning
is harmless, #2 is an outright show-stopper for sparc32 and
sparc64 and to be honest I would not be surprised if the
boot loader on some other platforms have this limitation too.

My suggestion would be to add this linker option in the various
arch/${FOO}/Makefile files for platforms and situations where it
is known to work properly.

Roland, if you agree could you submit a fix like that?

Alternatively, if you think build-id is so valuable, impress me with
your binutils ninja skills and show me how to perhaps do some objcopy
tricks wherein I could dike out the PT_LOAD segment build-id generates
for the "image" target et al.  in the arch/sparc{,64}/boot/Makefile's
rules.

Thanks a lot.

Re: "build-id" changes break sparc64

[Roland McGrath]

> (This is in reference to commit 18991197b4b588255ccabf472ebc84db7b66a19c)
> 
> When actually used, the build-id linker option causes problems for the
> sparc64 kernel in two ways:
> 
> 1) When building modules we get tons of warnings from the linker
>    such as:
> 
>   LD [M]  drivers/scsi/sr_mod.o
> ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.

This is probably a bug in the new ld code (which is still quite unreleased,
even if bleeding-edge distro builds are using it already).  Please work
with me (off the kernel list) to figure that out.

>    build-id puts a second PT_LOAD into the kernel image (albeit a tiny
>    one) and as a result it cannot be booted

This probably just requires a linker script tweak to use the NOTES macro.
Maybe:

--- a/arch/sparc64/kernel/vmlinux.lds.S
+++ b/arch/sparc64/kernel/vmlinux.lds.S
@@ -19,6 +19,7 @@ SECTIONS
     SCHED_TEXT
     LOCK_TEXT
     KPROBES_TEXT
+    NOTES
     *(.gnu.warning)
   } =0
   _etext = .;

I did not change every platform's script where I wasn't sure if a change
was required or what it needed to be.  It may be that every platform that
doesn't use the NOTES macro in its vmlinux.lds.S really needs to.  (Place
it anywhere you like that is appropriate for a few dozen read-only bytes
that will rarely be read.)

> Whilst #1 could be some kind of binutils bug and the warning
> is harmless, #2 is an outright show-stopper for sparc32 and
> sparc64 and to be honest I would not be surprised if the
> boot loader on some other platforms have this limitation too.

There's no expectation of changing anything about the layout.
Just a linker script tweak might be required so the tiny new
section falls somewhere proper in the existing layout.

> Alternatively, if you think build-id is so valuable, impress me with
> your binutils ninja skills and show me how to perhaps do some objcopy
> tricks wherein I could dike out the PT_LOAD segment build-id generates
> for the "image" target et al.  in the arch/sparc{,64}/boot/Makefile's
> rules.

I have indeed performed some odd feats in this area in the past, but in
this case there should be not any need for anything of the sort.

I would prefer not to let the feature lag while every platform gets around
to adding a Makefile line.  It really should be universal and simple to
deal with.  The ld feature is still so bleeding-edge that there should be
ample time to iron out kinks before anyone could be bit by it who wasn't
really asking for it.


Thanks,
Roland

Re: "build-id" changes break sparc64

[David Miller]

From: Roland McGrath <roland@redhat.com>
Date: Sun, 22 Jul 2007 01:59:40 -0700 (PDT)

> Maybe:
> 
> --- a/arch/sparc64/kernel/vmlinux.lds.S
> +++ b/arch/sparc64/kernel/vmlinux.lds.S
> @@ -19,6 +19,7 @@ SECTIONS
>      SCHED_TEXT
>      LOCK_TEXT
>      KPROBES_TEXT
> +    NOTES
>      *(.gnu.warning)
>    } =0
>    _etext = .;

This generates a syntax error.

The following fix works and is what I'll merge in to fix this
problem, thanks!

diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
index 4758388..15109c1 100644
--- a/arch/sparc/kernel/vmlinux.lds.S
+++ b/arch/sparc/kernel/vmlinux.lds.S
@@ -35,6 +35,8 @@ SECTIONS
   __ex_table : { *(__ex_table) }
   __stop___ex_table = .;
 
+  NOTES
+
   . = ALIGN(4096);
   __init_begin = .;
   _sinittext = .;
diff --git a/arch/sparc64/kernel/vmlinux.lds.S b/arch/sparc64/kernel/vmlinux.lds.S
index 4818617..b982fa3 100644
--- a/arch/sparc64/kernel/vmlinux.lds.S
+++ b/arch/sparc64/kernel/vmlinux.lds.S
@@ -45,6 +45,8 @@ SECTIONS
   __ex_table : { *(__ex_table) }
   __stop___ex_table = .;
 
+  NOTES
+
   . = ALIGN(PAGE_SIZE);
   __init_begin = .;
   .init.text : { 

Re: "build-id" changes break sparc64

[Roland McGrath]

It turns out the problem here is that some .o files wind up with their own
.note.gnu.build-id sections.  I got the makefile magic wrong, thinking that
LDFLAGS_MODULE was a variable specifically for .ko links.  It's also used
in cmd_link_multi-m.  So the problem David and Adrian saw is not actually
machine-dependent at all, nor is it an ld bug as I had guessed, but depends
on the configuration details that determine when cmd_link_multi-m gets used.

I'll post a makefile fix shortly.


Thanks,
Roland

Re: "build-id" changes break sparc64

[Paul Mackerras]

Roland McGrath writes:

> It turns out the problem here is that some .o files wind up with their own
> .note.gnu.build-id sections.  I got the makefile magic wrong, thinking that
> LDFLAGS_MODULE was a variable specifically for .ko links.  It's also used
> in cmd_link_multi-m.

Alan Modra (binutils hacker) has said to me in the past that using
ld -r to combine the objects in each directory is bad; he would much
rather that we gave all the individual objects to the final link,
since that enables ld to do better optimizations on some targets.  We
could actually do that quite easily by making the built-in.o files be
linker scripts listing the individual objects rather than creating
them with ld -r.

Paul.

Re: "build-id" changes break sparc64

[Sam Ravnborg]

On Mon, Jul 23, 2007 at 04:13:21PM +1000, Paul Mackerras wrote:
> Roland McGrath writes:
> 
> > It turns out the problem here is that some .o files wind up with their own
> > .note.gnu.build-id sections.  I got the makefile magic wrong, thinking that
> > LDFLAGS_MODULE was a variable specifically for .ko links.  It's also used
> > in cmd_link_multi-m.
> 
> Alan Modra (binutils hacker) has said to me in the past that using
> ld -r to combine the objects in each directory is bad; he would much
> rather that we gave all the individual objects to the final link,
> since that enables ld to do better optimizations on some targets.  We
> could actually do that quite easily by making the built-in.o files be
> linker scripts listing the individual objects rather than creating
> them with ld -r.

Should be doable without to much pain.
Alan can you please share with us exactly why this is better and what we may
run into of problems doing so.
A sample script would be nice too....

	Sam

Re: "build-id" changes break sparc64

[Roland McGrath]

> Should be doable without to much pain.
> Alan can you please share with us exactly why this is better and what we may
> run into of problems doing so.
> A sample script would be nice too....

This about does it.  Polish left as an exercise to the reader.
ld does "interesting" things if the linker scripts are inside --start-group.

diff --git a/Makefile b/Makefile
index cd47845..0000000 100644  
--- a/Makefile
+++ b/Makefile
@@ -613,12 +613,15 @@ vmlinux-all  := $(vmlinux-init) $(vmlinu
 vmlinux-lds  := arch/$(ARCH)/kernel/vmlinux.lds
 export KBUILD_VMLINUX_OBJS := $(vmlinux-all)
 
+vmlinux-link-main = $(filter-out %.a,$(vmlinux-main)) \
+		    --start-group $(filter %.a,$(vmlinux-main)) --end-group
+
 # Rule to link vmlinux - also used during CONFIG_KALLSYMS
 # May be overridden by arch/$(ARCH)/Makefile
 quiet_cmd_vmlinux__ ?= LD      $@
       cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) -o $@ \
       -T $(vmlinux-lds) $(vmlinux-init)                          \
-      --start-group $(vmlinux-main) --end-group                  \
+      $(vmlinux-link-main) \
       $(filter-out $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o FORCE ,$^)
 
 # Generate new vmlinux version
@@ -747,7 +750,7 @@ endif # ifdef CONFIG_KALLSYMS
 # relevant sections renamed as per the linker script.
 quiet_cmd_vmlinux-modpost = LD      $@
       cmd_vmlinux-modpost = $(LD) $(LDFLAGS) -r -o $@                          \
-	 $(vmlinux-init) --start-group $(vmlinux-main) --end-group             \
+	 $(vmlinux-init) $(vmlinux-link-main) \
 	 $(filter-out $(vmlinux-init) $(vmlinux-main) $(vmlinux-lds) FORCE ,$^)
 define rule_vmlinux-modpost
 	:
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3f7b451..0000000 100644  
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -274,11 +274,11 @@ $(sort $(subdir-obj-y)): $(subdir-ym) ;
 # Rule to compile a set of .o files into one .o file
 #
 ifdef builtin-target
-quiet_cmd_link_o_target = LD      $@
+quiet_cmd_link_o_target = LDS      $@
 # If the list of objects to link is empty, just create an empty built-in.o
-cmd_link_o_target = $(if $(strip $(obj-y)),\
-		      $(LD) $(ld_flags) -r -o $@ $(filter $(obj-y), $^),\
-		      rm -f $@; $(AR) rcs $@)
+cmd_link_o_target = echo > $@.new $(if $(strip $(obj-y)),\
+		    	   	       'INPUT($(filter $(obj-y), $^))') && \
+ 		    mv -f $@.new $@
 
 $(builtin-target): $(obj-y) FORCE
 	$(call if_changed,link_o_target)
@@ -311,11 +311,11 @@ $(filter $(addprefix $(obj)/,         \
 $($(subst $(obj)/,,$(@:.o=-objs)))    \
 $($(subst $(obj)/,,$(@:.o=-y)))), $^)
  
-quiet_cmd_link_multi-y = LD      $@
-cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
+quiet_cmd_link_multi-y = LDS     $@
+cmd_link_multi-y = echo > $@.new 'INPUT($(link_multi_deps))' && mv -f $@.new $@
 
 quiet_cmd_link_multi-m = LD [M]  $@
-cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
+cmd_link_multi-m = $(cmd_link_multi-y)
 
 # We would rather have a list of rules like
 # 	foo.o: $(foo-objs)

Re: "build-id" changes break sparc64

[Sam Ravnborg]

On Mon, Jul 23, 2007 at 12:26:13AM -0700, Roland McGrath wrote:
> > Should be doable without to much pain.
> > Alan can you please share with us exactly why this is better and what we may
> > run into of problems doing so.
> > A sample script would be nice too....
> 
> This about does it.  Polish left as an exercise to the reader.
> ld does "interesting" things if the linker scripts are inside --start-group.

Thanks - will take a look during the week.
I will most likely rename built-in.o to something else and reflect this
in Makefile.build. Just to make it clear this is now a linker script file.

But I would still like to hear from Alan what the benefits are.
I foresee that with this change it will take longer to build a kernel
after a single change but that a full kernel build is shorter.
And thats the wrong way to optimze...

	Sam
> 
> diff --git a/Makefile b/Makefile
> index cd47845..0000000 100644  
> --- a/Makefile
> +++ b/Makefile
> @@ -613,12 +613,15 @@ vmlinux-all  := $(vmlinux-init) $(vmlinu
>  vmlinux-lds  := arch/$(ARCH)/kernel/vmlinux.lds
>  export KBUILD_VMLINUX_OBJS := $(vmlinux-all)
>  
> +vmlinux-link-main = $(filter-out %.a,$(vmlinux-main)) \
> +		    --start-group $(filter %.a,$(vmlinux-main)) --end-group
> +
>  # Rule to link vmlinux - also used during CONFIG_KALLSYMS
>  # May be overridden by arch/$(ARCH)/Makefile
>  quiet_cmd_vmlinux__ ?= LD      $@
>        cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) -o $@ \
>        -T $(vmlinux-lds) $(vmlinux-init)                          \
> -      --start-group $(vmlinux-main) --end-group                  \
> +      $(vmlinux-link-main) \
>        $(filter-out $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o FORCE ,$^)
>  
>  # Generate new vmlinux version
> @@ -747,7 +750,7 @@ endif # ifdef CONFIG_KALLSYMS
>  # relevant sections renamed as per the linker script.
>  quiet_cmd_vmlinux-modpost = LD      $@
>        cmd_vmlinux-modpost = $(LD) $(LDFLAGS) -r -o $@                          \
> -	 $(vmlinux-init) --start-group $(vmlinux-main) --end-group             \
> +	 $(vmlinux-init) $(vmlinux-link-main) \
>  	 $(filter-out $(vmlinux-init) $(vmlinux-main) $(vmlinux-lds) FORCE ,$^)
>  define rule_vmlinux-modpost
>  	:
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3f7b451..0000000 100644  
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -274,11 +274,11 @@ $(sort $(subdir-obj-y)): $(subdir-ym) ;
>  # Rule to compile a set of .o files into one .o file
>  #
>  ifdef builtin-target
> -quiet_cmd_link_o_target = LD      $@
> +quiet_cmd_link_o_target = LDS      $@
>  # If the list of objects to link is empty, just create an empty built-in.o
> -cmd_link_o_target = $(if $(strip $(obj-y)),\
> -		      $(LD) $(ld_flags) -r -o $@ $(filter $(obj-y), $^),\
> -		      rm -f $@; $(AR) rcs $@)
> +cmd_link_o_target = echo > $@.new $(if $(strip $(obj-y)),\
> +		    	   	       'INPUT($(filter $(obj-y), $^))') && \
> + 		    mv -f $@.new $@
>  
>  $(builtin-target): $(obj-y) FORCE
>  	$(call if_changed,link_o_target)
> @@ -311,11 +311,11 @@ $(filter $(addprefix $(obj)/,         \
>  $($(subst $(obj)/,,$(@:.o=-objs)))    \
>  $($(subst $(obj)/,,$(@:.o=-y)))), $^)
>   
> -quiet_cmd_link_multi-y = LD      $@
> -cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
> +quiet_cmd_link_multi-y = LDS     $@
> +cmd_link_multi-y = echo > $@.new 'INPUT($(link_multi_deps))' && mv -f $@.new $@
>  
>  quiet_cmd_link_multi-m = LD [M]  $@
> -cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
> +cmd_link_multi-m = $(cmd_link_multi-y)
This signel change looks wrong. We do not want to generate a linker
script for modules.

	Sam

Re: "build-id" changes break sparc64

[Roland McGrath]

> > -cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
> > +cmd_link_multi-m = $(cmd_link_multi-y)
> This signel change looks wrong. We do not want to generate a linker
> script for modules.

multi-m has nothing to do with modules, really.  It's just for another
"built-in.o"-style object that happens to be the input for a .ko link
rather than the vmlinux link.  


Thanks,
Roland

Re: "build-id" changes break sparc64

[Sam Ravnborg]

On Mon, Jul 23, 2007 at 01:18:14AM -0700, Roland McGrath wrote:
> > > -cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
> > > +cmd_link_multi-m = $(cmd_link_multi-y)
> > This signel change looks wrong. We do not want to generate a linker
> > script for modules.
> 
> multi-m has nothing to do with modules, really.  It's just for another
> "built-in.o"-style object that happens to be the input for a .ko link
> rather than the vmlinux link.  
OK - so even in that case we should defer to the link script trick - OK.

	Sam

Re: "build-id" changes break sparc64

[Alan Modra]

On Mon, Jul 23, 2007 at 10:14:35AM +0200, Sam Ravnborg wrote:
> But I would still like to hear from Alan what the benefits are.

See http://sourceware.org/ml/binutils/2004-10/msg00178.html

-- 
Alan Modra
Australia Development Lab, IBM

Re: "build-id" changes break sparc64

[Al Viro]

On Mon, Jul 23, 2007 at 06:55:59PM +0930, Alan Modra wrote:
> On Mon, Jul 23, 2007 at 10:14:35AM +0200, Sam Ravnborg wrote:
> > But I would still like to hear from Alan what the benefits are.
> 
> See http://sourceware.org/ml/binutils/2004-10/msg00178.html

What does _not_ doing intermediates do to memory footprint of ld(1)
and time spent in there?

Re: "build-id" changes break sparc64

[Sam Ravnborg]

On Mon, Jul 23, 2007 at 12:49:36PM +0100, Al Viro wrote:
> On Mon, Jul 23, 2007 at 06:55:59PM +0930, Alan Modra wrote:
> > On Mon, Jul 23, 2007 at 10:14:35AM +0200, Sam Ravnborg wrote:
> > > But I would still like to hear from Alan what the benefits are.
> > 
> > See http://sourceware.org/ml/binutils/2004-10/msg00178.html
> 
> What does _not_ doing intermediates do to memory footprint of ld(1)
> and time spent in there?

x86_64 defconfig

rm vmlinux*; time make vmlinux

Vanilla tree: ~7,7 sec
With single shot ld (Roland's patch): 8,3 secs

So as expected slower. As we link twice the cost is ~0,3 sec 
for a x86_64 defconfig link on my box.

So it should be beneficial to do it and Alan's link did not really
convince me for the kernel usage.
Al - do you have any input there?

	Sam

Re: "build-id" changes break sparc64

[Al Viro]

On Mon, Jul 23, 2007 at 08:00:48PM +0200, Sam Ravnborg wrote:
> On Mon, Jul 23, 2007 at 12:49:36PM +0100, Al Viro wrote:
> > On Mon, Jul 23, 2007 at 06:55:59PM +0930, Alan Modra wrote:
> > > On Mon, Jul 23, 2007 at 10:14:35AM +0200, Sam Ravnborg wrote:
> > > > But I would still like to hear from Alan what the benefits are.
> > > 
> > > See http://sourceware.org/ml/binutils/2004-10/msg00178.html
> > 
> > What does _not_ doing intermediates do to memory footprint of ld(1)
> > and time spent in there?
> 
> x86_64 defconfig
> 
> rm vmlinux*; time make vmlinux
> 
> Vanilla tree: ~7,7 sec
> With single shot ld (Roland's patch): 8,3 secs
> 
> So as expected slower. As we link twice the cost is ~0,3 sec 
> for a x86_64 defconfig link on my box.

defconfig is not too interesting, but anyway - how much memory does it
take in process?  And, obviously, what kind of box it is?
 
> So it should be beneficial to do it and Alan's link did not really
> convince me for the kernel usage.
> Al - do you have any input there?

Depends.  If more-or-less allmodconfig doesn't kick everything else out
of cache and doesn't end up creating fsckloads of seeks...

Actually, if you want a killer, see what happens when building with debug
info...

Re: "build-id" changes break sparc64

[Adrian Bunk]

On Mon, Jul 23, 2007 at 07:56:48PM +0100, Al Viro wrote:
> On Mon, Jul 23, 2007 at 08:00:48PM +0200, Sam Ravnborg wrote:
> > On Mon, Jul 23, 2007 at 12:49:36PM +0100, Al Viro wrote:
> > > On Mon, Jul 23, 2007 at 06:55:59PM +0930, Alan Modra wrote:
> > > > On Mon, Jul 23, 2007 at 10:14:35AM +0200, Sam Ravnborg wrote:
> > > > > But I would still like to hear from Alan what the benefits are.
> > > > 
> > > > See http://sourceware.org/ml/binutils/2004-10/msg00178.html
> > > 
> > > What does _not_ doing intermediates do to memory footprint of ld(1)
> > > and time spent in there?
> > 
> > x86_64 defconfig
> > 
> > rm vmlinux*; time make vmlinux
> > 
> > Vanilla tree: ~7,7 sec
> > With single shot ld (Roland's patch): 8,3 secs
> > 
> > So as expected slower. As we link twice the cost is ~0,3 sec 
> > for a x86_64 defconfig link on my box.
> 
> defconfig is not too interesting, but anyway - how much memory does it
> take in process?  And, obviously, what kind of box it is?
>  
> > So it should be beneficial to do it and Alan's link did not really
> > convince me for the kernel usage.
> > Al - do you have any input there?
> 
> Depends.  If more-or-less allmodconfig doesn't kick everything else out
> of cache and doesn't end up creating fsckloads of seeks...
> 
> Actually, if you want a killer, see what happens when building with debug
> info...

Shouldn't "allyesconfig" be the worst case?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: "build-id" changes break sparc64

[Roland McGrath]

> rm vmlinux*; time make vmlinux
> 
> Vanilla tree: ~7,7 sec
> With single shot ld (Roland's patch): 8,3 secs

Shouldn't you compare:
	rm vmlinux*; find . -name built-in.o -print | xargs rm
	time make vmlinux
or something like that?


Thanks,
Roland

Re: "build-id" changes break sparc64

[Al Viro]

On Mon, Jul 23, 2007 at 12:12:15PM -0700, Roland McGrath wrote:
> > rm vmlinux*; time make vmlinux
> > 
> > Vanilla tree: ~7,7 sec
> > With single shot ld (Roland's patch): 8,3 secs
> 
> Shouldn't you compare:
> 	rm vmlinux*; find . -name built-in.o -print | xargs rm
> 	time make vmlinux
> or something like that?

No.  You are talking about the time of full build; there it would
be relevant.  For incrementals it's definitely not so.

Re: "build-id" changes break sparc64

[Roland McGrath]

> On Mon, Jul 23, 2007 at 12:12:15PM -0700, Roland McGrath wrote:
> > > rm vmlinux*; time make vmlinux
> > > 
> > > Vanilla tree: ~7,7 sec
> > > With single shot ld (Roland's patch): 8,3 secs
> > 
> > Shouldn't you compare:
> > 	rm vmlinux*; find . -name built-in.o -print | xargs rm
> > 	time make vmlinux
> > or something like that?
> 
> No.  You are talking about the time of full build; there it would
> be relevant.  For incrementals it's definitely not so.

Ah, I see what you are testing.  Well, a fair test then is to do something
that would actually happen, which is almost never just "rm vmlinux".
i.e., at least one ld -r -o foo/built-in.o from changing one source file.


Thanks,
Roland

Re: "build-id" changes break sparc64

[Sam Ravnborg]

On Mon, Jul 23, 2007 at 01:05:00PM -0700, Roland McGrath wrote:
> > On Mon, Jul 23, 2007 at 12:12:15PM -0700, Roland McGrath wrote:
> > > > rm vmlinux*; time make vmlinux
> > > > 
> > > > Vanilla tree: ~7,7 sec
> > > > With single shot ld (Roland's patch): 8,3 secs
> > > 
> > > Shouldn't you compare:
> > > 	rm vmlinux*; find . -name built-in.o -print | xargs rm
> > > 	time make vmlinux
> > > or something like that?
> > 
> > No.  You are talking about the time of full build; there it would
> > be relevant.  For incrementals it's definitely not so.
> 
> Ah, I see what you are testing.  Well, a fair test then is to do something
> that would actually happen, which is almost never just "rm vmlinux".
> i.e., at least one ld -r -o foo/built-in.o from changing one source file.

The normal case is one change somewhere => build kernel.
That can be tested by a simple
rm mm/memory.o; /usr/sbin/time make vmlinux

I will try that later.
For now I'm building an allyesconfig just to see if the build actually completes.
It is quite a number of .o files to link in one go and I fear ld will blow up 
and make my machine go OOM.

processor	: 0
vendor_id	: AuthenticAMD
cpu family	: 15
model		: 75
model name	: AMD Athlon(tm) 64 X2 Dual Core Processor 3800+
stepping	: 2
cpu MHz		: 2000.000
cache size	: 512 KB
physical id	: 0
siblings	: 2
core id		: 0
cpu cores	: 2
fpu		: yes
fpu_exception	: yes
cpuid level	: 1
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt rdtscp lm 3dnowext 3dnow pni cx16 lahf_lm cmp_legacy svm cr8_legacy
bogomips	: 4011.99
TLB size	: 1024 4K pages
clflush size	: 64
cache_alignment	: 64
address sizes	: 40 bits physical, 48 bits virtual
power management: ts fid vid ttp tm stc

processor	: 1
vendor_id	: AuthenticAMD
cpu family	: 15
model		: 75
model name	: AMD Athlon(tm) 64 X2 Dual Core Processor 3800+
stepping	: 2
cpu MHz		: 2000.000
cache size	: 512 KB
physical id	: 0
siblings	: 2
core id		: 1
cpu cores	: 2
fpu		: yes
fpu_exception	: yes
cpuid level	: 1
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt rdtscp lm 3dnowext 3dnow pni cx16 lahf_lm cmp_legacy svm cr8_legacy
bogomips	: 4011.99
TLB size	: 1024 4K pages
clflush size	: 64
cache_alignment	: 64
address sizes	: 40 bits physical, 48 bits virtual
power management: ts fid vid ttp tm stc

meminfo:
MemTotal:      1027044 kB
MemFree:         83040 kB
Buffers:          4996 kB
Cached:         510208 kB
SwapCached:          0 kB
Active:         447092 kB
Inactive:       406620 kB
SwapTotal:     3004112 kB
SwapFree:      2965072 kB
Dirty:          107740 kB
Writeback:           0 kB
AnonPages:      338528 kB
Mapped:          62552 kB
Slab:            50684 kB
SReclaimable:    30132 kB
SUnreclaim:      20552 kB
PageTables:      13688 kB
NFS_Unstable:        0 kB
Bounce:              0 kB
CommitLimit:   3517632 kB
Committed_AS:   749456 kB
VmallocTotal: 34359738367 kB
VmallocUsed:     40636 kB
VmallocChunk: 34359695867 kB

	Sam

[PATCH] Move --build-id option

[Roland McGrath]

My original makefile patch to use ld --build-id wound up using it in too
many places.  We want it only for the .ko and vmlinux links (and vmlinux
temporary links that determine the vmlinux layout).

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 Makefile                 |    8 +++-----
 scripts/Makefile.modpost |    4 ++--
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index cd47845..fe6c5dd 100644
--- a/Makefile
+++ b/Makefile
@@ -518,8 +518,6 @@ CFLAGS += $(call cc-option,-Wno-pointer-sign,)
 # Use --build-id when available.
 LDFLAGS_BUILD_ID = $(patsubst -Wl$(comma)%,%,\
 			      $(call ld-option, -Wl$(comma)--build-id,))
-LDFLAGS_MODULE += $(LDFLAGS_BUILD_ID)
-LDFLAGS_vmlinux += $(LDFLAGS_BUILD_ID)
 
 # Default kernel image to build when no specific target is given.
 # KBUILD_IMAGE may be overruled on the command line or
@@ -616,9 +614,9 @@ export KBUILD_VMLINUX_OBJS := $(vmlinux-all)
 # Rule to link vmlinux - also used during CONFIG_KALLSYMS
 # May be overridden by arch/$(ARCH)/Makefile
 quiet_cmd_vmlinux__ ?= LD      $@
-      cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) -o $@ \
-      -T $(vmlinux-lds) $(vmlinux-init)                          \
-      --start-group $(vmlinux-main) --end-group                  \
+      cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_BUILD_ID) $(LDFLAGS_vmlinux) \
+      -o $@ -T $(vmlinux-lds) $(vmlinux-init)		\
+      --start-group $(vmlinux-main) --end-group		\
       $(filter-out $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o FORCE ,$^)
 
 # Generate new vmlinux version
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index c6fcc59..a86a3b1 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -97,8 +97,8 @@ targets += $(modules:.ko=.mod.o)
 
 # Step 6), final link of the modules
 quiet_cmd_ld_ko_o = LD [M]  $@
-      cmd_ld_ko_o = $(LD) $(LDFLAGS) $(LDFLAGS_MODULE) -o $@		\
-			  $(filter-out FORCE,$^)
+      cmd_ld_ko_o = $(LD) $(LDFLAGS) $(LDFLAGS_BUILD_ID) $(LDFLAGS_MODULE) \
+      		    	  -o $@ $(filter-out FORCE,$^)
 
 $(modules): %.ko :%.o %.mod.o FORCE
 	$(call if_changed,ld_ko_o)

Re: [PATCH] Move --build-id option

[Sam Ravnborg]

On Sun, Jul 22, 2007 at 11:14:46PM -0700, Roland McGrath wrote:
> 
> My original makefile patch to use ld --build-id wound up using it in too
> many places.  We want it only for the .ko and vmlinux links (and vmlinux
> temporary links that determine the vmlinux layout).
> 
> Signed-off-by: Roland McGrath <roland@redhat.com>
> ---
>  Makefile                 |    8 +++-----
>  scripts/Makefile.modpost |    4 ++--
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index cd47845..fe6c5dd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -518,8 +518,6 @@ CFLAGS += $(call cc-option,-Wno-pointer-sign,)
>  # Use --build-id when available.
>  LDFLAGS_BUILD_ID = $(patsubst -Wl$(comma)%,%,\
>  			      $(call ld-option, -Wl$(comma)--build-id,))
> -LDFLAGS_MODULE += $(LDFLAGS_BUILD_ID)
As noted below keep this line.

> -LDFLAGS_vmlinux += $(LDFLAGS_BUILD_ID)
This needed to go otherwise you had to rely on all arch makefiles 
used += for LDFLAGS_vmlinux assignments.
>  
>  # Default kernel image to build when no specific target is given.
>  # KBUILD_IMAGE may be overruled on the command line or
> @@ -616,9 +614,9 @@ export KBUILD_VMLINUX_OBJS := $(vmlinux-all)
>  # Rule to link vmlinux - also used during CONFIG_KALLSYMS
>  # May be overridden by arch/$(ARCH)/Makefile
>  quiet_cmd_vmlinux__ ?= LD      $@
> -      cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) -o $@ \
> -      -T $(vmlinux-lds) $(vmlinux-init)                          \
> -      --start-group $(vmlinux-main) --end-group                  \
> +      cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_BUILD_ID) $(LDFLAGS_vmlinux) \
> +      -o $@ -T $(vmlinux-lds) $(vmlinux-init)		\
> +      --start-group $(vmlinux-main) --end-group		\
>        $(filter-out $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o FORCE ,$^)
This is OK.
>  
>  # Generate new vmlinux version
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index c6fcc59..a86a3b1 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -97,8 +97,8 @@ targets += $(modules:.ko=.mod.o)
>  
>  # Step 6), final link of the modules
>  quiet_cmd_ld_ko_o = LD [M]  $@
> -      cmd_ld_ko_o = $(LD) $(LDFLAGS) $(LDFLAGS_MODULE) -o $@		\
> -			  $(filter-out FORCE,$^)
> +      cmd_ld_ko_o = $(LD) $(LDFLAGS) $(LDFLAGS_BUILD_ID) $(LDFLAGS_MODULE) \
> +      		    	  -o $@ $(filter-out FORCE,$^)

This will not work as LDFLAGS_BUILD_ID is not exported.
Please make the change as outlined in earlier mail so we actually start
using LDFALGS_MODULE as documented.

Thanks,
	Sam

Re: [PATCH] Move --build-id option

[Roland McGrath]

> > -LDFLAGS_vmlinux += $(LDFLAGS_BUILD_ID)
> This needed to go otherwise you had to rely on all arch makefiles 
> used += for LDFLAGS_vmlinux assignments.

This line appears well after include $(srctree)/arch/$(ARCH)/Makefile.

> This will not work as LDFLAGS_BUILD_ID is not exported.

Oops!  I was overriding it in my testing, which exports it, so I didn't
notice that problem.

> Please make the change as outlined in earlier mail so we actually start
> using LDFALGS_MODULE as documented.

Patch coming.


Thanks,
Roland

[PATCH] Use LDFLAGS_MODULE only for .ko links

[Roland McGrath]

This replaces the "Move --build-id option" patch.

Thanks,
Roland

---

Sam Ravnborg pointed out that Documentation/kbuild/makefiles.txt already
says this is what it's for.  This patch makes the reality live up to the
documentation.  This fixes the problem of LDFLAGS_BUILD_ID getting into too
many places.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 Makefile                 |    2 +-
 scripts/Makefile.build   |    2 +-
 scripts/Makefile.modpost |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index cd47845..f510c39 100644
--- a/Makefile
+++ b/Makefile
@@ -299,7 +299,7 @@ CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise $(C
 MODFLAGS	= -DMODULE
 CFLAGS_MODULE   = $(MODFLAGS)
 AFLAGS_MODULE   = $(MODFLAGS)
-LDFLAGS_MODULE  = -r
+LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
 
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3f7b451..7fd6055 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -315,7 +315,7 @@ quiet_cmd_link_multi-y = LD      $@
 cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
 
 quiet_cmd_link_multi-m = LD [M]  $@
-cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
+cmd_link_multi-m = $(cmd_link_multi-y)
 
 # We would rather have a list of rules like
 # 	foo.o: $(foo-objs)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index c6fcc59..149e604 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -97,7 +97,7 @@ targets += $(modules:.ko=.mod.o)
 
 # Step 6), final link of the modules
 quiet_cmd_ld_ko_o = LD [M]  $@
-      cmd_ld_ko_o = $(LD) $(LDFLAGS) $(LDFLAGS_MODULE) -o $@		\
+      cmd_ld_ko_o = $(LD) -r $(LDFLAGS) $(LDFLAGS_MODULE) -o $@		\
 			  $(filter-out FORCE,$^)
 
 $(modules): %.ko :%.o %.mod.o FORCE

Re: [PATCH] Use LDFLAGS_MODULE only for .ko links

[Sam Ravnborg]

On Mon, Jul 23, 2007 at 01:12:08AM -0700, Roland McGrath wrote:
> This replaces the "Move --build-id option" patch.
> 
> Thanks,
> Roland
> 
> ---
> 
> Sam Ravnborg pointed out that Documentation/kbuild/makefiles.txt already
> says this is what it's for.  This patch makes the reality live up to the
> documentation.  This fixes the problem of LDFLAGS_BUILD_ID getting into too
> many places.
> 
> Signed-off-by: Roland McGrath <roland@redhat.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

[I though have not had the possibility to actually test the thing].

	Sam

Re: "build-id" changes break sparc64

[Sam Ravnborg]

On Sun, Jul 22, 2007 at 11:01:55PM -0700, Roland McGrath wrote:
> It turns out the problem here is that some .o files wind up with their own
> .note.gnu.build-id sections.  I got the makefile magic wrong, thinking that
> LDFLAGS_MODULE was a variable specifically for .ko links.

Reading Documentation/kbuild/makefiles.txt:
LDFLAGS_MODULE      Options for $(LD) when linking modules

        LDFLAGS_MODULE is used to set specific flags for $(LD) when
        linking the .ko files used for modules.

So it seems to be documented as such.

Browsing the source it looks like a bug that LDFLAGS_MODULE is used in
cmd_link_multi-m.
I suggest you to do the following:
Remove '-r' from LDFLAGS_MODULE in top-level Makefile
Hardcode '-r' in cmd_link_multi-m and remove LDFLAGS_MODULE in the same.
Hardcode '-r' in Makefile,modpost
Add your stuff to LDFLAGS_MODULE in toplevel Makefile.

Sorry for not prrviding a preliminary patch but away from my dev box
with no easy source access.

	Sam