module: extract patient module check into helper
authorLuis Chamberlain <mcgrof@kernel.org>
Sat, 11 Mar 2023 04:05:52 +0000 (20:05 -0800)
committerLuis Chamberlain <mcgrof@kernel.org>
Tue, 18 Apr 2023 18:15:24 +0000 (11:15 -0700)
The patient module check inside add_unformed_module() is large
enough as we need it. It is a bit hard to read too, so just
move it to a helper and do the inverse checks first to help
shift the code and make it easier to read. The new helper then
is module_patient_check_exists().

To make this work we need to mvoe the finished_loading() up,
we do that without making any functional changes to that routine.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
kernel/module/main.c

index 32554d8a5791a21699b324a14b439bf5988fbc54..75b23257128db8856630ba5a1ab1d3ebd579905d 100644 (file)
@@ -2447,27 +2447,6 @@ static int post_relocation(struct module *mod, const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
-/* Is this module of this name done loading?  No locks held. */
-static bool finished_loading(const char *name)
-{
-       struct module *mod;
-       bool ret;
-
-       /*
-        * The module_mutex should not be a heavily contended lock;
-        * if we get the occasional sleep here, we'll go an extra iteration
-        * in the wait_event_interruptible(), which is harmless.
-        */
-       sched_annotate_sleep();
-       mutex_lock(&module_mutex);
-       mod = find_module_all(name, strlen(name), true);
-       ret = !mod || mod->state == MODULE_STATE_LIVE
-               || mod->state == MODULE_STATE_GOING;
-       mutex_unlock(&module_mutex);
-
-       return ret;
-}
-
 /* Call module constructors. */
 static void do_mod_ctors(struct module *mod)
 {
@@ -2631,6 +2610,63 @@ static int may_init_module(void)
        return 0;
 }
 
+/* Is this module of this name done loading?  No locks held. */
+static bool finished_loading(const char *name)
+{
+       struct module *mod;
+       bool ret;
+
+       /*
+        * The module_mutex should not be a heavily contended lock;
+        * if we get the occasional sleep here, we'll go an extra iteration
+        * in the wait_event_interruptible(), which is harmless.
+        */
+       sched_annotate_sleep();
+       mutex_lock(&module_mutex);
+       mod = find_module_all(name, strlen(name), true);
+       ret = !mod || mod->state == MODULE_STATE_LIVE
+               || mod->state == MODULE_STATE_GOING;
+       mutex_unlock(&module_mutex);
+
+       return ret;
+}
+
+/* Must be called with module_mutex held */
+static int module_patient_check_exists(const char *name)
+{
+       struct module *old;
+       int err = 0;
+
+       old = find_module_all(name, strlen(name), true);
+       if (old == NULL)
+               return 0;
+
+       if (old->state == MODULE_STATE_COMING ||
+           old->state == MODULE_STATE_UNFORMED) {
+               /* Wait in case it fails to load. */
+               mutex_unlock(&module_mutex);
+               err = wait_event_interruptible(module_wq,
+                                      finished_loading(name));
+               mutex_lock(&module_mutex);
+               if (err)
+                       return err;
+
+               /* The module might have gone in the meantime. */
+               old = find_module_all(name, strlen(name), true);
+       }
+
+       /*
+        * We are here only when the same module was being loaded. Do
+        * not try to load it again right now. It prevents long delays
+        * caused by serialized module load failures. It might happen
+        * when more devices of the same type trigger load of
+        * a particular module.
+        */
+       if (old && old->state == MODULE_STATE_LIVE)
+               return -EEXIST;
+       return -EBUSY;
+}
+
 /*
  * We try to place it in the list now to make sure it's unique before
  * we dedicate too many resources.  In particular, temporary percpu
@@ -2639,41 +2675,14 @@ static int may_init_module(void)
 static int add_unformed_module(struct module *mod)
 {
        int err;
-       struct module *old;
 
        mod->state = MODULE_STATE_UNFORMED;
 
        mutex_lock(&module_mutex);
-       old = find_module_all(mod->name, strlen(mod->name), true);
-       if (old != NULL) {
-               if (old->state == MODULE_STATE_COMING
-                   || old->state == MODULE_STATE_UNFORMED) {
-                       /* Wait in case it fails to load. */
-                       mutex_unlock(&module_mutex);
-                       err = wait_event_interruptible(module_wq,
-                                              finished_loading(mod->name));
-                       if (err)
-                               goto out_unlocked;
-
-                       /* The module might have gone in the meantime. */
-                       mutex_lock(&module_mutex);
-                       old = find_module_all(mod->name, strlen(mod->name),
-                                             true);
-               }
-
-               /*
-                * We are here only when the same module was being loaded. Do
-                * not try to load it again right now. It prevents long delays
-                * caused by serialized module load failures. It might happen
-                * when more devices of the same type trigger load of
-                * a particular module.
-                */
-               if (old && old->state == MODULE_STATE_LIVE)
-                       err = -EEXIST;
-               else
-                       err = -EBUSY;
+       err = module_patient_check_exists(mod->name);
+       if (err)
                goto out;
-       }
+
        mod_update_bounds(mod);
        list_add_rcu(&mod->list, &modules);
        mod_tree_insert(mod);
@@ -2681,7 +2690,6 @@ static int add_unformed_module(struct module *mod)
 
 out:
        mutex_unlock(&module_mutex);
-out_unlocked:
        return err;
 }