lkml.org 
[lkml]   [2008]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] block2mtd lockdep_init_map warning
    From
    Date

    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);



    \
     
     \ /
      Last update: 2008-01-16 09:21    [W:0.028 / U:0.804 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site