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
--- drivers/net/mii.c~no 2002-11-19 11:41:20.000000000 -0600
+++ drivers/net/mii.c 2002-11-19 11:39:06.000000000 -0600
@@ -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);
Matt Reppert wrote:
> 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.)
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
On 19 Nov 02 at 12:51, Jeff Garzik wrote:
> Matt Reppert wrote:
>
> > 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.)
>
> 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.
Rusty told me that it is intentional. Add
no_module_init;
at the end of module. He even sent patch which fixes dozen of such
modules (15 I had on my system...) to Linus, but it get somehow lost.
Only question is whether we want to have it this way or no. And if
yes, whether we do not want to move no_module_init from linux/init.h to
linux/module.h: all of affected modules were already including
module.h to get MODULE_LICENSE() & other, but almost none of them
included init.h.
Petr Vandrovec
[email protected]
On Tue, 19 Nov 2002 12:51:44 -0500
Jeff Garzik <[email protected]> wrote:
> Matt Reppert wrote:
>
> > 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.)
>
> 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
On 19 Nov 02 at 12:15, Matt Reppert wrote:
> On Tue, 19 Nov 2002 12:51:44 -0500
> Jeff Garzik <[email protected]> wrote:
>
> > Matt Reppert wrote:
> >
> > > 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.)
> >
> > 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!")
I think that retrieving module name from module's binary is wrong: I
need to have dummy.o (network driver) insmodded two times to get my
test environment up.
I do not think that it is correct that I must add multiple device support
to the dummy due to new module loader, and creating two dummy.o,
with different .modulename sections, also does not look like reasonable
solution to me.
Best regards,
Petr Vandrovec
[email protected]
Petr Vandrovec wrote:
> I think that retrieving module name from module's binary is wrong: I
> need to have dummy.o (network driver) insmodded two times to get my
> test environment up.
agreed... this requirement is why the redundant no_module_init is needed :(
> I do not think that it is correct that I must add multiple device support
> to the dummy due to new module loader, and creating two dummy.o,
> with different .modulename sections, also does not look like reasonable
> solution to me.
The promise of minimal driver breakage is rapidly fading away.
Jeff, still grumbling from last email message :)
Matt Reppert wrote:
> On Tue, 19 Nov 2002 12:51:44 -0500
> Jeff Garzik wrote:
>
>
> >Matt Reppert wrote:
> >
> >
> >>drivers/net/mii.c doesn't export module init/cleanup functions. That
> >>means it
> >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!")
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
Petr Vandrovec wrote:
> Rusty told me that it is intentional. Add
>
> no_module_init;
>
> at the end of module. He even sent patch which fixes dozen of such
> modules (15 I had on my system...) to Linus, but it get somehow lost.
I know.
I'm hoping Linus intentionally dropped it, because it's silly. See the
other message I just posted. It's redundant because the module loader
can obviously figure out there is no init nor exit routine. It's just
like EXPORT_NO_SYMBOLS: redundant and obvious.
<rant>
Why the fsck is Rusty's new module code requiring all these driver
changes??? Note from your message above that there are "dozens" of
modules which worked just fine, but now they need to be changed under
Rusty's new system.
I thought Rusty's new stuff was going to cause minimal to no driver
breakage. You know, at kernel summit there was the thought that we
should just disable module -un-loading. I wish he had stuck with that
simple idea, plus module_param [because MODULE_PARM obviously sucks].
</rant>
Jeff, grumbling driver author who sees his drivers diverging and
breaking...
On Tue, 2002-11-19 at 17:50, Matt Reppert wrote:
> 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.)
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
[email protected] said:
> Rusty told me that it is intentional. Add
> no_module_init;
> at the end of module. He even sent patch which fixes dozen of such
> modules (15 I had on my system...) to Linus, but it get somehow lost.
> Only question is whether we want to have it this way or no.
Some questions you might want to consider before that one:
1. How do we handle "insmod -odummy0 dummy.o ; insmod -odummy1 dummy.o"
2. When does the module name differ from the filename (modulo .o) ?
3. Given your answers to #1 and #2, is the module name redundant anyway?
--
dwmw2
In message <[email protected]> you write:
> The promise of minimal driver breakage is rapidly fading away.
Perhaps: but this, at least, is being fixed by having a "finishing"
step for modules. And it *did* catch three "modules" that weren't
really, so it's not completely useless.
Think back: who in their right mind would compile and test patches to
a rapidly-changing kernel, when those changes required userspace tool
changes and you didn't know if it was going to go in or not? If you
care about modules in 2.5, you're probably a developer who needs
modules to do their job, so why rock the boat?
So I had to make a number of judgement calls without *any* input from
anyone else. This was one: change the makefiles to add an extra step,
or require modules to say "I really am a module, not some random
object file".
On the one hand I've been trying to feed this to Linus slowly so that
*now* lkml can give feedback, on the other hand, fend of those who are
complaining that their needed feature isn't back in yet.
You've seen some of my decisions reversed already: noone wishes more
than me this had happened months ago 8(
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
In message <[email protected]> you write:
> I think that retrieving module name from module's binary is wrong: I
> need to have dummy.o (network driver) insmodded two times to get my
> test environment up.
>
> I do not think that it is correct that I must add multiple device support
> to the dummy due to new module loader, and creating two dummy.o,
> with different .modulename sections, also does not look like reasonable
> solution to me.
Doing this based on module name has been deprecated basically forever,
because it doesn't work *at all* when you build it into the kernel.
The reason that modules know their own name is that boot parameters
are called by it.
Four possible solutions:
1) Fix dummy to be a real device driver.
2) Add a module parameter to set the device name on dummy.o
3) Allow the build system to construct multiple modules.
4) Implement modprobe --rename.
If none of them appeals, we can discuss it.
Hope that helps,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
In message <[email protected]> you write:
> Matt Reppert wrote:
>
> > 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.)
>
> 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.
Linus, this is a better fix, until the module post-link patch goes in
(rth and I are working on it, Kai likes the idea for other reasons).
Once again, fix up my assumptions so we err on the side of not
breaking anything.
Untested (on plane home and low on battery), but trivial.
Hope this helps,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5-bk/kernel/module.c working-2.5-bk-modname/kernel/module.c
--- linux-2.5-bk/kernel/module.c 2002-11-20 05:58:00.000000000 +1100
+++ working-2.5-bk-modname/kernel/module.c 2002-11-20 09:18:04.000000000 +1100
@@ -895,12 +895,6 @@ static struct module *load_module(void *
#endif
}
- if (!modnameindex) {
- DEBUGP("Module has no name!\n");
- err = -ENOEXEC;
- goto free_hdr;
- }
-
/* Now allocate space for the module proper, and copy name and args. */
err = strlen_user(uargs);
if (err < 0)
@@ -917,8 +911,17 @@ static struct module *load_module(void *
err = -EFAULT;
goto free_mod;
}
- strncpy(mod->name, (char *)hdr + sechdrs[modnameindex].sh_offset,
- sizeof(mod->name)-1);
+
+ if (modnameindex)
+ strncpy(mod->name,
+ (char *)hdr + sechdrs[modnameindex].sh_offset,
+ sizeof(mod->name)-1);
+ else {
+ /* FIXME: With module post-link phase, always insert
+ .modname section. --RR */
+ static unsigned int num_unknown;
+ sprintf(mod->name, "unknown%u", num_unknown++);
+ }
if (find_module(mod->name)) {
err = -EEXIST;
On Tue, 19 Nov 2002, Jeff Garzik wrote:
> 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.
I'm afraid it's not.
> 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.
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.
--
bill davidsen <[email protected]>
CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.
In message <[email protected]> you write:
> 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.)
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.
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.49/include/linux/init.h working-2.5.49-modname/include/linux/init.h
--- linux-2.5.49/include/linux/init.h Tue Nov 19 09:58:51 2002
+++ working-2.5.49-modname/include/linux/init.h Mon Nov 25 14:30:26 2002
@@ -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) \
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.49/include/linux/module.h working-2.5.49-modname/include/linux/module.h
--- linux-2.5.49/include/linux/module.h Mon Nov 25 08:44:18 2002
+++ working-2.5.49-modname/include/linux/module.h Mon Nov 25 14:35:57 2002
@@ -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
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.49/kernel/module.c working-2.5.49-modname/kernel/module.c
--- linux-2.5.49/kernel/module.c Mon Nov 25 08:44:19 2002
+++ working-2.5.49-modname/kernel/module.c Mon Nov 25 14:35:40 2002
@@ -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;
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.49/lib/zlib_deflate/deflate_syms.c working-2.5.49-modname/lib/zlib_deflate/deflate_syms.c
--- linux-2.5.49/lib/zlib_deflate/deflate_syms.c Tue Nov 19 09:58:52 2002
+++ working-2.5.49-modname/lib/zlib_deflate/deflate_syms.c Mon Nov 25 13:55:31 2002
@@ -19,5 +19,3 @@ EXPORT_SYMBOL(zlib_deflateReset);
EXPORT_SYMBOL(zlib_deflateCopy);
EXPORT_SYMBOL(zlib_deflateParams);
MODULE_LICENSE("GPL");
-
-no_module_init;
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.49/lib/zlib_inflate/inflate_syms.c working-2.5.49-modname/lib/zlib_inflate/inflate_syms.c
--- linux-2.5.49/lib/zlib_inflate/inflate_syms.c Tue Nov 19 09:58:52 2002
+++ working-2.5.49-modname/lib/zlib_inflate/inflate_syms.c Mon Nov 25 13:55:31 2002
@@ -20,5 +20,3 @@ EXPORT_SYMBOL(zlib_inflateReset);
EXPORT_SYMBOL(zlib_inflateSyncPoint);
EXPORT_SYMBOL(zlib_inflateIncomp);
MODULE_LICENSE("GPL");
-
-no_module_init;
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.49/scripts/Makefile.lib working-2.5.49-modname/scripts/Makefile.lib
--- linux-2.5.49/scripts/Makefile.lib Mon Nov 25 08:44:19 2002
+++ working-2.5.49-modname/scripts/Makefile.lib Mon Nov 25 13:55:31 2002
@@ -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
# ===========================================================================