On Monday 07 January 2008 21:05:26 Peter Zijlstra wrote:
> On Sun, 2008-01-06 at 14:11 -0500, Erez Zadok wrote:
> > > Ingo, Peter, does either of you actually care about this problem? In
> > > the last round when I debugged this problem there was a notable lack of
> > > reaction from either of you.
> >
> > The problem appears to be an interaction of two components--module
> > loading and lockdep--that's perhaps why it wasn't given enough attention.
>
> Would something like this work for people?
Hi Peter,
There's nothing wrong with this patch, but I think it papers over a more
general problem: we enter the module (to parse args) while it's not in the
module list. This also means we won't get a nice oops if it crashes.
This is untested, but does it solve it for you?
Thanks,
Rusty.
diff -r 68fd1b22db89 kernel/module.c
--- a/kernel/module.c Mon Jan 07 18:59:50 2008 +1100
+++ b/kernel/module.c Tue Jan 08 11:46:11 2008 +1100
@@ -2043,6 +2043,11 @@ static struct module *load_module(void _
printk(KERN_WARNING "%s: Ignoring obsolete parameters\n",
mod->name);
+ /* Now sew it into the lists so we can get lockdep and oops
+ * info during argument parsing. Noone should access us, since
+ * strong_try_module_get() will fail. */
+ stop_machine_run(__link_module, mod, NR_CPUS);
+
/* Size of section 0 is 0, so this works well if no params */
err = parse_args(mod->name, mod->args,
(struct kernel_param *)
@@ -2051,7 +2056,7 @@ static struct module *load_module(void _
/ sizeof(struct kernel_param),
NULL);
if (err < 0)
- goto arch_cleanup;
+ goto unlink;
err = mod_sysfs_setup(mod,
(struct kernel_param *)
@@ -2059,7 +2064,7 @@ static struct module *load_module(void _
sechdrs[setupindex].sh_size
/ sizeof(struct kernel_param));
if (err < 0)
- goto arch_cleanup;
+ goto unlink;
add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
@@ -2074,7 +2079,8 @@ static struct module *load_module(void _
/* Done! */
return mod;
- arch_cleanup:
+ unlink:
+ stop_machine_run(__unlink_module, mod, NR_CPUS);
module_arch_cleanup(mod);
cleanup:
module_unload_free(mod);
@@ -2130,10 +2136,6 @@ sys_init_module(void __user *umod,
mutex_unlock(&module_mutex);
return PTR_ERR(mod);
}
-
- /* Now sew it into the lists. They won't access us, since
- strong_try_module_get() will fail. */
- stop_machine_run(__link_module, mod, NR_CPUS);
/* Drop lock so they can recurse */
mutex_unlock(&module_mutex);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
