[RELEASE] module-init-tools 0.8

[RELEASE] module-init-tools 0.8

[Rusty Russell]

[ Linus, please apply patch! ]

	http://www.[cc].kernel.org/pub/linux/kernel/people/rusty/modules

(Source RPM untested, but not markedly different from previous one).

This release needs depmod again, which should help speed for those of
you with 1300 modules.  A replacement depmod is provided, since the
previous one gets rightfully confused by 2.5.47+ kernels.  You will
require a small kernel patch to 2.5.50 (below) for PCI and USB tables
to work.

Also included is modules.conf2modprobe.conf, which is fairly
simplistic but should get most people up and running.  This will be
enhanced as new features go into the new modprobe.

Some dummy options are implemented, and "modprobe -c" is implemented
too, which should help Mandrake and RedHat's init scripts deal with
the change.

Many thanks to those who provided patches, bug reports, and copies of
their init scripts.  Your feedback is greatly appreciated!

Please report any bugs to rusty@rustcorp.com.au.

Thanks!
Rusty.

diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.50/include/linux/module.h working-2.5.50-table/include/linux/module.h
--- linux-2.5.50/include/linux/module.h	Mon Nov 25 08:44:18 2002
+++ working-2.5.50-table/include/linux/module.h	Thu Nov 28 10:59:39 2002
@@ -14,7 +14,7 @@
 #include <linux/compiler.h>
 #include <linux/cache.h>
 #include <linux/kmod.h>
-#include <linux/elf.h>
+#include <linux/stringify.h>
 
 #include <asm/module.h>
 #include <asm/uaccess.h> /* For struct exception_table_entry */
@@ -40,11 +40,14 @@ struct kernel_symbol
 
 #ifdef MODULE
 
-#define MODULE_GENERIC_TABLE(gtype,name)	\
-static const unsigned long __module_##gtype##_size \
-  __attribute__ ((unused)) = sizeof(struct gtype##_id); \
-static const struct gtype##_id * __module_##gtype##_table \
-  __attribute__ ((unused)) = name
+/* For replacement modutils, use an alias not a pointer. */
+#define MODULE_GENERIC_TABLE(gtype,name)			\
+static const unsigned long __module_##gtype##_size		\
+  __attribute__ ((unused)) = sizeof(struct gtype##_id);		\
+static const struct gtype##_id * __module_##gtype##_table	\
+  __attribute__ ((unused)) = name;				\
+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. */
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [RELEASE] module-init-tools 0.8

[Gerd Knorr]

>  Please report any bugs to rusty@rustcorp.com.au.

System (SuSE 8.1) still doesn't boot up cleanly.  After logging in as
root I can see a number of modprobe processes hanging around in the
process table:

bogomips root ~# ps -ax | grep modprobe
  621 ?        S      0:00 /sbin/modprobe -- char-major-6
  622 ?        D      0:00 /sbin/modprobe -- parport_lowlevel
  805 ?        D      0:00 /sbin/modprobe -- autofs
  809 ?        D      0:00 /sbin/modprobe -- autofs
  867 ?        D      0:00 /sbin/modprobe -- autofs
  872 ?        D      0:00 /sbin/modprobe -- net-pf-17
 1184 ttyS0    S      0:00 grep modprobe
bogomips root ~# grep char-major-6 /etc/modprobe.conf
alias char-major-6 lp
alias char-major-67 coda
bogomips root ~# grep parport_lowlevel /etc/modprobe.conf
alias parport_lowlevel parport_pc

Smells like a deadlock due to request_module() in some modules init
function or something like this.

lsmod doesn't work at this point (hangs too, likely the same lock).
The deadlock prevents any further module loading (autofs, nfs and
others) and makes the system unusable.


Module debugging is next to impossible right now.  The apm.o module
oopses for me in 2.5.50.  ksymoops isn't able to translate any symbol
located in modules.  The in-kernel symbol decoder (CONFIG_KALLSYMS)
doesn't work too.

  Gerd

-- 
You can't please everybody.  And usually if you _try_ to please
everybody, the end result is one big mess.
				-- Linus Torvalds, 2002-04-20

Re: [RELEASE] module-init-tools 0.8

[Bill Davidsen]

On Thu, 28 Nov 2002, Gerd Knorr wrote:

> lsmod doesn't work at this point (hangs too, likely the same lock).
> The deadlock prevents any further module loading (autofs, nfs and
> others) and makes the system unusable.
> 
> 
> Module debugging is next to impossible right now.  The apm.o module
> oopses for me in 2.5.50.  ksymoops isn't able to translate any symbol
> located in modules.  The in-kernel symbol decoder (CONFIG_KALLSYMS)
> doesn't work too.

The new module stuff has been in for about three weeks now, many people
are having problems with it, and I have yet to see a single post praising
the *actual* benefits. Will there be a time when this is reverted and
rescheduled for a future release (2.7?) or is this a do-or-die feature?

It doesn't have the feel of something solid having a few corner cases
fixed, it feels like a bunch of band-aids which will unstick in future
releases and continue to be high maintenence.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [RELEASE] module-init-tools 0.8

[Tomas Szepe]

> > lsmod doesn't work at this point (hangs too, likely the same lock).
> > The deadlock prevents any further module loading (autofs, nfs and
> > others) and makes the system unusable.
> > 
> > Module debugging is next to impossible right now.  The apm.o module
> > oopses for me in 2.5.50.  ksymoops isn't able to translate any symbol
> > located in modules.  The in-kernel symbol decoder (CONFIG_KALLSYMS)
> > doesn't work too.
> 
> The new module stuff has been in for about three weeks now, many people
> are having problems with it, and I have yet to see a single post praising
> the *actual* benefits. Will there be a time when this is reverted and
> rescheduled for a future release (2.7?) or is this a do-or-die feature?
> 
> It doesn't have the feel of something solid having a few corner cases
> fixed, it feels like a bunch of band-aids which will unstick in future
> releases and continue to be high maintenence.

Also I can't see how the new module infrastructure could have made it
in w/o having been complete, *functional*, proven and thoroughly reviewed
off-tree in the first place (which I thought was pretty much a standard
around here).  Mature, drop-in replacement projects like Keith Owen's
kbuild 2.5 are getting ignored while something as wild as Rusty's "welcome
in module hell exhibit" is merged right when the tree is supposed to start
stabilizing.

And heck, I haven't even seen Viro and Hellwig complaining!
What's going on?  :)

-- 
Tomas Szepe <szepe@pinerecords.com>

Re: [RELEASE] module-init-tools 0.8

[Christoph Hellwig]

On Thu, Nov 28, 2002 at 07:22:15PM +0100, Tomas Szepe wrote:
> And heck, I haven't even seen Viro and Hellwig complaining!
> What's going on?  :)

I have complained once in the very beginning.  Reading the arches
might help.

I'm back at 2.5.47-xfs for daily work now until some brave souls
get the new module stuff in shape.  It doesn't look like this is
going to happen anytime soon, though.

Re: [RELEASE] module-init-tools 0.8

[Jan-Benedict Glaw]

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

On Thu, 2002-11-28 12:09:51 -0500, Bill Davidsen <davidsen@tmr.com>
wrote in message <Pine.LNX.3.96.1021128115833.12997A-100000@gatekeeper.tmr.com>:
> On Thu, 28 Nov 2002, Gerd Knorr wrote:
> The new module stuff has been in for about three weeks now, many people
> are having problems with it, and I have yet to see a single post praising
> the *actual* benefits. Will there be a time when this is reverted and
> rescheduled for a future release (2.7?) or is this a do-or-die feature?

It's not only that i386 is b0rked to some degree. Ever looked at some
other architectures? Again, most (if not all) won't compile again. Eg.
last Alpha kernel which worked for me (TM) was 2.5.44...

MfG, JBG

-- 
   Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481
   "Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur
    fuer einen Freien Staat voll Freier Bürger" | im Internet!
   Shell Script APT-Proxy: http://lug-owl.de/~jbglaw/software/ap2/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [RELEASE] module-init-tools 0.8

[Gerd Knorr]

>  alias char-major-6 lp
>  alias parport_lowlevel parport_pc
>  
>  Smells like a deadlock due to request_module() in some modules init
>  function or something like this.

Next try:  Changed "alias char-major-6" to "off".  Works better, at
least the system comes up with all network stuff working and I can
login as user (with $HOME at nfs).  Various modules still fail to
load (bttv driver, matroxfb):

WARNING: Error inserting v4l2_common (/lib/modules/2.5.50/kernel/v4l2-common.o): Invalid module format
WARNING: Error inserting video_buf (/lib/modules/2.5.50/kernel/video-buf.o): Invalid module format
FATAL: Error inserting bttv (/lib/modules/2.5.50/kernel/bttv.o): Unknown symbol in module

WARNING: Error inserting matroxfb_misc (/lib/modules/2.5.50/kernel/matroxfb_misc.o): Invalid module format
WARNING: Error inserting matroxfb_accel (/lib/modules/2.5.50/kernel/matroxfb_accel.o): Invalid module format
WARNING: Error inserting matroxfb_DAC1064 (/lib/modules/2.5.50/kernel/matroxfb_DAC1064.o): Invalid module format
FATAL: Error inserting matroxfb_base (/lib/modules/2.5.50/kernel/matroxfb_base.o): Unknown symbol in module

  Gerd

-- 
You can't please everybody.  And usually if you _try_ to please
everybody, the end result is one big mess.
				-- Linus Torvalds, 2002-04-20

Re: [RELEASE] module-init-tools 0.8

[Marco d'Itri]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On Nov 28, Gerd Knorr <kraxel@bytesex.org> wrote:

 >login as user (with $HOME at nfs).  Various modules still fail to
 >load (bttv driver, matroxfb):
 >
 >WARNING: Error inserting v4l2_common (/lib/modules/2.5.50/kernel/v4l2-common.o): Invalid module format
 >WARNING: Error inserting video_buf (/lib/modules/2.5.50/kernel/video-buf.o): Invalid module format
 >FATAL: Error inserting bttv (/lib/modules/2.5.50/kernel/bttv.o): Unknown symbol in module

I experience the same problem (with saa7134) since 2.5.49.
2.5.48 worked.

This happens both with module-init-tools 0.7 and 0.8.

-- 
ciao,
Marco

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [RELEASE] module-init-tools 0.8

[Rusty Russell]

In message <200211281823.gASINAuN014312@bytesex.org> you write:
> >  alias char-major-6 lp
> >  alias parport_lowlevel parport_pc
> >  
> >  Smells like a deadlock due to request_module() in some modules init
> >  function or something like this.
> 
> Next try:  Changed "alias char-major-6" to "off".  Works better, at
> least the system comes up with all network stuff working and I can
> login as user (with $HOME at nfs).  Various modules still fail to
> load (bttv driver, matroxfb):
> 
> WARNING: Error inserting v4l2_common (/lib/modules/2.5.50/kernel/v4l2-common.
o): Invalid module format

Yes, Linus unfortunately has been dropping my patches.  Please apply.

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

Name: Module Name Solution
Author: Kai Germaschewski
Depends: 

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
 # ===========================================================================

Re: [RELEASE] module-init-tools 0.8

[Rusty Russell]

In message <200211281616.gASGGOE6012229@bytesex.org> you write:
> >  Please report any bugs to rusty@rustcorp.com.au.
> 
> System (SuSE 8.1) still doesn't boot up cleanly.  After logging in as
> root I can see a number of modprobe processes hanging around in the
> process table:
> 
> bogomips root ~# ps -ax | grep modprobe
>   621 ?        S      0:00 /sbin/modprobe -- char-major-6
>   622 ?        D      0:00 /sbin/modprobe -- parport_lowlevel
>   805 ?        D      0:00 /sbin/modprobe -- autofs
>   809 ?        D      0:00 /sbin/modprobe -- autofs
>   867 ?        D      0:00 /sbin/modprobe -- autofs
>   872 ?        D      0:00 /sbin/modprobe -- net-pf-17
>  1184 ttyS0    S      0:00 grep modprobe

I suspect one of these modules is trying to load other modules in its
init routine.  I wondered if anyone did this, I guess they do 8(.

Hmm, looks like parport.  I'll send a patch to drop the module lock
around module->init().  It's a good idea *anyway*: if a module oopses
in its init routine it's nice to be able to still use modules.

> Module debugging is next to impossible right now.  The apm.o module
> oopses for me in 2.5.50.  ksymoops isn't able to translate any symbol
> located in modules.  The in-kernel symbol decoder (CONFIG_KALLSYMS)
> doesn't work too.

Please try (previously posted) patch below,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: Kallsyms inside module fix
Author: Rusty Russell
Status: Tested on 2.5.50

D: Two fixes.  Firstly, set ALLOC on the right section so we actually
D: keep the symbol names and don't deref a freed section, and secondly
D: get the symbol size (more) correct.

diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.50/kernel/module.c working-2.5.50-ksymoops/kernel/module.c
--- linux-2.5.50/kernel/module.c	Mon Nov 25 08:44:19 2002
+++ working-2.5.50-ksymoops/kernel/module.c	Thu Nov 28 16:22:56 2002
@@ -892,7 +892,7 @@ static struct module *load_module(void *
 		}
 #ifdef CONFIG_KALLSYMS
 		/* symbol and string tables for decoding later. */
-		if (sechdrs[i].sh_type == SHT_SYMTAB || i == hdr->e_shstrndx)
+		if (sechdrs[i].sh_type == SHT_SYMTAB || i == strindex)
 			sechdrs[i].sh_flags |= SHF_ALLOC;
 #endif
 #ifndef CONFIG_MODULE_UNLOAD
@@ -1165,7 +1165,14 @@ static const char *get_ksymbol(struct mo
 			       unsigned long *size,
 			       unsigned long *offset)
 {
-	unsigned int i, next = 0, best = 0;
+	unsigned int i, best = 0;
+	unsigned long nextval;
+
+	/* At worse, next value is at end of module */
+	if (inside_core(mod, addr))
+		nextval = (unsigned long)mod->module_core+mod->core_size;
+	else 
+		nextval = (unsigned long)mod->module_init+mod->init_size;
 
 	/* Scan for closest preceeding symbol, and next symbol. (ELF
            starts real symbols at 1). */
@@ -1177,22 +1186,14 @@ static const char *get_ksymbol(struct mo
 		    && mod->symtab[i].st_value > mod->symtab[best].st_value)
 			best = i;
 		if (mod->symtab[i].st_value > addr
-		    && mod->symtab[i].st_value < mod->symtab[next].st_value)
-			next = i;
+		    && mod->symtab[i].st_value < nextval)
+			nextval = mod->symtab[i].st_value;
 	}
 
 	if (!best)
 		return NULL;
 
-	if (!next) {
-		/* Last symbol?  It ends at the end of the module then. */
-		if (inside_core(mod, addr))
-			*size = mod->module_core+mod->core_size - (void*)addr;
-		else
-			*size = mod->module_init+mod->init_size - (void*)addr;
-	} else
-		*size = mod->symtab[next].st_value - addr;
-
+	*size = nextval - mod->symtab[best].st_value;
 	*offset = addr - mod->symtab[best].st_value;
 	return mod->strtab + mod->symtab[best].st_name;
 }

Re: [RELEASE] module-init-tools 0.8

[Rusty Russell]

In message <200211281616.gASGGOE6012229@bytesex.org> you write:
> Smells like a deadlock due to request_module() in some modules init
> function or something like this.

This doesn't happen on my test box for any of my modules, so I can't
really check.  But I'd appreciate it if you would try the patch below.

Thanks for the report!
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: Module init reentry fix
Author: Rusty Russell
Status: Tested on 2.5.50

D: This changes the code to drop the module_mutex() before calling the
D: module's init function, so module init functions can call
D: request_module().  This was trivial before someone broke the module
D: code to start non-live.  Now it requires us to keep info on the
D: exact module state.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.50/include/linux/module.h working-2.5.50-initlock/include/linux/module.h
--- linux-2.5.50/include/linux/module.h	2002-11-25 08:44:18.000000000 +1100
+++ working-2.5.50-initlock/include/linux/module.h	2002-11-29 18:57:25.000000000 +1100
@@ -100,12 +100,18 @@ void *__symbol_get_gpl(const char *symbo
 struct module_ref
 {
 	atomic_t count;
-} ____cacheline_aligned;
+} ____cacheline_aligned_in_smp;
+
+enum module_state
+{
+	MODULE_STATE_LIVE,
+	MODULE_STATE_COMING,
+	MODULE_STATE_GOING,
+};
 
 struct module
 {
-	/* Am I live (yet)? */
-	int live;
+	enum module_state state;
 
 	/* Member of list of modules */
 	struct list_head list;
@@ -163,6 +169,14 @@ struct module
 	char args[0];
 };
 
+/* FIXME: It'd be nice to isolate modules during init, too, so they
+   aren't used before they (may) fail.  But presently too much code
+   (IDE & SCSI) require entry into the module during init.*/
+static inline int module_is_live(struct module *mod)
+{
+	return mod->state != MODULE_STATE_GOING;
+}
+
 #ifdef CONFIG_MODULE_UNLOAD
 
 void __symbol_put(const char *symbol);
@@ -181,7 +195,7 @@ static inline int try_module_get(struct 
 
 	if (module) {
 		unsigned int cpu = get_cpu();
-		if (likely(module->live))
+		if (likely(module_is_live(module)))
 			local_inc(&module->ref[cpu].count);
 		else
 			ret = 0;
@@ -196,7 +210,7 @@ static inline void module_put(struct mod
 		unsigned int cpu = get_cpu();
 		local_dec(&module->ref[cpu].count);
 		/* Maybe they're waiting for us to drop reference? */
-		if (unlikely(!module->live))
+		if (unlikely(!module_is_live(module)))
 			wake_up_process(module->waiter);
 		put_cpu();
 	}
@@ -205,7 +219,7 @@ static inline void module_put(struct mod
 #else /*!CONFIG_MODULE_UNLOAD*/
 static inline int try_module_get(struct module *module)
 {
-	return !module || module->live;
+	return !module || module_is_live(module);
 }
 static inline void module_put(struct module *module)
 {
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.50/kernel/module.c working-2.5.50-initlock/kernel/module.c
--- linux-2.5.50/kernel/module.c	2002-11-25 08:44:19.000000000 +1100
+++ working-2.5.50-initlock/kernel/module.c	2002-11-29 18:59:48.000000000 +1100
@@ -41,6 +41,14 @@
 static DECLARE_MUTEX(module_mutex);
 LIST_HEAD(modules); /* FIXME: Accessed w/o lock on oops by some archs */
 
+/* We require a truly strong try_module_get() */
+static inline int strong_try_module_get(struct module *mod)
+{
+	if (mod && mod->state == MODULE_STATE_COMING)
+		return 0;
+	return try_module_get(mod);
+}
+
 /* Convenient structure for holding init and core sizes */
 struct sizes
 {
@@ -375,7 +383,7 @@ sys_delete_module(const char *name_user,
 	}
 
 	/* Already dying? */
-	if (!mod->live) {
+	if (mod->state == MODULE_STATE_GOING) {
 		/* FIXME: if (force), slam module count and wake up
                    waiter --RR */
 		DEBUGP("%s already dying\n", mod->name);
@@ -383,6 +391,16 @@ sys_delete_module(const char *name_user,
 		goto out;
 	}
 
+	/* Coming up?  Allow force on stuck modules. */
+	if (mod->state == MODULE_STATE_COMING) {
+		forced = try_force(flags);
+		if (!forced) {
+			/* This module can't be removed */
+			ret = -EBUSY;
+			goto out;
+		}
+	}
+
 	if (!mod->exit || mod->unsafe) {
 		forced = try_force(flags);
 		if (!forced) {
@@ -404,7 +422,7 @@ sys_delete_module(const char *name_user,
 			ret = -EWOULDBLOCK;
 	} else {
 		mod->waiter = current;
-		mod->live = 0;
+		mod->state = MODULE_STATE_GOING;
 	}
 	restart_refcounts();
 
@@ -504,7 +523,7 @@ static inline void module_unload_free(st
 
 static inline int use_module(struct module *a, struct module *b)
 {
-	return try_module_get(b);
+	return strong_try_module_get(b);
 }
 
 static inline void module_unload_init(struct module *mod)
@@ -575,7 +594,7 @@ void *__symbol_get(const char *symbol)
 
 	spin_lock_irqsave(&modlist_lock, flags);
 	value = __find_symbol(symbol, &ksg);
-	if (value && !try_module_get(ksg->owner))
+	if (value && !strong_try_module_get(ksg->owner))
 		value = 0;
 	spin_unlock_irqrestore(&modlist_lock, flags);
 
@@ -932,12 +951,8 @@ static struct module *load_module(void *
 		goto free_mod;
 	}
 
-	/* Initialize the lists, since they will be list_del'd if init fails */
-	INIT_LIST_HEAD(&mod->extable.list);
-	INIT_LIST_HEAD(&mod->list);
-	INIT_LIST_HEAD(&mod->symbols.list);
 	mod->symbols.owner = mod;
-	mod->live = 0;
+	mod->state = MODULE_STATE_COMING;
 	module_unload_init(mod);
 
 	/* How much space will we need?  (Common area in first) */
@@ -1094,51 +1109,39 @@ sys_init_module(void *umod,
 	flush_icache_range((unsigned long)mod->module_core,
 			   (unsigned long)mod->module_core + mod->core_size);
 
-	/* Now sew it into exception list (just in case...). */
+	/* Now sew it into the lists.  They won't access us, since
+           strong_try_module_get() will fail. */
 	spin_lock_irq(&modlist_lock);
 	list_add(&mod->extable.list, &extables);
+	list_add_tail(&mod->symbols.list, &symbols);
 	spin_unlock_irq(&modlist_lock);
+	list_add(&mod->list, &modules);
+
+	/* Drop lock so they can recurse */
+	up(&module_mutex);
 
-	/* Note, setting the mod->live to 1 here is safe because we haven't
-	 * linked the module into the system's kernel symbol table yet,
-	 * which means that the only way any other kernel code can call
-	 * into this module right now is if this module hands out entry
-	 * pointers to the other code.  We assume that no module hands out
-	 * entry pointers to the rest of the kernel unless it is ready to
-	 * have them used.
-	 */
-	mod->live = 1;
 	/* Start the module */
 	ret = mod->init ? mod->init() : 0;
 	if (ret < 0) {
 		/* Init routine failed: abort.  Try to protect us from
-                   buggy refcounters. */
+		   buggy refcounters. */
+		mod->state = MODULE_STATE_GOING;
 		synchronize_kernel();
-		if (mod->unsafe) {
+		if (mod->unsafe)
 			printk(KERN_ERR "%s: module is now stuck!\n",
 			       mod->name);
-			/* Mark it "live" so that they can force
-			   deletion later, and we don't keep getting
-			   woken on every decrement. */
-		} else {
-			mod->live = 0;
+		else {
+			down(&module_mutex);
 			free_module(mod);
-		}
-		up(&module_mutex);
+			up(&module_mutex);
 		return ret;
 	}
 
 	/* Now it's a first class citizen! */
-	spin_lock_irq(&modlist_lock);
-	list_add_tail(&mod->symbols.list, &symbols);
-	spin_unlock_irq(&modlist_lock);
-	list_add(&mod->list, &modules);
-
+	mod->state = MODULE_STATE_LIVE;
 	module_free(mod, mod->module_init);
 	mod->module_init = NULL;
 
-	/* All ok! */
-	up(&module_mutex);
 	return 0;
 }
 

Re: [RELEASE] module-init-tools 0.8

[Rusty Russell]

In message <Pine.LNX.4.44.0211272325370.924-100000@lap.molina> you write:
> With the patch you included, I get the following:
> 
> In file included from include/linux/raid/md.h:27,
>                  from init/do_mounts.c:25:
> include/linux/module.h:159: parse error before "Elf32_Sym"

Argh.... Put back the #include <linux/elf.h> at the top of the file.
It's required for CONFIG_KALLSYMS=y.

The main reason for separating out moduleloader.h was so modules.h
didn't include elf.h, because xfs defines AT_GID and AT_UID itself.
The kallsyms patch put that back.

We could make module.symtab a void*, but that's just wrong.  I think
we actually have to solve this clash, rather than hack around it,
since this is going to be a recurring problem.

Steve?  Changing the prefix on xfs or elf seems equally painful (xfs
because it'll be a big patch, elf because it's standardized and been
like that forever).

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

Re: [RELEASE] module-init-tools 0.8

[Nathan Scott]

On Fri, Nov 29, 2002 at 11:59:49AM +1100, Rusty Russell wrote:
> In message <Pine.LNX.4.44.0211272325370.924-100000@lap.molina> you write:
> > With the patch you included, I get the following:
> > 
> > In file included from include/linux/raid/md.h:27,
> >                  from init/do_mounts.c:25:
> > include/linux/module.h:159: parse error before "Elf32_Sym"
> 
> Argh.... Put back the #include <linux/elf.h> at the top of the file.
> It's required for CONFIG_KALLSYMS=y.
> 
> The main reason for separating out moduleloader.h was so modules.h
> didn't include elf.h, because xfs defines AT_GID and AT_UID itself.
> The kallsyms patch put that back.
> 
> We could make module.symtab a void*, but that's just wrong.  I think
> we actually have to solve this clash, rather than hack around it,
> since this is going to be a recurring problem.
> 
> Steve?  Changing the prefix on xfs or elf seems equally painful (xfs
> because it'll be a big patch, elf because it's standardized and been
> like that forever).

We changed XFS when this problem first came up.  Linus hasn't yet
pulled from Christoph's bitkeeper tree though, so we're a bit out
of sync at the moment.

cheers.

-- 
Nathan

Re: [RELEASE] module-init-tools 0.8

[Thomas Molina]

On Fri, 29 Nov 2002, Nathan Scott wrote:

> > Steve?  Changing the prefix on xfs or elf seems equally painful (xfs
> > because it'll be a big patch, elf because it's standardized and been
> > like that forever).
> 
> We changed XFS when this problem first came up.  Linus hasn't yet
> pulled from Christoph's bitkeeper tree though, so we're a bit out
> of sync at the moment.

Sounds like I need to drop back and punt.  I probably need to wait for the 
syncing to happen and bk to be updated.  I need to have 2.4 kernels 
working (specifically RedHat).  I'm doing this testing on my laptop, which 
because I'm going to be moving, will be my only machine for the next month 
or so.  I want to get 2.5 working, but module loading doesn't work and 
modutils-2.4.21-4.i386.rpm and 
modutils-2.4.21-5.i386.rpm break 2.4 kernels.

Re: [RELEASE] module-init-tools 0.8

[Keith Owens]

On Thu, 28 Nov 2002 20:24:33 -0600 (CST), 
Thomas Molina <tmolina@copper.net> wrote:
>I want to get 2.5 working, but module loading doesn't work and 
>modutils-2.4.21-4.i386.rpm and 
>modutils-2.4.21-5.i386.rpm break 2.4 kernels.

Whose version of modutils are you complaining about?  My base versions
of modutils 2.4.21 and 2.4.22 work fine for me on 2.4 kernels.

Re: [RELEASE] module-init-tools 0.8

[Thomas Molina]

On Fri, 29 Nov 2002, Keith Owens wrote:

> On Thu, 28 Nov 2002 20:24:33 -0600 (CST), 
> Thomas Molina <tmolina@copper.net> wrote:
> >I want to get 2.5 working, but module loading doesn't work and 
> >modutils-2.4.21-4.i386.rpm and 
> >modutils-2.4.21-5.i386.rpm break 2.4 kernels.
> 
> Whose version of modutils are you complaining about?  My base versions
> of modutils 2.4.21 and 2.4.22 work fine for me on 2.4 kernels.

Keith, 

I am currently using your modutils-2.4.22-1.i386.rpm which work fine 
with 2.4 kernels.  The ones I said were borked were in Rusty's directory 
at ftp.kernel.org/pub/linux/kernel/people/rusty/modules.  I was using his 
versions since I was testing some of his patches.