lkml.org 
[lkml]   [2010]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/2] module: fix bne2 "gave up waiting for init of module libcrc32c"


On Tue, 1 Jun 2010, Brandon Philips wrote:
>
> FWIW, I tried this same idea initially and it breaks because the
> kobject EEXIST is coming from mod_sysfs_init() which happens further
> up in load_module() before the list_add_rcu().

Ahh, right. So we'd need to mvoe that down too. As Rusty says, the kobject
EEXIST warning should be fairly harmless - albeit annoying. Do things
actually _work_ with that thing apart from the kobject warning?

> I also tried the obvious variation of moving the list_add_rcu() up
> to where the find_module is but got:

Yeah, I don't think moving it up will work, because then the module symbol
resolver will see itself before having set up the symbols. So I think we
need to expose it on the modules list only after having done the
"simplify_symbols()" etc.

I dunno.

Does this work?

(Caveat emptor - I tried to make sure the error cases nest correctly, and
it all compiles, but it's untested. As usual. Rusty mentioned a "see
testing patch", but I didn't see it. Maybe he did the same thing)

Linus
---
kernel/module.c | 35 +++++++++++++++++++----------------
1 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index a1f46a5..247c0aa 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2198,11 +2198,6 @@ static noinline struct module *load_module(void __user *umod,
goto free_mod;
}

- if (find_module(mod->name)) {
- err = -EEXIST;
- goto free_mod;
- }
-
mod->state = MODULE_STATE_COMING;

/* Allow arches to frob section contents and sizes. */
@@ -2293,11 +2288,6 @@ static noinline struct module *load_module(void __user *umod,
/* Now we've moved module, initialize linked lists, etc. */
module_unload_init(mod);

- /* add kobject, so we can reference it. */
- err = mod_sysfs_init(mod);
- if (err)
- goto free_unload;
-
/* Set up license info based on the info section */
set_license(mod, get_modinfo(sechdrs, infoindex, "license"));

@@ -2486,16 +2476,28 @@ static noinline struct module *load_module(void __user *umod,
* The mutex protects against concurrent writers.
*/
mutex_lock(&module_mutex);
+
+ if (find_module(mod->name)) {
+ err = -EEXIST;
+ /* This will also unlock the mutex */
+ goto already_exists;
+ }
+
list_add_rcu(&mod->list, &modules);
mutex_unlock(&module_mutex);

+ /* add kobject, so we can reference it. */
+ err = mod_sysfs_init(mod);
+ if (err)
+ goto unlink;
+
err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
if (err < 0)
- goto unlink;
+ goto sysfs_uninit;

err = mod_sysfs_setup(mod, mod->kp, mod->num_kp);
if (err < 0)
- goto unlink;
+ goto sysfs_uninit;
add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);

@@ -2507,18 +2509,19 @@ static noinline struct module *load_module(void __user *umod,
/* Done! */
return mod;

+ sysfs_uninit:
+ free_modinfo(mod);
+ kobject_del(&mod->mkobj.kobj);
+ kobject_put(&mod->mkobj.kobj);
unlink:
mutex_lock(&module_mutex);
/* Unlink carefully: kallsyms could be walking list. */
list_del_rcu(&mod->list);
+ already_exists:
mutex_unlock(&module_mutex);
synchronize_sched();
module_arch_cleanup(mod);
cleanup:
- free_modinfo(mod);
- kobject_del(&mod->mkobj.kobj);
- kobject_put(&mod->mkobj.kobj);
- free_unload:
module_unload_free(mod);
#if defined(CONFIG_MODULE_UNLOAD)
free_percpu(mod->refptr);

\
 
 \ /
  Last update: 2010-06-02 06:43    [W:0.130 / U:0.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site