CONFIG_RELOCATABLE modpost fix
Run modpost on vmlinux regardless of CONFIG_MODULES. The modpost code is also
extended to ignore references from ".pci_fixup" to ".init.text".
Signed-off-by: Magnus Damm <[email protected]>
---
This patch applies on top of Eric W. Biedermans CONFIG_RELOCATABLE tree
Makefile | 1 +
scripts/Makefile | 4 ++--
scripts/Makefile.modpost | 6 +++++-
scripts/mod/modpost.c | 14 +++++++++++---
--- 0001/Makefile
+++ work/Makefile 2006-08-07 13:11:44.000000000 +0900
@@ -723,6 +723,7 @@ endif # ifdef CONFIG_KALLSYMS
# vmlinux image - including updated kernel symbols
vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) $(kallsyms.o) FORCE
$(call if_changed_rule,vmlinux__)
+ $(Q)$(MAKE) -rR -f $(srctree)/scripts/Makefile.modpost __modpost_kernel
$(Q)rm -f .old_version
# The actual objects are generated when descending,
--- 0001/scripts/Makefile
+++ work/scripts/Makefile 2006-08-07 13:09:47.000000000 +0900
@@ -16,7 +16,7 @@ hostprogs-$(CONFIG_IKCONFIG) += bin2
always := $(hostprogs-y)
subdir-$(CONFIG_MODVERSIONS) += genksyms
-subdir-$(CONFIG_MODULES) += mod
+subdir-y += mod
# Let clean descend into subdirs
-subdir- += basic kconfig package
+subdir- += basic kconfig package mod
--- 0001/scripts/Makefile.modpost
+++ work/scripts/Makefile.modpost 2006-08-07 13:10:44.000000000 +0900
@@ -61,7 +61,11 @@ quiet_cmd_modpost = MODPOST
$(filter-out FORCE,$^)
PHONY += __modpost
-__modpost: $(wildcard vmlinux) $(modules:.ko=.o) FORCE
+__modpost: $(modules:.ko=.o) FORCE
+ $(call cmd,modpost)
+
+PHONY += __modpost_kernel
+__modpost_kernel: $(wildcard vmlinux) FORCE
$(call cmd,modpost)
# Declare generated files as targets for modpost
--- 0001/scripts/mod/modpost.c
+++ work/scripts/mod/modpost.c 2006-08-07 16:45:30.000000000 +0900
@@ -581,8 +581,8 @@ static int strrcmp(const char *s, const
* fromsec = .data
* atsym = *driver, *_template, *_sht, *_ops, *_probe, *probe_one
**/
-static int secref_whitelist(const char *tosec, const char *fromsec,
- const char *atsym)
+static int secref_whitelist(const char *modname, const char *tosec,
+ const char *fromsec, const char *atsym)
{
int f1 = 1, f2 = 1;
const char **s;
@@ -596,6 +596,13 @@ static int secref_whitelist(const char *
NULL
};
+ /* Ignore all references from .pci_fixup section if vmlinux */
+ if (strcmp(modname, "vmlinux") == 0) {
+ if ((strcmp(fromsec, ".pci_fixup") == 0) &&
+ (strcmp(tosec, ".init.text") == 0))
+ return 1;
+ }
+
/* Check for pattern 1 */
if (strcmp(tosec, ".init.data") != 0)
f1 = 0;
@@ -726,7 +733,8 @@ static void warn_sec_mismatch(const char
/* check whitelist - we may ignore it */
if (before &&
- secref_whitelist(secname, fromsec, elf->strtab + before->st_name))
+ secref_whitelist(modname, secname, fromsec,
+ elf->strtab + before->st_name))
return;
if (before && after) {
On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> CONFIG_RELOCATABLE modpost fix
>
> Run modpost on vmlinux regardless of CONFIG_MODULES. The modpost code is also
> extended to ignore references from ".pci_fixup" to ".init.text".
I've splitted the patch in two parts.
First is this one (I slightly modified your version and removed trailing
whitespaces).
Sam
commit 1f43a633dc485f90fddf667270179058a07b9d77
Author: Magnus Damm <[email protected]>
Date: Tue Aug 8 17:32:11 2006 +0900
kbuild: ignore references from ".pci_fixup" to ".init.text"
The modpost code is extended to ignore references
from ".pci_fixup" to ".init.text".
Signed-off-by: Magnus Damm <[email protected]>
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index dfde0e8..5028d46 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -581,8 +581,8 @@ static int strrcmp(const char *s, const
* fromsec = .data
* atsym = *driver, *_template, *_sht, *_ops, *_probe, *probe_one
**/
-static int secref_whitelist(const char *tosec, const char *fromsec,
- const char *atsym)
+static int secref_whitelist(const char *modname, const char *tosec,
+ const char *fromsec, const char *atsym)
{
int f1 = 1, f2 = 1;
const char **s;
@@ -618,8 +618,15 @@ static int secref_whitelist(const char *
for (s = pat2sym; *s; s++)
if (strrcmp(atsym, *s) == 0)
f1 = 1;
+ if (f1 && f2)
+ return 1;
- return f1 && f2;
+ /* Whitelist all references from .pci_fixup section if vmlinux */
+ if (is_vmlinux(modname)) {
+ if ((strcmp(fromsec, ".pci_fixup") == 0) &&
+ (strcmp(tosec, ".init.text") == 0))
+ return 1;
+ }
}
/**
@@ -726,7 +733,8 @@ static void warn_sec_mismatch(const char
/* check whitelist - we may ignore it */
if (before &&
- secref_whitelist(secname, fromsec, elf->strtab + before->st_name))
+ secref_whitelist(modname, secname, fromsec,
+ elf->strtab + before->st_name))
return;
if (before && after) {
On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> CONFIG_RELOCATABLE modpost fix
>
> Run modpost on vmlinux regardless of CONFIG_MODULES.
Below is my take on this one.
- Dropped -rR since this is default now
- Dropped subdir- assignment in scripts/Makefile since it is redundant
- Always pass vmlinux ti modpost so we have full updated info
- Print out number of modules being mod posted to distingush from
vmlinux one
- use vmlinux as target name to enable nicer quiet command print
Sam
diff --git a/Makefile b/Makefile
index 72ef9f6..6b86f4e 100644
--- a/Makefile
+++ b/Makefile
@@ -724,6 +724,7 @@ endif # ifdef CONFIG_KALLSYMS
# vmlinux image - including updated kernel symbols
vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) $(kallsyms.o) FORCE
$(call if_changed_rule,vmlinux__)
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost $@
$(Q)rm -f .old_version
# The actual objects are generated when descending,
diff --git a/scripts/Makefile b/scripts/Makefile
index d531a1f..ea41de8 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -19,7 +19,7 @@ # The following hostprogs-y programs are
hostprogs-y += unifdef
subdir-$(CONFIG_MODVERSIONS) += genksyms
-subdir-$(CONFIG_MODULES) += mod
+subdir-y += mod
# Let clean descend into subdirs
subdir- += basic kconfig package
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 0a64688..9137df2 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -51,19 +51,25 @@ _modpost: $(modules)
# Step 2), invoke modpost
# Includes step 3,4
-quiet_cmd_modpost = MODPOST
+quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
cmd_modpost = scripts/mod/modpost \
$(if $(CONFIG_MODVERSIONS),-m) \
$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a,) \
$(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
$(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
$(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
- $(filter-out FORCE,$^)
+ $(wildcard vmlinux) $(filter-out FORCE,$^)
PHONY += __modpost
-__modpost: $(wildcard vmlinux) $(modules:.ko=.o) FORCE
+__modpost: $(modules:.ko=.o) FORCE
$(call cmd,modpost)
+quiet_cmd_kernel-mod = MODPOST $@
+ cmd_kernel-mod = $(cmd_modpost)
+
+vmlinux: FORCE
+ $(call cmd,kernel-mod)
+
# Declare generated files as targets for modpost
$(symverfile): __modpost ;
$(modules:.ko=.mod.c): __modpost ;
Sam Ravnborg <[email protected]> writes:
> On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
>> CONFIG_RELOCATABLE modpost fix
>>
>> Run modpost on vmlinux regardless of CONFIG_MODULES.
> Below is my take on this one.
> - Dropped -rR since this is default now
> - Dropped subdir- assignment in scripts/Makefile since it is redundant
> - Always pass vmlinux ti modpost so we have full updated info
> - Print out number of modules being mod posted to distingush from
> vmlinux one
> - use vmlinux as target name to enable nicer quiet command print
Sam, Magnus:
I'm dense. Why do we want to run modpost if we are building a kernel
that doesn't support modules?
I haven't mucked with modpost at all so I don't have a good feel for
what it does, or why we want to run it.
My quick skimming says modpost is all about generating the module
version symbol scrambling. Which if that is all it does means it is
senseless to run this without modules.
Eric
On Tue, Aug 08, 2006 at 01:59:03PM -0600, Eric W. Biederman wrote:
> Sam, Magnus:
>
> I'm dense. Why do we want to run modpost if we are building a kernel
> that doesn't support modules?
>
> I haven't mucked with modpost at all so I don't have a good feel for
> what it does, or why we want to run it.
>
> My quick skimming says modpost is all about generating the module
> version symbol scrambling. Which if that is all it does means it is
> senseless to run this without modules.
Recently modpost has been enhanced to do section reference checks.
So modpost is used to check that there is no references from .text to
.init.text - the latter will be freed by the kernel so we better avoid
it.
The consistency checks does a bit more than just that simple check but
this was enough reason to run it twice in the build process.
Sam
Hi Sam,
Thanks for picking up the patch!
On Tue, 2006-08-08 at 17:16 +0200, Sam Ravnborg wrote:
> On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> > CONFIG_RELOCATABLE modpost fix
> >
> > Run modpost on vmlinux regardless of CONFIG_MODULES. The modpost code is also
> > extended to ignore references from ".pci_fixup" to ".init.text".
> I've splitted the patch in two parts.
> First is this one (I slightly modified your version and removed trailing
> whitespaces).
>
> Sam
>
> commit 1f43a633dc485f90fddf667270179058a07b9d77
> Author: Magnus Damm <[email protected]>
> Date: Tue Aug 8 17:32:11 2006 +0900
>
> kbuild: ignore references from ".pci_fixup" to ".init.text"
>
> The modpost code is extended to ignore references
> from ".pci_fixup" to ".init.text".
>
> Signed-off-by: Magnus Damm <[email protected]>
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index dfde0e8..5028d46 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -581,8 +581,8 @@ static int strrcmp(const char *s, const
> * fromsec = .data
> * atsym = *driver, *_template, *_sht, *_ops, *_probe, *probe_one
> **/
> -static int secref_whitelist(const char *tosec, const char *fromsec,
> - const char *atsym)
> +static int secref_whitelist(const char *modname, const char *tosec,
> + const char *fromsec, const char *atsym)
> {
> int f1 = 1, f2 = 1;
> const char **s;
> @@ -618,8 +618,15 @@ static int secref_whitelist(const char *
> for (s = pat2sym; *s; s++)
> if (strrcmp(atsym, *s) == 0)
> f1 = 1;
> + if (f1 && f2)
> + return 1;
>
> - return f1 && f2;
> + /* Whitelist all references from .pci_fixup section if vmlinux */
> + if (is_vmlinux(modname)) {
> + if ((strcmp(fromsec, ".pci_fixup") == 0) &&
> + (strcmp(tosec, ".init.text") == 0))
> + return 1;
> + }
> }
You forget to return a value which result in the following warning:
HOSTCC scripts/mod/modpost.o
scripts/mod/modpost.c: In function `secref_whitelist':
scripts/mod/modpost.c:630: warning: control reaches end of non-void
function
HOSTCC scripts/mod/sumversion.o
Cheers,
/magnus
Hi again Sam,
On Tue, 2006-08-08 at 20:39 +0200, Sam Ravnborg wrote:
> On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> > CONFIG_RELOCATABLE modpost fix
> >
> > Run modpost on vmlinux regardless of CONFIG_MODULES.
> Below is my take on this one.
> - Dropped -rR since this is default now
> - Dropped subdir- assignment in scripts/Makefile since it is redundant
> - Always pass vmlinux ti modpost so we have full updated info
> - Print out number of modules being mod posted to distingush from
> vmlinux one
> - use vmlinux as target name to enable nicer quiet command print
Your patch seems to work as expected if I add a return 0 at the end of
modpost.c:secref_whitelist(). I like how you printed out the number of
modules being processed. I have one minor comment about your patch:
Modpost seems to get run twice on vmlinux if the kernel is built with
"make all". I think it would be best to run modpost on vmlinux only when
vmlinux is built - never when modules are processed.
Thanks,
/ magnus
On Wed, Aug 09, 2006 at 10:32:36AM +0900, Magnus Damm wrote:
> Your patch seems to work as expected if I add a return 0 at the end of
> modpost.c:secref_whitelist(). I like how you printed out the number of
> modules being processed.
Thanks - fixed now. My gcc (3.4.6-r1 from Gentoo did not warn)
>I have one minor comment about your patch:
>
> Modpost seems to get run twice on vmlinux if the kernel is built with
> "make all". I think it would be best to run modpost on vmlinux only when
> vmlinux is built - never when modules are processed.
Thesecond time modpost runs vmlinux is used to pick up symbol
information to check that all symbols are valid etc.
The alternative was to trust the symbols being read from Module.symvers
and that would be OK in most cases but I could imagine situations where
Module.symvers was deleted but vmlinux kept.
So therefore the more expensive solution to run modpost twice on vmlinux
was chosen.
Sam
On Wed, 2006-08-09 at 08:29 +0200, Sam Ravnborg wrote:
> On Wed, Aug 09, 2006 at 10:32:36AM +0900, Magnus Damm wrote:
>
> > Your patch seems to work as expected if I add a return 0 at the end of
> > modpost.c:secref_whitelist(). I like how you printed out the number of
> > modules being processed.
> Thanks - fixed now. My gcc (3.4.6-r1 from Gentoo did not warn)
Interesting. I have the 3.4.6-r1 ebuild installed too, but I happened to
have the 3.3.6 profile selected by default. Which explains why things
still work as expected here.
> >I have one minor comment about your patch:
> >
> > Modpost seems to get run twice on vmlinux if the kernel is built with
> > "make all". I think it would be best to run modpost on vmlinux only when
> > vmlinux is built - never when modules are processed.
>
> Thesecond time modpost runs vmlinux is used to pick up symbol
> information to check that all symbols are valid etc.
> The alternative was to trust the symbols being read from Module.symvers
> and that would be OK in most cases but I could imagine situations where
> Module.symvers was deleted but vmlinux kept.
>
> So therefore the more expensive solution to run modpost twice on vmlinux
> was chosen.
I understand. I'm not that worried about build performance, more the
fact that all the warnings from vmlinux will get spit out twice.
Thanks,
/ magnus