diff options
| author | Linus Torvalds <torvalds@linux-foundation.org> | 2010-06-05 13:17:36 -0400 |
|---|---|---|
| committer | Rusty Russell <rusty@rustcorp.com.au> | 2010-06-04 21:47:37 -0400 |
| commit | 3bafeb6247042dcbb72b0141ec7c7107de9f0b99 (patch) | |
| tree | f9b8f2437455332cd64d94bedb0836ac0f2b6fd9 /kernel | |
| parent | 75676500f8298f0ee89db12db97294883c4b768e (diff) | |
module: move find_module check to end
I think Rusty may have made the lock a bit _too_ finegrained there, and
didn't add it to some places that needed it. It looks, for example, like
PATCH 1/2 actually drops the lock in places where it's needed
("find_module()" is documented to need it, but now load_module() didn't
hold it at all when it did the find_module()).
Rather than adding a new "module_loading" list, I think we should be able
to just use the existing "modules" list, and just fix up the locking a
bit.
In fact, maybe we could just move the "look up existing module" a bit
later - optimistically assuming that the module doesn't exist, and then
just undoing the work if it turns out that we were wrong, just before
adding ourselves to the list.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Diffstat (limited to 'kernel')
| -rw-r--r-- | kernel/module.c | 12 |
1 files changed, 7 insertions, 5 deletions
diff --git a/kernel/module.c b/kernel/module.c index d293c213c22c..28450047852a 100644 --- a/kernel/module.c +++ b/kernel/module.c | |||
| @@ -2226,11 +2226,6 @@ static noinline struct module *load_module(void __user *umod, | |||
| 2226 | goto free_mod; | 2226 | goto free_mod; |
| 2227 | } | 2227 | } |
| 2228 | 2228 | ||
| 2229 | if (find_module(mod->name)) { | ||
| 2230 | err = -EEXIST; | ||
| 2231 | goto free_mod; | ||
| 2232 | } | ||
| 2233 | |||
| 2234 | mod->state = MODULE_STATE_COMING; | 2229 | mod->state = MODULE_STATE_COMING; |
| 2235 | 2230 | ||
| 2236 | /* Allow arches to frob section contents and sizes. */ | 2231 | /* Allow arches to frob section contents and sizes. */ |
| @@ -2509,6 +2504,12 @@ static noinline struct module *load_module(void __user *umod, | |||
| 2509 | * The mutex protects against concurrent writers. | 2504 | * The mutex protects against concurrent writers. |
| 2510 | */ | 2505 | */ |
| 2511 | mutex_lock(&module_mutex); | 2506 | mutex_lock(&module_mutex); |
| 2507 | if (find_module(mod->name)) { | ||
| 2508 | err = -EEXIST; | ||
| 2509 | /* This will also unlock the mutex */ | ||
| 2510 | goto already_exists; | ||
| 2511 | } | ||
| 2512 | |||
| 2512 | list_add_rcu(&mod->list, &modules); | 2513 | list_add_rcu(&mod->list, &modules); |
| 2513 | mutex_unlock(&module_mutex); | 2514 | mutex_unlock(&module_mutex); |
| 2514 | 2515 | ||
| @@ -2535,6 +2536,7 @@ static noinline struct module *load_module(void __user *umod, | |||
| 2535 | mutex_lock(&module_mutex); | 2536 | mutex_lock(&module_mutex); |
| 2536 | /* Unlink carefully: kallsyms could be walking list. */ | 2537 | /* Unlink carefully: kallsyms could be walking list. */ |
| 2537 | list_del_rcu(&mod->list); | 2538 | list_del_rcu(&mod->list); |
| 2539 | already_exists: | ||
| 2538 | mutex_unlock(&module_mutex); | 2540 | mutex_unlock(&module_mutex); |
| 2539 | synchronize_sched(); | 2541 | synchronize_sched(); |
| 2540 | module_arch_cleanup(mod); | 2542 | module_arch_cleanup(mod); |
