[PATCH] also discard .*init sections after module initialization

[PATCH] also discard .*init sections after module initialization

[Jan Beulich]

Just like .init, these sections are supposed to be unneeded after init,
and modpost warns about improper section references anyway. Likewise
for .*exit which, other than .exit, aren't needed for the module unload
path.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 arch/ia64/kernel/module.c       |    3 ++-
 arch/powerpc/kernel/module_32.c |   14 ++++++++------
 arch/powerpc/kernel/module_64.c |    5 ++---
 include/linux/module.h          |    2 ++
 kernel/module.c                 |   37 +++++++++++++++++++++++++++++++++++--
 5 files changed, 49 insertions(+), 12 deletions(-)

--- linux-2.6.31-rc1/arch/ia64/kernel/module.c	2009-06-26 17:49:37.000000000 +0200
+++ 2.6.31-rc1-module-discard-xxxinit/arch/ia64/kernel/module.c	2009-04-27 12:06:09.000000000 +0200
@@ -473,7 +473,8 @@ module_frob_arch_sections (Elf_Ehdr *ehd
 
 		gots += count_gots(rels, numrels);
 		fdescs += count_fdescs(rels, numrels);
-		if (strstr(secstrings + s->sh_name, ".init"))
+		if (is_init_section_name(secstrings
+					 + sechdrs[s->sh_info].sh_name))
 			init_plts += count_plts(rels, numrels);
 		else
 			core_plts += count_plts(rels, numrels);
--- linux-2.6.31-rc1/arch/powerpc/kernel/module_32.c	2009-03-24 00:12:14.000000000 +0100
+++ 2.6.31-rc1-module-discard-xxxinit/arch/powerpc/kernel/module_32.c	2009-04-27 12:06:09.000000000 +0200
@@ -111,17 +111,19 @@ static unsigned long get_plt_size(const 
 	/* Everything marked ALLOC (this includes the exported
            symbols) */
 	for (i = 1; i < hdr->e_shnum; i++) {
-		/* If it's called *.init*, and we're not init, we're
-                   not interested */
-		if ((strstr(secstrings + sechdrs[i].sh_name, ".init") != 0)
-		    != is_init)
-			continue;
-
 		/* We don't want to look at debug sections. */
 		if (strstr(secstrings + sechdrs[i].sh_name, ".debug") != 0)
 			continue;
 
 		if (sechdrs[i].sh_type == SHT_RELA) {
+			/* If it's .init-like, and we're not init, we're
+			   not interested */
+			if (is_init_section_name(secstrings
+						 + sechdrs[sechdrs[i].sh_info]
+						   .sh_name)
+			    != is_init)
+				continue;
+
 			DEBUGP("Found relocations in section %u\n", i);
 			DEBUGP("Ptr: %p.  Number: %u\n",
 			       (void *)hdr + sechdrs[i].sh_offset,
--- linux-2.6.31-rc1/arch/powerpc/kernel/module_64.c	2009-06-10 05:05:27.000000000 +0200
+++ 2.6.31-rc1-module-discard-xxxinit/arch/powerpc/kernel/module_64.c	2009-04-27 12:06:09.000000000 +0200
@@ -206,7 +206,6 @@ int module_frob_arch_sections(Elf64_Ehdr
 
 	/* Find .toc and .stubs sections, symtab and strtab */
 	for (i = 1; i < hdr->e_shnum; i++) {
-		char *p;
 		if (strcmp(secstrings + sechdrs[i].sh_name, ".stubs") == 0)
 			me->arch.stubs_section = i;
 		else if (strcmp(secstrings + sechdrs[i].sh_name, ".toc") == 0)
@@ -216,8 +215,8 @@ int module_frob_arch_sections(Elf64_Ehdr
 					  sechdrs[i].sh_size);
 
 		/* We don't handle .init for the moment: rename to _init */
-		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
-			p[0] = '_';
+		if (is_init_section_name(secstrings + sechdrs[i].sh_name))
+			secstrings[sechdrs[i].sh_name] = '_';
 
 		if (sechdrs[i].sh_type == SHT_SYMTAB)
 			dedotify((void *)hdr + sechdrs[i].sh_offset,
--- linux-2.6.31-rc1/include/linux/module.h	2009-06-26 17:50:00.000000000 +0200
+++ 2.6.31-rc1-module-discard-xxxinit/include/linux/module.h	2009-04-27 12:07:17.000000000 +0200
@@ -401,6 +401,8 @@ static inline int within_module_init(uns
 	       addr < (unsigned long)mod->module_init + mod->init_size;
 }
 
+bool is_init_section_name(const char *);
+
 /* Search for module by name: must hold module_mutex. */
 struct module *find_module(const char *name);
 
--- linux-2.6.31-rc1/kernel/module.c	2009-06-26 17:50:00.000000000 +0200
+++ 2.6.31-rc1-module-discard-xxxinit/kernel/module.c	2009-04-27 12:11:51.000000000 +0200
@@ -1655,6 +1655,39 @@ static long get_offset(struct module *mo
 	return ret;
 }
 
+bool is_init_section_name(const char *name)
+{
+	unsigned int i, n;
+	static const char *const prefixes[] = {
+#ifndef CONFIG_HOTPLUG_CPU
+		"cpu",
+#endif
+#ifndef CONFIG_HOTPLUG
+		"dev",
+#endif
+#ifndef CONFIG_MEMORY_HOTPLUG
+		"mem",
+#endif
+		""
+	};
+
+	if (*name++ != '.')
+		return false;
+	for (i = 0; i < ARRAY_SIZE(prefixes); ++i) {
+		n = strlen(prefixes[i]);
+		if (strncmp(name, prefixes[i], n) == 0)
+			break;
+	}
+	if (i >= ARRAY_SIZE(prefixes))
+		return false;
+#ifdef CONFIG_MODULE_UNLOAD
+	if (n == 0 && strncmp(name, "exit", 4) == 0)
+		return false;
+#endif
+	return strncmp(name + n, "init", 4) == 0
+	       || strncmp(name + n, "exit", 4) == 0;
+}
+
 /* Lay out the SHF_ALLOC sections in a way not dissimilar to how ld
    might -- code, read-only data, read-write data, small data.  Tally
    sizes, and place the offsets into sh_entsize fields: high bit means it
@@ -1686,7 +1719,7 @@ static void layout_sections(struct modul
 			if ((s->sh_flags & masks[m][0]) != masks[m][0]
 			    || (s->sh_flags & masks[m][1])
 			    || s->sh_entsize != ~0UL
-			    || strstarts(secstrings + s->sh_name, ".init"))
+			    || is_init_section_name(secstrings + s->sh_name))
 				continue;
 			s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
 			DEBUGP("\t%s\n", secstrings + s->sh_name);
@@ -1703,7 +1736,7 @@ static void layout_sections(struct modul
 			if ((s->sh_flags & masks[m][0]) != masks[m][0]
 			    || (s->sh_flags & masks[m][1])
 			    || s->sh_entsize != ~0UL
-			    || !strstarts(secstrings + s->sh_name, ".init"))
+			    || !is_init_section_name(secstrings + s->sh_name))
 				continue;
 			s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
 					 | INIT_OFFSET_MASK);

Re: [PATCH] also discard .*init sections after module initialization

[Rusty Russell]

On Tue, 30 Jun 2009 09:27:26 pm Jan Beulich wrote:
> Just like .init, these sections are supposed to be unneeded after init,
> and modpost warns about improper section references anyway.

Tim Abbot had a section renaming patch, for -ffunction-sections.  Can we kill 
two birds with one stone and have a unique string for init sections, and 
another for exit sections?

> Likewise
> for .*exit which, other than .exit, aren't needed for the module unload
> path.

Currently true, but I can't see that being true in general.

If we're going to abstract init detection, it would be good to wrap init 
section renaming (for powerpc64 at least) too.

Thanks,
Rusty.

Re: [PATCH] also discard .*init sections after module initialization

[Jan Beulich]

>>> Rusty Russell <rusty@rustcorp.com.au> 01.07.09 07:22 >>>
>On Tue, 30 Jun 2009 09:27:26 pm Jan Beulich wrote:
>> Just like .init, these sections are supposed to be unneeded after init,
>> and modpost warns about improper section references anyway.
>
>Tim Abbot had a section renaming patch, for -ffunction-sections.  Can we kill 
>two birds with one stone and have a unique string for init sections, and 
>another for exit sections?

Not sure, would need to see what that patch does first. And then, the patch
here doesn't do any renaming.

>> Likewise
>> for .*exit which, other than .exit, aren't needed for the module unload
>> path.
>
>Currently true, but I can't see that being true in general.

Why? .*exit sections deal with device/memory/CPU hot remove, which is
orthogonal to module unload.

Jan

Re: [PATCH] also discard .*init sections after module initialization

[Tim Abbott]

On Wed, 1 Jul 2009, Jan Beulich wrote:

> >>> Rusty Russell <rusty@rustcorp.com.au> 01.07.09 07:22 >>>
> >On Tue, 30 Jun 2009 09:27:26 pm Jan Beulich wrote:
> >> Just like .init, these sections are supposed to be unneeded after init,
> >> and modpost warns about improper section references anyway.
> >
> >Tim Abbot had a section renaming patch, for -ffunction-sections.  Can we kill 
> >two birds with one stone and have a unique string for init sections, and 
> >another for exit sections?
> 
> Not sure, would need to see what that patch does first. And then, the patch
> here doesn't do any renaming.

We don't need to rename the .{dev,cpu,mem}init.* sections for 
-ffunction-sections -fdata-sections; the only things that need to be 
renamed for that are sections with names that start with ".text.", 
".data.", ".bss.", and ".rodata." since those are the section names 
generated by -ffunction-sections -fdata-sections.  So I think that this 
change is orthogonal to my efforts.

> >> Likewise
> >> for .*exit which, other than .exit, aren't needed for the module unload
> >> path.
> >
> >Currently true, but I can't see that being true in general.
> 
> Why? .*exit sections deal with device/memory/CPU hot remove, which is
> orthogonal to module unload.

For the core kernel, the .devinit sections are currently being squashed 
into either the .data section or the .init.data section depending on 
whether hotplug is enabled using the vmlinux linker scripts.  Can we use 
the same approach for modules (i.e. have a module linker script that does 
this)?  It seems that would avoid having two duplicate mechanisms for 
doing the same thing, one in the module loader and one in the core 
kernel's linker scripts.

	-Tim Abbott

Re: [PATCH] also discard .*init sections after module initialization

[Jan Beulich]

>>> Tim Abbott <tabbott@ksplice.com> 06.07.09 17:55 >>>
>On Wed, 1 Jul 2009, Jan Beulich wrote:
>> Why? .*exit sections deal with device/memory/CPU hot remove, which is
>> orthogonal to module unload.
>
>For the core kernel, the .devinit sections are currently being squashed 
>into either the .data section or the .init.data section depending on 
>whether hotplug is enabled using the vmlinux linker scripts.  Can we use 
>the same approach for modules (i.e. have a module linker script that does 
>this)?  It seems that would avoid having two duplicate mechanisms for 
>doing the same thing, one in the module loader and one in the core 
>kernel's linker scripts.

I'd have to play with this a little, as I'm not sure ld -r can actually do what
you're suggesting.

Jan

Re: [PATCH] also discard .*init sections after module initialization

[Amerigo Wang]

On Tue, Jun 30, 2009 at 12:57:26PM +0100, Jan Beulich wrote:
>Just like .init, these sections are supposed to be unneeded after init,
>and modpost warns about improper section references anyway. Likewise
>for .*exit which, other than .exit, aren't needed for the module unload
>path.
>

<snip>

> 
>+bool is_init_section_name(const char *name)
>+{


According to your description, this function name
looks confusing... at least for me.