CONFIG_RELOCATABLE modpost fix

PATCH In review Magnus Damm 3 participants 9 messages Aug 8, 2006 – Aug 9, 2006 Kbuild

lore

Share & tools

lore

Magnus Damm

author

2006-08-08 08:31 UTC #0 lore raw

PATCH 1/1 CONFIG_RELOCATABLE modpost fix

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 +++++++++++---

Makefile +1 -0

@@ -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,

scripts/Makefile +2 -2

@@ -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

scripts/Makefile.modpost +5 -1

@@ -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

scripts/mod/modpost.c +11 -3

@@ -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) {

Sam Ravnborg

2006-08-08 15:17 UTC #1 lore raw

Quoted 4 lines · Magnus Damm

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]>

scripts/mod/modpost.c +12 -4

index dfde0e8..5028d46 100644
@@ -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) {

Sam Ravnborg

2006-08-08 18:40 UTC #3 lore raw

In reply to Magnus Damm

Quoted 3 lines · Magnus Damm

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

Makefile +1 -0

index 72ef9f6..6b86f4e 100644
@@ -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,

scripts/Makefile +1 -1

index d531a1f..ea41de8 100644
@@ -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

scripts/Makefile.modpost +9 -3

index 0a64688..9137df2 100644
@@ -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 ;

Eric W. Biederman

2006-08-08 19:59 UTC #4 lore raw

Quoted 11 lines · Sam Ravnborg

Quoted 3 lines · Magnus Damm
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

Sam Ravnborg

2006-08-08 22:06 UTC #5 lore raw

Quoted 11 lines · Eric W. Biederman

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

Magnus Damm

author

2006-08-09 01:11 UTC #2 lore raw

In reply to Sam Ravnborg

Hi Sam,

Thanks for picking up the patch!

Quoted 42 lines · Sam Ravnborg

Quoted 4 lines · Magnus Damm
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]>
scripts/mod/modpost.c +3 -2
index dfde0e8..5028d46 100644
@@ -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)

scripts/mod/modpost.c static int secref_whitelist(const char * · line 630

Quoted 10 lines

scripts/mod/modpost.c +7 -1

+		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

Magnus Damm

author

2006-08-09 01:31 UTC #6 lore raw

In reply to Sam Ravnborg

Hi again Sam,

Quoted 11 lines · Sam Ravnborg

Quoted 3 lines · Magnus Damm
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

Sam Ravnborg

2006-08-09 06:29 UTC #7 lore raw

Quoted 3 lines · Magnus Damm

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)

Quoted 5 lines

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

Magnus Damm

author

2006-08-09 07:05 UTC #8 lore raw

Quoted 6 lines · Sam Ravnborg

Quoted 3 lines · Magnus Damm
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.

Quoted 6 lines

Quoted 5 lines
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.

Quoted 8 lines

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