[PATCH] Embed __this_module in module itself.

[PATCH] Embed __this_module in module itself.

[Rusty Russell]

Please check out this patch, which embeds the module structure into
the module itself (in ".gnu.linkonce.this_module").  Slight
simplification, and gives Dave that page back, as I promised.

Testers welcome,
Rusty.

Name: Embedded __this_module
Author: Rusty Russell
Status: Tested in 2.5.52

D: This makes __this_module a real symbol for modules, rather than
D: allocating at load time.  This makes things a little simpler, and
D: also saves about a page on architectures which have their own
D: module allocator (eg. Sparc64).

diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.52/include/linux/module.h working-2.5.52-thismodule/include/linux/module.h
--- linux-2.5.52/include/linux/module.h	Tue Dec 17 08:10:39 2002
+++ working-2.5.52-thismodule/include/linux/module.h	Mon Dec 23 16:37:53 2002
@@ -42,11 +42,6 @@ struct kernel_symbol
 
 #ifdef MODULE
 
-#ifdef KBUILD_MODNAME
-static const char __module_name[MODULE_NAME_LEN] __attribute__((section(".gnu.linkonce.modname"))) = \
-  __stringify(KBUILD_MODNAME);
-#endif
-
 /* For replacement modutils, use an alias not a pointer. */
 #define MODULE_GENERIC_TABLE(gtype,name)			\
 static const unsigned long __module_##gtype##_size		\
@@ -56,9 +51,6 @@ static const struct gtype##_id * __modul
 extern const struct gtype##_id __mod_##gtype##_table		\
   __attribute__ ((unused, alias(__stringify(name))))
 
-/* This is magically filled in by the linker, but THIS_MODULE must be
-   a constant so it works in initializers. */
-extern struct module __this_module;
 #define THIS_MODULE (&__this_module)
 
 #else  /* !MODULE */
@@ -176,7 +174,7 @@ struct module
 
 	/* The command line arguments (may be mangled).  People like
 	   keeping pointers to this stuff */
-	char args[0];
+	char *args;
 };
 
 /* FIXME: It'd be nice to isolate modules during init, too, so they
@@ -288,6 +286,11 @@ static inline const char *module_address
 	return NULL;
 }
 #endif /* CONFIG_MODULES */
+
+#ifdef MODULE
+/* This defines __this_module. */
+#include <linux/thismodule.h>
+#endif
 
 /* For archs to search exception tables */
 extern struct list_head extables;
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.52/include/linux/thismodule.h working-2.5.52-thismodule/include/linux/thismodule.h
--- linux-2.5.52/include/linux/thismodule.h	Thu Jan  1 10:00:00 1970
+++ working-2.5.52-thismodule/include/linux/thismodule.h	Mon Dec 23 16:10:04 2002
@@ -0,0 +1,19 @@
+#ifndef _LINUX_THISMODULE_H
+#define _LINUX_THISMODULE_H
+/* This stub is linked into every module. */
+
+/* These could be defined in the module, or will resolve to the kernel stubs */
+extern int __initfn(void);
+extern void __exitfn(void);
+
+/* We use the linker to do some of the work for us. */
+static struct module __this_module
+__attribute__((section(".gnu.linkonce.this_module"))) = {
+	.name = __stringify(KBUILD_MODNAME),
+	.symbols = { .owner = &__this_module },
+	.init = __initfn,
+#ifdef CONFIG_MODULE_UNLOAD
+	.exit = __exitfn,
+#endif
+};
+#endif /* MODULE */
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.52/kernel/module.c working-2.5.52-thismodule/kernel/module.c
--- linux-2.5.52/kernel/module.c	Tue Dec 17 08:11:03 2002
+++ working-2.5.52-thismodule/kernel/module.c	Mon Dec 23 16:37:14 2002
@@ -32,7 +32,7 @@
 #include <asm/pgalloc.h>
 #include <asm/cacheflush.h>
 
-#if 0
+#if 1
 #define DEBUGP printk
 #else
 #define DEBUGP(fmt , a...)
@@ -60,6 +60,13 @@ struct sizes
 	unsigned long core_size;
 };
 
+/* Stub function for modules which don't have an initfn */
+int __initfn(void)
+{
+	return 0;
+}
+EXPORT_SYMBOL(__initfn);
+
 /* Find a symbol, return value and the symbol group */
 static unsigned long __find_symbol(const char *name,
 				   struct kernel_symbol_group **group)
@@ -357,6 +364,12 @@ static inline int try_force(unsigned int
 }
 #endif /* CONFIG_MODULE_FORCE_UNLOAD */
 
+/* Stub function for modules which don't have an exitfn */
+void __exitfn(void)
+{
+}
+EXPORT_SYMBOL(__exitfn);
+
 asmlinkage long
 sys_delete_module(const char *name_user, unsigned int flags)
 {
@@ -405,7 +418,7 @@ sys_delete_module(const char *name_user,
 		}
 	}
 
-	if (!mod->exit || mod->unsafe) {
+	if (mod->exit == __exitfn || mod->unsafe) {
 		forced = try_force(flags);
 		if (!forced) {
 			/* This module can't be removed */
@@ -452,8 +465,7 @@ sys_delete_module(const char *name_user,
 
  destroy:
 	/* Final destruction now noone is using it. */
-	if (mod->exit)
-		mod->exit();
+	mod->exit();
 	free_module(mod);
 
  out:
@@ -473,7 +485,7 @@ static void print_unload_info(struct seq
 	if (mod->unsafe)
 		seq_printf(m, " [unsafe]");
 
-	if (!mod->exit)
+	if (mod->exit == __exitfn)
 		seq_printf(m, " [permanent]");
 
 	seq_printf(m, "\n");
@@ -715,6 +727,7 @@ static void free_module(struct module *m
 	module_unload_free(mod);
 
 	/* Finally, free the module structure */
+	kfree(mod->args);
 	module_free(mod, mod);
 }
 
@@ -770,27 +783,6 @@ static void *copy_section(const char *na
 	return dest;
 }
 
-/* Look for the special symbols */
-static int grab_private_symbols(Elf_Shdr *sechdrs,
-				unsigned int symbolsec,
-				const char *strtab,
-				struct module *mod)
-{
-	Elf_Sym *sym = (void *)sechdrs[symbolsec].sh_offset;
-	unsigned int i;
-
-	for (i = 1; i < sechdrs[symbolsec].sh_size/sizeof(*sym); i++) {
-		if (symbol_is("__initfn", strtab + sym[i].st_name))
-			mod->init = (void *)sym[i].st_value;
-#ifdef CONFIG_MODULE_UNLOAD
-		if (symbol_is("__exitfn", strtab + sym[i].st_name))
-			mod->exit = (void *)sym[i].st_value;
-#endif
-	}
-
-	return 0;
-}
-
 /* Deal with the given section */
 static int handle_section(const char *name,
 			  Elf_Shdr *sechdrs,
@@ -809,9 +801,6 @@ static int handle_section(const char *na
 	case SHT_RELA:
 		ret = apply_relocate_add(sechdrs, strtab, symindex, i, mod);
 		break;
-	case SHT_SYMTAB:
-		ret = grab_private_symbols(sechdrs, i, strtab, mod);
-		break;
 	default:
 		DEBUGP("Ignoring section %u: %s\n", i,
 		       sechdrs[i].sh_type==SHT_NULL ? "NULL":
@@ -919,9 +908,6 @@ static void simplify_symbols(Elf_Shdr *s
 						       strtab + sym[i].st_name,
 						       mod,
 						       &ksg);
-			/* We fake up "__this_module" */
-			if (symbol_is("__this_module", strtab+sym[i].st_name))
-				sym[i].st_value = (unsigned long)mod;
 		}
 	}
 }
@@ -963,9 +949,9 @@ static struct module *load_module(void *
 {
 	Elf_Ehdr *hdr;
 	Elf_Shdr *sechdrs;
-	char *secstrings;
+	char *secstrings, *args;
 	unsigned int i, symindex, exportindex, strindex, setupindex, exindex,
-		modnameindex, obsparmindex;
+		modindex, obsparmindex;
 	long arglen;
 	unsigned long common_length;
 	struct sizes sizes, used;
@@ -1006,7 +992,7 @@ static struct module *load_module(void *
 	exportindex = setupindex = obsparmindex = 0;
 
 	/* And these should exist, but gcc whinges if we don't init them */
-	symindex = strindex = exindex = modnameindex = 0;
+	symindex = strindex = exindex = modindex = 0;
 
 	/* Find where important sections are */
 	for (i = 1; i < hdr->e_shnum; i++) {
@@ -1015,10 +1001,10 @@ static struct module *load_module(void *
 			DEBUGP("Symbol table in section %u\n", i);
 			symindex = i;
 		} else if (strcmp(secstrings+sechdrs[i].sh_name,
-				  ".gnu.linkonce.modname") == 0) {
-			/* This module's name */
-			DEBUGP("Module name in section %u\n", i);
-			modnameindex = i;
+				  ".gnu.linkonce.this_module") == 0) {
+			/* The module struct */
+			DEBUGP("Module in section %u\n", i);
+			modindex = i;
 		} else if (strcmp(secstrings+sechdrs[i].sh_name, "__ksymtab")
 			   == 0) {
 			/* Exported symbols. */
@@ -1057,30 +1043,28 @@ static struct module *load_module(void *
 #endif
 	}
 
-	if (!modnameindex) {
-		DEBUGP("Module has no name!\n");
+	if (!modindex) {
+		printk(KERN_WARNING "No module found in object\n");
 		err = -ENOEXEC;
 		goto free_hdr;
 	}
+	mod = (void *)hdr + sechdrs[modindex].sh_offset;
 
-	/* Now allocate space for the module proper, and copy name and args. */
+	/* Now copy in args */
 	err = strlen_user(uargs);
 	if (err < 0)
 		goto free_hdr;
 	arglen = err;
 
-	mod = module_alloc(sizeof(*mod) + arglen+1);
-	if (!mod) {
+	args = kmalloc(arglen+1, GFP_KERNEL);
+	if (!args) {
 		err = -ENOMEM;
 		goto free_hdr;
 	}
-	memset(mod, 0, sizeof(*mod) + arglen+1);
-	if (copy_from_user(mod->args, uargs, arglen) != 0) {
+	if (copy_from_user(args, uargs, arglen) != 0) {
 		err = -EFAULT;
 		goto free_mod;
 	}
-	strncpy(mod->name, (char *)hdr + sechdrs[modnameindex].sh_offset,
-		sizeof(mod->name)-1);
 
 	if (find_module(mod->name)) {
 		err = -EEXIST;
@@ -1089,7 +1073,6 @@ static struct module *load_module(void *
 
 	mod->symbols.owner = mod;
 	mod->state = MODULE_STATE_COMING;
-	module_unload_init(mod);
 
 	/* How much space will we need?  (Common area in first) */
 	common_length = read_commons(hdr, &sechdrs[symindex]);
@@ -1138,6 +1121,9 @@ static struct module *load_module(void *
 			if (IS_ERR(ptr))
 				goto cleanup;
 			sechdrs[i].sh_offset = (unsigned long)ptr;
+			/* Have we just copied __this_module across? */ 
+			if (i == modindex)
+				mod = ptr;
 		} else {
 			sechdrs[i].sh_offset += (unsigned long)hdr;
 		}
@@ -1146,6 +1132,9 @@ static struct module *load_module(void *
 	if (used.init_size > mod->init_size || used.core_size > mod->core_size)
 		BUG();
 
+	/* Now we've moved module, initialize linked lists, etc. */
+	module_unload_init(mod);
+
 	/* Fix up syms, so that st_value is a pointer to location. */
 	simplify_symbols(sechdrs, symindex, strindex, mod->module_core, mod);
 
@@ -1182,6 +1171,7 @@ static struct module *load_module(void *
 	if (err < 0)
 		goto cleanup;
 
+	mod->args = args;
 	if (obsparmindex) {
 		err = obsolete_params(mod->name, mod->args,
 				      (struct obsolete_modparm *)
@@ -1214,7 +1204,7 @@ static struct module *load_module(void *
  free_core:
 	module_free(mod, mod->module_core);
  free_mod:
-	module_free(mod, mod);
+	kfree(args);
  free_hdr:
 	vfree(hdr);
 	if (err < 0) return ERR_PTR(err);

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Embed __this_module in module itself.

[David S. Miller]

   From: Rusty Russell <rusty@rustcorp.com.au>
   Date: Mon, 23 Dec 2002 19:38:41 +1100

   Please check out this patch, which embeds the module structure into
   the module itself (in ".gnu.linkonce.this_module").  Slight
   simplification, and gives Dave that page back, as I promised.

Looks like the right idea to me Rusty.

[PATCH] Embed __this_module in module itself.

[Rusty Russell]

[ Oops, forgot to CC lkml on this to Linus, and people might want to see ]

Linus, please apply.

	Rather than have the module loader the module structure and
resolve the symbols __this_module to it, make __this_module a real
structure inside the module, using the linkonce trick we used for
module names.

	This saves us an allocation (saving a page per module on
archs which need the module structure close by), and means we don't
have to fill in a few module fields.

Works fine here.  Requires the previous "Overzealous permenant mark
removed" patch, otherwise you'll get some rejects.

Thanks,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: Embedded __this_module
Author: Rusty Russell
Status: Tested in 2.5.52
Depends: Module/module-noexit.patch.gz

D: This makes __this_module a real symbol for modules, rather than
D: allocating at load time.  This makes things a little simpler, and
D: also saves about a page on architectures which have their own
D: module allocator (eg. Sparc64).

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .1411-2.5.53-embedded-thismodule.pre/include/linux/init.h .1411-2.5.53-embedded-thismodule/include/linux/init.h
--- .1411-2.5.53-embedded-thismodule.pre/include/linux/init.h	2002-12-17 08:10:38.000000000 +1100
+++ .1411-2.5.53-embedded-thismodule/include/linux/init.h	2002-12-27 21:05:37.000000000 +1100
@@ -147,14 +147,13 @@ struct obs_kernel_param {
 #define module_init(initfn)					\
 	static inline initcall_t __inittest(void)		\
 	{ return initfn; }					\
-	int __initfn(void) __attribute__((alias(#initfn)));
+	int init_module(void) __attribute__((alias(#initfn)));
 
 /* This is only required if you want to be unloadable. */
 #define module_exit(exitfn)					\
 	static inline exitcall_t __exittest(void)		\
 	{ return exitfn; }					\
-	void __exitfn(void) __attribute__((alias(#exitfn)));
-
+	void cleanup_module(void) __attribute__((alias(#exitfn)));
 
 #define __setup(str,func) /* nothing */
 #endif
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .1411-2.5.53-embedded-thismodule.pre/include/linux/module.h .1411-2.5.53-embedded-thismodule/include/linux/module.h
--- .1411-2.5.53-embedded-thismodule.pre/include/linux/module.h	2002-12-17 08:10:39.000000000 +1100
+++ .1411-2.5.53-embedded-thismodule/include/linux/module.h	2002-12-27 21:05:37.000000000 +1100
@@ -40,12 +40,11 @@ struct kernel_symbol
 	char name[MODULE_NAME_LEN];
 };
 
-#ifdef MODULE
+/* These are either module local, or the kernel's dummy ones. */
+extern int init_module(void);
+extern void cleanup_module(void);
 
-#ifdef KBUILD_MODNAME
-static const char __module_name[MODULE_NAME_LEN] __attribute__((section(".gnu.linkonce.modname"))) = \
-  __stringify(KBUILD_MODNAME);
-#endif
+#ifdef MODULE
 
 /* For replacement modutils, use an alias not a pointer. */
 #define MODULE_GENERIC_TABLE(gtype,name)			\
@@ -56,9 +55,6 @@ static const struct gtype##_id * __modul
 extern const struct gtype##_id __mod_##gtype##_table		\
   __attribute__ ((unused, alias(__stringify(name))))
 
-/* This is magically filled in by the linker, but THIS_MODULE must be
-   a constant so it works in initializers. */
-extern struct module __this_module;
 #define THIS_MODULE (&__this_module)
 
 #else  /* !MODULE */
@@ -176,7 +172,7 @@ struct module
 
 	/* The command line arguments (may be mangled).  People like
 	   keeping pointers to this stuff */
-	char args[0];
+	char *args;
 };
 
 /* FIXME: It'd be nice to isolate modules during init, too, so they
@@ -289,6 +285,19 @@ static inline const char *module_address
 }
 #endif /* CONFIG_MODULES */
 
+#if defined(MODULE) && defined(KBUILD_MODNAME)
+/* We make the linker do some of the work. */
+struct module __this_module
+__attribute__((section(".gnu.linkonce.this_module"))) = {
+	.name = __stringify(KBUILD_MODNAME),
+	.symbols = { .owner = &__this_module },
+	.init = init_module,
+#ifdef CONFIG_MODULE_UNLOAD
+	.exit = cleanup_module,
+#endif
+};
+#endif /* MODULE && KBUILD_MODNAME */
+
 /* For archs to search exception tables */
 extern struct list_head extables;
 extern spinlock_t modlist_lock;
@@ -342,13 +351,6 @@ extern int module_dummy_usage;
   && __mod_between((p),(n),(m)->module_init,(m)->init_size))	\
  || __mod_between((p),(n),(m)->module_core,(m)->core_size))
 
-/* Old-style "I'll just call it init_module and it'll be run at
-   insert".  Use module_init(myroutine) instead. */
-#ifdef MODULE
-#define init_module(voidarg) __initfn(void)
-#define cleanup_module(voidarg) __exitfn(void)
-#endif
-
 /*
  * The exception and symbol tables, and the lock
  * to protect them.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .1411-2.5.53-embedded-thismodule.pre/kernel/module.c .1411-2.5.53-embedded-thismodule/kernel/module.c
--- .1411-2.5.53-embedded-thismodule.pre/kernel/module.c	2002-12-27 21:05:37.000000000 +1100
+++ .1411-2.5.53-embedded-thismodule/kernel/module.c	2002-12-27 21:05:37.000000000 +1100
@@ -60,6 +60,13 @@ struct sizes
 	unsigned long core_size;
 };
 
+/* Stub function for modules which don't have an initfn */
+int init_module(void)
+{
+	return 0;
+}
+EXPORT_SYMBOL(init_module);
+
 /* Find a symbol, return value and the symbol group */
 static unsigned long __find_symbol(const char *name,
 				   struct kernel_symbol_group **group)
@@ -357,6 +364,12 @@ static inline int try_force(unsigned int
 }
 #endif /* CONFIG_MODULE_FORCE_UNLOAD */
 
+/* Stub function for modules which don't have an exitfn */
+void cleanup_module(void)
+{
+}
+EXPORT_SYMBOL(cleanup_module);
+
 asmlinkage long
 sys_delete_module(const char *name_user, unsigned int flags)
 {
@@ -406,7 +419,8 @@ sys_delete_module(const char *name_user,
 	}
 
 	/* If it has an init func, it must have an exit func to unload */
-	if ((mod->init && !mod->exit) || mod->unsafe) {
+	if ((mod->init != init_module && mod->exit == cleanup_module)
+	    || mod->unsafe) {
 		forced = try_force(flags);
 		if (!forced) {
 			/* This module can't be removed */
@@ -453,8 +467,7 @@ sys_delete_module(const char *name_user,
 
  destroy:
 	/* Final destruction now noone is using it. */
-	if (mod->exit)
-		mod->exit();
+	mod->exit();
 	free_module(mod);
 
  out:
@@ -474,7 +487,7 @@ static void print_unload_info(struct seq
 	if (mod->unsafe)
 		seq_printf(m, " [unsafe]");
 
-	if (mod->init && !mod->exit)
+	if (mod->init != init_module && mod->exit == cleanup_module)
 		seq_printf(m, " [permanent]");
 
 	seq_printf(m, "\n");
@@ -708,15 +721,15 @@ static void free_module(struct module *m
 	list_del(&mod->extable.list);
 	spin_unlock_irq(&modlist_lock);
 
-	/* These may be NULL, but that's OK */
-	module_free(mod, mod->module_init);
-	module_free(mod, mod->module_core);
-
 	/* Module unload stuff */
 	module_unload_free(mod);
 
-	/* Finally, free the module structure */
-	module_free(mod, mod);
+	/* This may be NULL, but that's OK */
+	module_free(mod, mod->module_init);
+	kfree(mod->args);
+
+	/* Finally, free the core (containing the module structure) */
+	module_free(mod, mod->module_core);
 }
 
 void *__symbol_get(const char *symbol)
@@ -771,27 +784,6 @@ static void *copy_section(const char *na
 	return dest;
 }
 
-/* Look for the special symbols */
-static int grab_private_symbols(Elf_Shdr *sechdrs,
-				unsigned int symbolsec,
-				const char *strtab,
-				struct module *mod)
-{
-	Elf_Sym *sym = (void *)sechdrs[symbolsec].sh_offset;
-	unsigned int i;
-
-	for (i = 1; i < sechdrs[symbolsec].sh_size/sizeof(*sym); i++) {
-		if (symbol_is("__initfn", strtab + sym[i].st_name))
-			mod->init = (void *)sym[i].st_value;
-#ifdef CONFIG_MODULE_UNLOAD
-		if (symbol_is("__exitfn", strtab + sym[i].st_name))
-			mod->exit = (void *)sym[i].st_value;
-#endif
-	}
-
-	return 0;
-}
-
 /* Deal with the given section */
 static int handle_section(const char *name,
 			  Elf_Shdr *sechdrs,
@@ -810,9 +802,6 @@ static int handle_section(const char *na
 	case SHT_RELA:
 		ret = apply_relocate_add(sechdrs, strtab, symindex, i, mod);
 		break;
-	case SHT_SYMTAB:
-		ret = grab_private_symbols(sechdrs, i, strtab, mod);
-		break;
 	default:
 		DEBUGP("Ignoring section %u: %s\n", i,
 		       sechdrs[i].sh_type==SHT_NULL ? "NULL":
@@ -920,9 +909,6 @@ static void simplify_symbols(Elf_Shdr *s
 						       strtab + sym[i].st_name,
 						       mod,
 						       &ksg);
-			/* We fake up "__this_module" */
-			if (symbol_is("__this_module", strtab+sym[i].st_name))
-				sym[i].st_value = (unsigned long)mod;
 		}
 	}
 }
@@ -964,9 +950,9 @@ static struct module *load_module(void *
 {
 	Elf_Ehdr *hdr;
 	Elf_Shdr *sechdrs;
-	char *secstrings;
+	char *secstrings, *args;
 	unsigned int i, symindex, exportindex, strindex, setupindex, exindex,
-		modnameindex, obsparmindex;
+		modindex, obsparmindex;
 	long arglen;
 	unsigned long common_length;
 	struct sizes sizes, used;
@@ -1007,7 +993,7 @@ static struct module *load_module(void *
 	exportindex = setupindex = obsparmindex = 0;
 
 	/* And these should exist, but gcc whinges if we don't init them */
-	symindex = strindex = exindex = modnameindex = 0;
+	symindex = strindex = exindex = modindex = 0;
 
 	/* Find where important sections are */
 	for (i = 1; i < hdr->e_shnum; i++) {
@@ -1016,10 +1002,10 @@ static struct module *load_module(void *
 			DEBUGP("Symbol table in section %u\n", i);
 			symindex = i;
 		} else if (strcmp(secstrings+sechdrs[i].sh_name,
-				  ".gnu.linkonce.modname") == 0) {
-			/* This module's name */
-			DEBUGP("Module name in section %u\n", i);
-			modnameindex = i;
+				  ".gnu.linkonce.this_module") == 0) {
+			/* The module struct */
+			DEBUGP("Module in section %u\n", i);
+			modindex = i;
 		} else if (strcmp(secstrings+sechdrs[i].sh_name, "__ksymtab")
 			   == 0) {
 			/* Exported symbols. */
@@ -1058,39 +1044,35 @@ static struct module *load_module(void *
 #endif
 	}
 
-	if (!modnameindex) {
-		DEBUGP("Module has no name!\n");
+	if (!modindex) {
+		printk(KERN_WARNING "No module found in object\n");
 		err = -ENOEXEC;
 		goto free_hdr;
 	}
+	mod = (void *)hdr + sechdrs[modindex].sh_offset;
 
-	/* Now allocate space for the module proper, and copy name and args. */
+	/* Now copy in args */
 	err = strlen_user(uargs);
 	if (err < 0)
 		goto free_hdr;
 	arglen = err;
 
-	mod = module_alloc(sizeof(*mod) + arglen+1);
-	if (!mod) {
+	args = kmalloc(arglen+1, GFP_KERNEL);
+	if (!args) {
 		err = -ENOMEM;
 		goto free_hdr;
 	}
-	memset(mod, 0, sizeof(*mod) + arglen+1);
-	if (copy_from_user(mod->args, uargs, arglen) != 0) {
+	if (copy_from_user(args, uargs, arglen+1) != 0) {
 		err = -EFAULT;
 		goto free_mod;
 	}
-	strncpy(mod->name, (char *)hdr + sechdrs[modnameindex].sh_offset,
-		sizeof(mod->name)-1);
 
 	if (find_module(mod->name)) {
 		err = -EEXIST;
 		goto free_mod;
 	}
 
-	mod->symbols.owner = mod;
 	mod->state = MODULE_STATE_COMING;
-	module_unload_init(mod);
 
 	/* How much space will we need?  (Common area in first) */
 	common_length = read_commons(hdr, &sechdrs[symindex]);
@@ -1139,6 +1121,9 @@ static struct module *load_module(void *
 			if (IS_ERR(ptr))
 				goto cleanup;
 			sechdrs[i].sh_offset = (unsigned long)ptr;
+			/* Have we just copied __this_module across? */ 
+			if (i == modindex)
+				mod = ptr;
 		} else {
 			sechdrs[i].sh_offset += (unsigned long)hdr;
 		}
@@ -1147,6 +1132,9 @@ static struct module *load_module(void *
 	if (used.init_size > mod->init_size || used.core_size > mod->core_size)
 		BUG();
 
+	/* Now we've moved module, initialize linked lists, etc. */
+	module_unload_init(mod);
+
 	/* Fix up syms, so that st_value is a pointer to location. */
 	simplify_symbols(sechdrs, symindex, strindex, mod->module_core, mod);
 
@@ -1183,6 +1171,7 @@ static struct module *load_module(void *
 	if (err < 0)
 		goto cleanup;
 
+	mod->args = args;
 	if (obsparmindex) {
 		err = obsolete_params(mod->name, mod->args,
 				      (struct obsolete_modparm *)
@@ -1215,7 +1204,7 @@ static struct module *load_module(void *
  free_core:
 	module_free(mod, mod->module_core);
  free_mod:
-	module_free(mod, mod);
+	kfree(args);
  free_hdr:
 	vfree(hdr);
 	if (err < 0) return ERR_PTR(err);
@@ -1266,7 +1255,7 @@ sys_init_module(void *umod,
 	up(&module_mutex);
 
 	/* Start the module */
-	ret = mod->init ? mod->init() : 0;
+	ret = mod->init();
 	if (ret < 0) {
 		/* Init routine failed: abort.  Try to protect us from
                    buggy refcounters. */

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Embed __this_module in module itself.

[Miles Bader]

When I try to build modules using 2.5.54, the resulting .ko files lack
the .gnu.linkonce.* sections, which causes the kernel module loader to
fail on them -- those sections _are_ present in the .o files, but the
linker apparently removes them!

Any clues why it's doing this (I tried GNU ld versions 2.13.2.1 and
2.11.92.0.12.3, and both act the same)?  Is there something
arch-specific I need to do?

[The .ko file is produce by just doing `ld -r', e.g.:

   v850e-elf-ld   -r -o drivers/block/loop.ko drivers/block/loop.o

For instance, for the loop device, here's the output of `readelf -S loop.o':

---------------- begin ----------------
There are 25 section headers, starting at offset 0x1d99c:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        00000000 000034 00186c 00  AX  0   0  2
  [ 2] .rela.text        RELA            00000000 01ea40 0005c4 0c     23   1  4
  [ 3] .data             PROGBITS        00000000 0018a0 0000a4 00  WA  0   0  4
  [ 4] .rela.data        RELA            00000000 01f004 000078 0c     23   3  4
  [ 5] .bss              NOBITS          00000000 001944 000008 00  WA  0   0  4
  [ 6] .call_table_data  PROGBITS        00000000 001944 000000 00  WA  0   0  1
  [ 7] .call_table_text  PROGBITS        00000000 001944 000000 00  AX  0   0  1
  [ 8] .stab             PROGBITS        00000000 001944 008538 0c     10   0  4
  [ 9] .rela.stab        RELA            00000000 01f07c 000b10 0c     23   8  4
  [10] .stabstr          STRTAB          00000000 009e7c 013448 00      0   0  1
  [11] .plt              NOBITS          00000000 01d2c8 000000 00  AX  0   0  8
  [12] .init.plt         NOBITS          00000000 01d2c8 000000 00  AX  0   0  8
  [13] .gnu.linkonce.thi PROGBITS        00000000 01d2c8 0000b0 00  WA  0   0  4
  [14] .rela.gnu.linkonc RELA            00000000 01fb8c 000024 0c     23   d  4
  [15] .rodata           PROGBITS        00000000 01d378 0001bf 00   A  0   0  1
  [16] __obsparm         PROGBITS        00000000 01d538 000080 00  WA  0   0  4
  [17] __ksymtab         PROGBITS        00000000 01d5b8 000080 00   A  0   0  4
  [18] .rela__ksymtab    RELA            00000000 01fbb0 000018 0c     23  11  4
  [19] .init.text        PROGBITS        00000000 01d638 000220 00  AX  0   0  2
  [20] .rela.init.text   RELA            00000000 01fbc8 000138 0c     23  13  4
  [21] .comment          PROGBITS        00000000 01d858 00002d 00      0   0  1
  [22] .shstrtab         STRTAB          00000000 01d885 000115 00      0   0  1
  [23] .symtab           SYMTAB          00000000 01dd84 000710 10     24  34  4
  [24] .strtab           STRTAB          00000000 01e494 0005ab 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)
----------------  end  ----------------


and here's the output of `readelf -S loop.ko'

---------------- begin ----------------
There are 23 section headers, starting at offset 0x1d938:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .plt              NOBITS          00100000 000038 000000 00  AX  0   0  8
  [ 2] .text             PROGBITS        00100000 000038 00191c 00 WAX  0   0  4
  [ 3] .rela.text        RELA            00000000 01dcd0 0005e8 0c     21   2  4
  [ 4] .call_table_data  PROGBITS        0010191c 001954 000000 00  WA  0   0  1
  [ 5] .call_table_text  PROGBITS        0010191c 001954 000000 00  AX  0   0  1
  [ 6] .rodata           PROGBITS        0010191c 001954 0001bf 00   A  0   0  1
  [ 7] .data             PROGBITS        00101adc 001b14 0000a4 00  WA  0   0  4
  [ 8] .rela.data        RELA            00000000 01e2b8 000078 0c     21   7  4
  [ 9] .bss              NOBITS          00101b80 001bb8 000008 00  WA  0   0  4
  [10] .stab             PROGBITS        00000000 001bb8 008538 0c     12   0  4
  [11] .rela.stab        RELA            00000000 01e330 000b10 0c     21   a  4
  [12] .stabstr          STRTAB          00000000 00a0f0 013448 00      0   0  1
  [13] .comment          PROGBITS        00000000 01d538 00002d 00      0   0  1
  [14] .init.plt         NOBITS          00000030 01d568 000000 00  AX  0   0  8
  [15] __obsparm         PROGBITS        00000030 01d568 000080 00  WA  0   0  4
  [16] __ksymtab         PROGBITS        000000b0 01d5e8 000080 00   A  0   0  4
  [17] .rela__ksymtab    RELA            00000000 01ee40 000018 0c     21  10  4
  [18] .init.text        PROGBITS        00000130 01d668 000220 00  AX  0   0  2
  [19] .rela.init.text   RELA            00000000 01ee58 000138 0c     21  12  4
  [20] .shstrtab         STRTAB          00000000 01d888 0000b0 00      0   0  1
  [21] .symtab           SYMTAB          00000000 01ef90 000780 10     22  3b  4
  [22] .strtab           STRTAB          00000000 01f710 0005ab 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)
----------------  end  ----------------


Thanks,

-Miles
-- 
Somebody has to do something, and it's just incredibly pathetic that it
has to be us.  -- Jerry Garcia

Re: [PATCH] Embed __this_module in module itself.

[Miles Bader]

Miles Bader <miles@lsi.nec.co.jp> writes:
> When I try to build modules using 2.5.54, the resulting .ko files lack
> the .gnu.linkonce.* sections, which causes the kernel module loader to
> fail on them -- those sections _are_ present in the .o files, but the
> linker apparently removes them!

Ok, I found out why this is happening -- the v850 default linker
scripts, for whatever reason, merge any section called `.gnu.linker.t*'
with .text.

I can prevent this by adding the option `--unique=.gnu.linkonce.this_module'
to the linker flags (specifically, to LDFLAGS_MODULE in the top-level
Makefile).  I suppose another way to do it would be to rename the
section something that doesn't match `.gnu.linker.t*'.

What's the right way to handle this?

[from perusing the ld srcs, a few other archs seem to have the same
`feature,' though the only that I think has linux support is `cris']

Thanks,

-Miles
-- 
The secret to creativity is knowing how to hide your sources.
  --Albert Einstein

Re: [PATCH] Embed __this_module in module itself.

[Rusty Russell]

In message <buoadid1pxl.fsf@mcspd15.ucom.lsi.nec.co.jp> you write:
> Miles Bader <miles@lsi.nec.co.jp> writes:
> > When I try to build modules using 2.5.54, the resulting .ko files lack
> > the .gnu.linkonce.* sections, which causes the kernel module loader to
> > fail on them -- those sections _are_ present in the .o files, but the
> > linker apparently removes them!
> 
> Ok, I found out why this is happening -- the v850 default linker
> scripts, for whatever reason, merge any section called `.gnu.linker.t*'
> with .text.

That's about as wierdass as it comes.

> I can prevent this by adding the option `--unique=.gnu.linkonce.this_module'
> to the linker flags (specifically, to LDFLAGS_MODULE in the top-level
> Makefile).  I suppose another way to do it would be to rename the
> section something that doesn't match `.gnu.linker.t*'.

Or you could add this flag in arch/v850/Makefile.

> What's the right way to handle this?
> 
> [from perusing the ld srcs, a few other archs seem to have the same
> `feature,' though the only that I think has linux support is `cris']

I think:
	LDFLAGS_MODULE += -T arch/v850/module.lds

And include a linker script there which works.

Thanks for finding this...
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Embed __this_module in module itself.

[Sam Ravnborg]

On Tue, Jan 07, 2003 at 02:36:54PM +0900, Miles Bader wrote:
> Miles Bader <miles@lsi.nec.co.jp> writes:
> > When I try to build modules using 2.5.54, the resulting .ko files lack
> > the .gnu.linkonce.* sections, which causes the kernel module loader to
> > fail on them -- those sections _are_ present in the .o files, but the
> > linker apparently removes them!
> 
> Ok, I found out why this is happening -- the v850 default linker
> scripts, for whatever reason, merge any section called `.gnu.linker.t*'
> with .text.
ld per default uses a default linker-scripts as you note.
Could another solution be to provide a fixed linker script, used for all
invocations of ld?

LDFLAGS += -T arch/v850/v850.lds

Not knowing much about v850, I wonder why you do not need to set the -m
option. Most other architectures do this.

	Sam

Re: [PATCH] Embed __this_module in module itself.

[Miles Bader]

Sam Ravnborg <sam@ravnborg.org> writes:
> Not knowing much about v850, I wonder why you do not need to set the -m
> option. Most other architectures do this.

???

A far as I can see, no architecture does anything different than the
default.

[Why on earth would -m be needed, anyway?]

-Miles
-- 
Fast, small, soon; pick any 2.

Re: [PATCH] Embed __this_module in module itself.

[Sam Ravnborg]

On Thu, Jan 09, 2003 at 10:18:53AM +0900, Miles Bader wrote:
> Sam Ravnborg <sam@ravnborg.org> writes:
> > Not knowing much about v850, I wonder why you do not need to set the -m
> > option. Most other architectures do this.
> 
> ???
> 
> A far as I can see, no architecture does anything different than the
> default.

A little grepping gave the following result:

i386/Makefile:LDFLAGS           := -m elf_i386
m68k/Makefile:LDFLAGS := -m m68kelf
mips/Makefile:LDFLAGS           := -G 0
ppc64/Makefile:LDFLAGS          := -m elf64ppc
s390/Makefile:LDFLAGS           := -m elf_s390
s390x/Makefile:LDFLAGS          := -m elf64_s390
sh/Makefile:LDFLAGS             := -EL
sh/Makefile:LDFLAGS             := -EB
sparc/Makefile:LDFLAGS          := -m elf32_sparc
sparc64/Makefile:LDFLAGS                := -m elf64_sparc
x86_64/Makefile:LDFLAGS         := -m elf_x86_64

Little less than half of the architectures defines their own LDFLAGS.
Most of them set an emulation, most probarly inherited from i386.

> 
> [Why on earth would -m be needed, anyway?]

I do not know, but as can be seen above several architectures use it.


I have seen your proposed patch for gnu.linkonce.
I do prefer to have it in arch/v850/Makefile because this is a workaround
for an architecture specific bug in ld.
Why not provide your own link script?

	Sam

Re: [PATCH] Embed __this_module in module itself.

[Richard B. Johnson]

On Thu, 9 Jan 2003, Sam Ravnborg wrote:

> On Thu, Jan 09, 2003 at 10:18:53AM +0900, Miles Bader wrote:
> > Sam Ravnborg <sam@ravnborg.org> writes:
> > > Not knowing much about v850, I wonder why you do not need to set the -m
> > > option. Most other architectures do this.
> > 
> > ???
> > 
> > A far as I can see, no architecture does anything different than the
> > default.
> 
> A little grepping gave the following result:
> 
> i386/Makefile:LDFLAGS           := -m elf_i386
> m68k/Makefile:LDFLAGS := -m m68kelf
> mips/Makefile:LDFLAGS           := -G 0
> ppc64/Makefile:LDFLAGS          := -m elf64ppc
> s390/Makefile:LDFLAGS           := -m elf_s390
> s390x/Makefile:LDFLAGS          := -m elf64_s390
> sh/Makefile:LDFLAGS             := -EL
> sh/Makefile:LDFLAGS             := -EB
> sparc/Makefile:LDFLAGS          := -m elf32_sparc
> sparc64/Makefile:LDFLAGS                := -m elf64_sparc
> x86_64/Makefile:LDFLAGS         := -m elf_x86_64
> 
> Little less than half of the architectures defines their own LDFLAGS.
> Most of them set an emulation, most probarly inherited from i386.
> 
> > 
> > [Why on earth would -m be needed, anyway?]
> 
> I do not know, but as can be seen above several architectures use it.
> 
> 
> I have seen your proposed patch for gnu.linkonce.
> I do prefer to have it in arch/v850/Makefile because this is a workaround
> for an architecture specific bug in ld.
> Why not provide your own link script?
> 
> 	Sam

The 'emulation' really defines the linker-script to use. The name
is pretty much a misnomer. Different architectures require different
linking parameters and you can force the linker to provide a `a.out`
(ZMAGIC) output as well as ELF.

Execute  `ld -V` and the linker script will be displayed. If you
do `ld -V -m x` it will show the emulations provided, on ix86
machines, it's i386linux (for a.out) and elf_i386 (for ELF).

Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.