public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Jan Beulich <jbeulich@novell.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: your change to prevent needless re-building of vmlinux.o
Date: Sat, 17 May 2008 14:52:04 +0200	[thread overview]
Message-ID: <20080517125204.GA19741@uranus.ravnborg.org> (raw)
In-Reply-To: <482DCD89.76E4.0078.0@novell.com>

Hi Jan.

On Fri, May 16, 2008 at 05:08:09PM +0100, Jan Beulich wrote:
> Sam,
> 
> while also looking into this issue (found time only now) I realized this is
> already worked around in .26-rc2. However, it would seem to me that
> suppressing the dependency on init/built-in.o altogether is likely to
> cause problems in the future.
What is not obvious is that the only reason we link vmlinux.o is to
support the section mismatch check.
I once had a version where I used vmlinux.o as an intermediate step
but that did not speed up the following link so I dropped it again.
Therefore the suppressed init/built-in.o dependency will only
cause us to miss a section mismatch check in very rare cases and
will not cause troubles for a normal kernel link.

I have also had in mind to make the vmlinux.o link dependent
on CONFIG_DEBUG_SECTION_MISMATCH so we could skip it for a
kernel build where we do not do this analysis.
But the vmlinux.o link were kept on the rationale that
CONFIG_DEBUG_SECTION_MISMATCH should be used to trigger an
analysis for all links and in the normal situation we would
do the check only on vmlinux.o and for the individual modules.

So I hope this explains why init/built-in.o is supressed today.


> Going through the version generating
> logic again I realized that since vmlinux.o is always built before the first
> of the (perhaps multiple, due to kallsyms) final linking stages, it ought
> to be possible to simply move the version generation into the vmlinux.o
> rule. And indeed, this
> 
> define rule_vmlinux-modpost
> 	:
> 	+$(call cmd,vmlinux_version)
> 	+$(call cmd,vmlinux-modpost)
> 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost $@
> 	$(Q)echo 'cmd_$@ := $(cmd_vmlinux-modpost)' > $(dot-target).cmd
> endef
> 
> with the two other invocations of vmlinux_version removed, seems to
> do what is wanted.A

So what you suggest is something like this:

diff --git a/Makefile b/Makefile
index 3140145..3dbfffc 100644
--- a/Makefile
+++ b/Makefile
@@ -657,7 +657,6 @@ vmlinux-init := $(head-y) $(init-y)
 vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y)
 vmlinux-all  := $(vmlinux-init) $(vmlinux-main)
 vmlinux-lds  := arch/$(SRCARCH)/kernel/vmlinux.lds
-export KBUILD_VMLINUX_OBJS := $(vmlinux-all)
 
 # Rule to link vmlinux - also used during CONFIG_KALLSYMS
 # May be overridden by arch/$(ARCH)/Makefile
@@ -690,8 +689,6 @@ quiet_cmd_sysmap = SYSMAP
 # First command is ':' to allow us to use + in front of the rule
 define rule_vmlinux__
 	:
-	$(if $(CONFIG_KALLSYMS),,+$(call cmd,vmlinux_version))
-
 	$(call cmd,vmlinux__)
 	$(Q)echo 'cmd_$@ := $(cmd_vmlinux__)' > $(@D)/.$(@F).cmd
 
@@ -747,8 +744,7 @@ endef
 # First command is ':' to allow us to use + in front of this rule
 cmd_ksym_ld = $(cmd_vmlinux__)
 define rule_ksym_ld
-	: 
-	+$(call cmd,vmlinux_version)
+	:
 	$(call cmd,vmlinux__)
 	$(Q)echo 'cmd_$@ := $(cmd_vmlinux__)' > $(@D)/.$(@F).cmd
 endef
@@ -798,6 +794,7 @@ quiet_cmd_vmlinux-modpost = LD      $@
 define rule_vmlinux-modpost
 	:
 	+$(call cmd,vmlinux-modpost)
+	+$(call cmd,vmlinux_version)
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost $@
 	$(Q)echo 'cmd_$@ := $(cmd_vmlinux-modpost)' > $(dot-target).cmd
 endef
@@ -810,7 +807,6 @@ endif
 ifdef CONFIG_SAMPLES
 	$(Q)$(MAKE) $(build)=samples
 endif
-	$(call vmlinux-modpost)
 	$(call if_changed_rule,vmlinux__)
 	$(Q)rm -f .old_version
 

This is a welcome simplifications and I like it.
If we decide to keep the intermediate vmlinux.o link than we should
do so.
If you agree I can try to give it a spin in -next.
[There are two unrelated clean-ups in the patch - both are noops].

Everything that can simplify the final link of vmlinux is welcome.

I have several times been tempted to move it all to a shell script.
What has prevented me to do so has only been:
1) that um has some subtle requirement and therefore redefine the
   vmlinux link
2) that last time I reworked this stuff Andrew could not boot one
   of his systems and I never figured out why
3) lack of time..

But if someone comes up with a patch that moves all this stuff out
of the top-level Makefile I am all open for it.

> 
> What do you think? Am I overlooking some aspect here?
> 
> Btw., another slight anomaly I noticed is that modpost on the modules
> is always performed (even when nothing changed in the tree), while
> on vmlinux.o it is only performed when something caused a re-link. Is
> that intentional, or just a to-be-tolerated side effect?
It is intentional as in we do not have information available
if a module has been build or not.
modpost take care not to overwrite module.mod.c files if the
generated file equals the old file. But could we skip the modpost
entirely it would be better. But that was not trivial last I looked
at it (loong time ago).

	Sam

  reply	other threads:[~2008-05-17 12:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-16 16:08 your change to prevent needless re-building of vmlinux.o Jan Beulich
2008-05-17 12:52 ` Sam Ravnborg [this message]
2008-05-18 18:52   ` [RFC PATCH] move link of vmlinux from Makefile to a script Sam Ravnborg
2008-05-19  7:42     ` Jan Beulich
2008-05-19  8:19       ` Sam Ravnborg
2008-05-19  8:35         ` Jan Beulich
2008-05-19  7:36   ` your change to prevent needless re-building of vmlinux.o Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080517125204.GA19741@uranus.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=jbeulich@novell.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox