module: move find_module check to end
authorLinus Torvalds <torvalds@linux-foundation.org>
Sat, 5 Jun 2010 17:17:36 +0000 (11:17 -0600)
committerRusty Russell <rusty@rustcorp.com.au>
Sat, 5 Jun 2010 01:47:37 +0000 (11:17 +0930)
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>
kernel/module.c

index d293c213c22c9d91540c12237b09114ab0e83a53..28450047852a7c3aa0ce73f90810f17ea41904fd 100644 (file)
@@ -2226,11 +2226,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.  */
@@ -2509,6 +2504,12 @@ 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);
 
@@ -2535,6 +2536,7 @@ static noinline struct module *load_module(void __user *umod,
        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);