On Tue, 2008-01-08 at 11:47 +1100, Rusty Russell wrote: > 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?
I think it should, Erez care to give it a spin? > 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/
