mii module broken under new scheme
Subsystems (3)
PATCH 1/1 mii module broken under new scheme
drivers/net/mii.c doesn't export module init/cleanup functions. That means it can't be loaded under the new module scheme. This patch adds do-nothing functions for it, which allows it to load. (8139too depends on mii, so without this I don't have network.)
Matt
net/mii.c renamed from net/mii.c~no +16 -0
@@ -348,3 +348,19 @@EXPORT_SYMBOL (mii_check_media);EXPORT_SYMBOL (generic_mii_ioctl); +static void __init mii_init_module (void ) +{ +#ifdef MODULE +printk (KERN_INFO "mii" "\n" ); +#endif + +return 0 ; +} + + +static void __exit mii_cleanup_module (void) +{ +} + +module_init (mii_init_module); +module_exit (mii_cleanup_module);
Quoted 4 lines · Matt Reppert
drivers/net/mii.cdoesn't export module init/cleanup functions. That means it can't be loaded under the new module scheme. This patch adds do-nothing functions for it, which allows it to load. (8139too depends on mii, so without this I don't have network.)
ahhh! I was wondering what was up, but since I was busy with other things I just compiled it into the kernel and continued on my way.
That's a bug in the new module loader.
Jeff
Quoted 8 lines · On Tue, 19 Nov 2002 12:51:44 -0500 Jeff Garzik <[email protected]> wrote:
Quoted 5 lines · Matt Reppert
drivers/net/mii.cdoesn't export module init/cleanup functions. That means it
can't be loaded under the new module scheme. This patch adds do-nothing functions for it, which allows it to load. (8139too depends on mii, so without this I don't have network.)
Quoted 4 lines
ahhh! I was wondering what was up, but since I was busy with other
things I just compiled it into the kernel and continued on my way.That's a bug in the new module loader.
Not so sure I agree ... recompiled the kernel with debugging output in module.c and when I try to insert mii.o without above patch it complains "Module has no name!" and returns -ENOEXEC from the syscall. I think naming mii.o would be a good idea. This may not be the best way to do it, but it works. (Granted, I'm not terribly familiar with all the modules code changes yet, but ... ) Having anonymous output in lsmod would be somewhat confusing :) ("Well, whatever it is, 8139too needs it, don't touch it!")
Just noticed above patch is -p0. Whoops.
Matt
In message <[email protected]> you write:
Quoted 8 lines
Quoted 5 lines · Matt Reppert
drivers/net/mii.cdoesn't export module init/cleanup functions. That means it
can't be loaded under the new module scheme. This patch adds do-nothing functions for it, which allows it to load. (8139too depends on mii, so without this I don't have network.)
Quoted 4 lines
ahhh! I was wondering what was up, but since I was busy with other
things I just compiled it into the kernel and continued on my way.That's a bug in the new module loader.
Yes. But the workaround of calling the module "unknown" isn't nice
either: just put "no_module_init;" in and be done with it (it's also a big hint that the module doesn't do anything itself).
Richard Henderson, Kai and myself are discussing a post-link stage for modules, which will allow us to add the .modname section at that time, but 99% of the cases are already fixed.
Sorry for any trouble,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 17 lines · Matt Reppert
Quoted 11 lines · On Tue, 19 Nov 2002 12:51:44 -0500 Jeff Garzik wrote:
Quoted 2 lines · Matt Reppert
drivers/net/mii.cdoesn't export module init/cleanup functions. That means itahhh! I was wondering what was up, but since I was busy with other
things I just compiled it into the kernel and continued on my way.That's a bug in the new module loader.
Quoted 8 lines
Not so sure I agree ... recompiled the kernel with debugging output in
module.cand when I try to insert mii.o without above patch it complains "Module has no name!" and returns-ENOEXECfrom the syscall. I think naming mii.o would be a good idea. This may not be the best way to do it, but it works. (Granted, I'm not terribly familiar with all the modules code changes yet, but ... ) Having anonymous output in lsmod would be somewhat confusing :) ("Well, whatever it is, 8139too needs it, don't touch it!")
Your patch is fine to get people going... I'm glad you posted it on lkml for others to benefit.
However, for inclusion in the kernel, it is not needed. Quite simply, mii.c is essentially a library, and it needs absolutely no initialization nor cleanup at all. Thus, it is a bug in the module loader that any code changes at all are required.
There exists a no_module_init tag, which is in theory the proper fix for this problem under rusty's system, but that is itself a bug: it's redundant just like the silly EXPORT_NO_SYMBOLS tag -- it's stating the obvious. The module loader needs to notice a lack of init_module and exit_module and handle it accordingly.
Jeff
Quoted 4 lines · Jeff Garzik
However, for inclusion in the kernel, it is not needed. Quite simply,
mii.cis essentially a library, and it needs absolutely no initialization nor cleanup at all. Thus, it is a bug in the module loader that any code changes at all are required.
I'm afraid it's not.
Quoted 5 lines
There exists a
no_module_inittag, which is in theory the proper fix for this problem under rusty's system, but that is itself a bug: it's redundant just like the sillyEXPORT_NO_SYMBOLStag -- it's stating the obvious. The module loader needs to notice a lack ofinit_moduleandexit_moduleand handle it accordingly.
It does.
I reread the discussion on this, and I still don't see what great
benefit the new loader brings, or why it was allowed to break all the existing modules.
A software interface is a contract between caller and service. As long
as it doesn't change you can rewrite either side of it without breaking the other. That reduces maintenance and is something I thought was standard software engineering, best practices, etc.
Signature · 3 lines
bill davidsen <[email protected]> CTO, TMR Associates, Inc Doing interesting things with little computers since 1979.
Quoted 4 lines · Matt Reppert
drivers/net/mii.cdoesn't export module init/cleanup functions. That means it can't be loaded under the new module scheme. This patch adds do-nothing functions for it, which allows it to load. (8139too depends on mii, so without this I don't have network.)
The bug is in the module layer. This isn't the right fix at all. When the module layer is fixed to behave sanely the problem will go away
In message <[email protected]> you write:
Quoted 4 lines
drivers/net/mii.cdoesn't export module init/cleanup functions. That means it can't be loaded under the new module scheme. This patch adds do-nothing functions for it, which allows it to load. (8139too depends on mii, so without this I don't have network.)
Correct fix is to fix the module code, which is done in the (attached) patch.
Hope that helps!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Name: Module Name Solution
Author: Kai Germaschewski
D: Well, I have another solution, which doesn't need additional Makefile
D: magic or anything.
D:
D: I just put the module name into each .o file where <linux/module.h> is
D: included. Putting it into the section .gnu.linkonce.modname has the effect
D: that even for multi-part modules, we only end up with one copy of the
D: name.
D:
D: Caveat: I'm using the preprocessor macro KBUILD_MODNAME to know what to
D: put into .gnu.linkonce.modname. The following used to happen:
D:
D: (drivers/isdn/eicon/Makefile)
D:
D: divas-objs := common.o Divas_mod.o ...
D: eicon-objs := common.o eicon_mod.o ...
D:
D: Divas_mod.o is compiled with -DKBUILD_MODNAME=divas
D: eicon_mod.o is compiled with -DKBUILD_MODNAME=eicon
D: common.o is compiled with -DKBUILD_MODNAME=divas_eicon
D:
D: So in the case above, both divas.o and eicon.o would end up with
D: a .gnu.linkonce.modname section containing "divas_eicon"
D:
D: My fix to this is to not define KBUILD_MODNAME when compiling an object
D: whilch will be linked into more than one module - so common.o gets no
D: .gnu.linkonce.modname section at all. Works fine here.
D:
D: Now, doing this I remove one of the reasons why we would need modules
D: linked as '.ko' ;), but it seems much cleaner than generating a temporary
D: file, using objcopy etc.
include/linux/init.h +0 -16
@@ -125,14 +125,6 @@ extern struct kernel_param __setup_start*/ #define module_exit (x) __exitcall(x); -/** -* no_module_init - code needs no initialization. -* -* The equivalent of declaring an empty init function which returns 0. -* Every module must have exactly one module_init() or no_module_init -* invocation. */ -#define no_module_init -#else /* MODULE */ /* Don't use these in modules, but some people do... */
@@ -144,11 +136,6 @@ extern struct kernel_param __setup_start#define device_initcall (fn) module_init(fn)#define late_initcall (fn) module_init(fn) -/* Each module knows its own name. */ -#define __DEFINE_MODULE_NAME \ - char __module_name[] __attribute__((section(".modulename"))) = \ - __stringify(KBUILD_MODNAME) -/* These macros create a dummy inline: gcc 2.9x does not count alias as usage, hence the `unused function' warning when __init functions are declared static. We use the dummy __*_module_inline functions
@@ -157,12 +144,9 @@ extern struct kernel_param __setup_start/* Each module must use one module_init(), or one no_module_init */ #define module_init (initfn) \ - __DEFINE_MODULE_NAME; \ static inline initcall_t __inittest(void) \ { return initfn; } \ int __initfn(void) __attribute__((alias(#initfn))); - -#define no_module_init __DEFINE_MODULE_NAME/* This is only required if you want to be unloadable. */ #define module_exit (exitfn) \
include/linux/module.h +6 -6
@@ -40,6 +40,11 @@ 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 +#define MODULE_GENERIC_TABLE (gtype,name) \ static const unsigned long __module_##gtype##_size \ __attribute__ ((unused)) = sizeof(struct gtype##_id); \
@@ -306,12 +311,7 @@ extern int module_dummy_usage;/* Old-style "I'll just call it init_module and it'll be run at insert". Use module_init(myroutine) instead. */ #ifdef MODULE -/* Used as "int init_module(void) { ... }". Get funky to insert modname. */ -#define init_module (voidarg) \ - __initfn(void); \ - char __module_name[] __attribute__((section(".modulename"))) = \ - __stringify(KBUILD_MODNAME); \ - int __initfn(void) +#define init_module (voidarg) __initfn(void)#define cleanup_module (voidarg) __exitfn(void)#endif
kernel/module.c +2 -2
@@ -864,8 +864,8 @@ static struct module *load_module(void */* Internal symbols */ DEBUGP ("Symbol table in section %u\n" , i); symindex = i; - }else if (strcmp (secstrings+sechdrs[i].sh_name,".modulename" ) - ==0 ) { + }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;
lib/zlib_deflate/deflate_syms.c +0 -2
@@ -19,5 +19,3 @@ EXPORT_SYMBOL(zlib_deflateReset);EXPORT_SYMBOL (zlib_deflateCopy);EXPORT_SYMBOL (zlib_deflateParams);MODULE_LICENSE ("GPL" ); - -no_module_init;
lib/zlib_inflate/inflate_syms.c +0 -2
@@ -20,5 +20,3 @@ EXPORT_SYMBOL(zlib_inflateReset);EXPORT_SYMBOL (zlib_inflateSyncPoint);EXPORT_SYMBOL (zlib_inflateIncomp);MODULE_LICENSE ("GPL" ); - -no_module_init;
scripts/Makefile.lib +3 -3
@@ -124,14 +124,14 @@ depfile = $(subst $(comma),_,$(@D)/.$(@F # than one module. In that case KBUILD_MODNAME will be set to foo_bar, # where foo and bar are the name of the modules. basename_flags = -DKBUILD_BASENAME=$(subst $(comma),_,$(subst -,_,$(*F))) -modname_flags = -DKBUILD_MODNAME=$(subst $(comma),_,$(subst -,_,$(modname))) +modname_flags = $(if $(filter 1,$(words $(modname))),-DKBUILD_MODNAME=$(subst $(comma),_,$(subst -,_,$(modname)))) c_flags = -Wp,-MD,$(depfile) $(CFLAGS) $(NOSTDINC_FLAGS) \ $(modkern_cflags) $(EXTRA_CFLAGS) $(CFLAGS_$(*F).o) \ $(basename_flags) $(modname_flags) $(export_flags) # Finds the multi-part object the current object will be linked into -modname-multi = $(subst $(space),_,$(sort $(foreach m,$(multi-used),\ - $(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))) +modname-multi = $(sort $(foreach m,$(multi-used),\ + $(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=)))) # Shipped files # ===========================================================================