diff options
author | Rusty Russell <rusty@rustcorp.com.au> | 2010-05-19 19:33:39 -0400 |
---|---|---|
committer | Rusty Russell <rusty@rustcorp.com.au> | 2010-05-19 04:03:39 -0400 |
commit | 480b02df3aa9f07d1c7df0cd8be7a5ca73893455 (patch) | |
tree | 27b51ecc15bb30b0d79bb0c702a0124c8b4848fc /kernel/module.c | |
parent | fedb3d27d9e8606b3867b5ae49d6258458a07a72 (diff) |
module: drop the lock while waiting for module to complete initialization.
This fixes "gave up waiting for init of module libcrc32c." which
happened at boot time due to multiple parallel module loads.
The problem was a deadlock: we wait for a module to finish
initializing, but we keep the module_lock mutex so it can't complete.
In particular, this could reasonably happen if a module does a
request_module() in its initialization routine.
So we change use_module() to return an errno rather than a bool, and if
it's -EBUSY we drop the lock and wait in the caller, then reaquire the
lock.
Reported-by: Brandon Philips <brandon@ifup.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tested-by: Brandon Philips <brandon@ifup.org>
Diffstat (limited to 'kernel/module.c')
-rw-r--r-- | kernel/module.c | 59 |
1 files changed, 37 insertions, 22 deletions
diff --git a/kernel/module.c b/kernel/module.c index e2564580f3f1..970d773aec62 100644 --- a/kernel/module.c +++ b/kernel/module.c | |||
@@ -561,33 +561,26 @@ int use_module(struct module *a, struct module *b) | |||
561 | struct module_use *use; | 561 | struct module_use *use; |
562 | int no_warn, err; | 562 | int no_warn, err; |
563 | 563 | ||
564 | if (b == NULL || already_uses(a, b)) return 1; | 564 | if (b == NULL || already_uses(a, b)) |
565 | |||
566 | /* If we're interrupted or time out, we fail. */ | ||
567 | if (wait_event_interruptible_timeout( | ||
568 | module_wq, (err = strong_try_module_get(b)) != -EBUSY, | ||
569 | 30 * HZ) <= 0) { | ||
570 | printk("%s: gave up waiting for init of module %s.\n", | ||
571 | a->name, b->name); | ||
572 | return 0; | 565 | return 0; |
573 | } | ||
574 | 566 | ||
575 | /* If strong_try_module_get() returned a different error, we fail. */ | 567 | /* If we're interrupted or time out, we fail. */ |
568 | err = strong_try_module_get(b); | ||
576 | if (err) | 569 | if (err) |
577 | return 0; | 570 | return err; |
578 | 571 | ||
579 | DEBUGP("Allocating new usage for %s.\n", a->name); | 572 | DEBUGP("Allocating new usage for %s.\n", a->name); |
580 | use = kmalloc(sizeof(*use), GFP_ATOMIC); | 573 | use = kmalloc(sizeof(*use), GFP_ATOMIC); |
581 | if (!use) { | 574 | if (!use) { |
582 | printk("%s: out of memory loading\n", a->name); | 575 | printk("%s: out of memory loading\n", a->name); |
583 | module_put(b); | 576 | module_put(b); |
584 | return 0; | 577 | return -ENOMEM; |
585 | } | 578 | } |
586 | 579 | ||
587 | use->module_which_uses = a; | 580 | use->module_which_uses = a; |
588 | list_add(&use->list, &b->modules_which_use_me); | 581 | list_add(&use->list, &b->modules_which_use_me); |
589 | no_warn = sysfs_create_link(b->holders_dir, &a->mkobj.kobj, a->name); | 582 | no_warn = sysfs_create_link(b->holders_dir, &a->mkobj.kobj, a->name); |
590 | return 1; | 583 | return 0; |
591 | } | 584 | } |
592 | EXPORT_SYMBOL_GPL(use_module); | 585 | EXPORT_SYMBOL_GPL(use_module); |
593 | 586 | ||
@@ -880,7 +873,7 @@ static inline void module_unload_free(struct module *mod) | |||
880 | 873 | ||
881 | int use_module(struct module *a, struct module *b) | 874 | int use_module(struct module *a, struct module *b) |
882 | { | 875 | { |
883 | return strong_try_module_get(b) == 0; | 876 | return strong_try_module_get(b); |
884 | } | 877 | } |
885 | EXPORT_SYMBOL_GPL(use_module); | 878 | EXPORT_SYMBOL_GPL(use_module); |
886 | 879 | ||
@@ -1051,17 +1044,39 @@ static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs, | |||
1051 | struct module *owner; | 1044 | struct module *owner; |
1052 | const struct kernel_symbol *sym; | 1045 | const struct kernel_symbol *sym; |
1053 | const unsigned long *crc; | 1046 | const unsigned long *crc; |
1047 | DEFINE_WAIT(wait); | ||
1048 | int err; | ||
1049 | long timeleft = 30 * HZ; | ||
1054 | 1050 | ||
1051 | again: | ||
1055 | sym = find_symbol(name, &owner, &crc, | 1052 | sym = find_symbol(name, &owner, &crc, |
1056 | !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true); | 1053 | !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true); |
1057 | /* use_module can fail due to OOM, | 1054 | if (!sym) |
1058 | or module initialization or unloading */ | 1055 | return NULL; |
1059 | if (sym) { | 1056 | |
1060 | if (!check_version(sechdrs, versindex, name, mod, crc, owner) | 1057 | if (!check_version(sechdrs, versindex, name, mod, crc, owner)) |
1061 | || !use_module(mod, owner)) | 1058 | return NULL; |
1062 | sym = NULL; | 1059 | |
1063 | } | 1060 | prepare_to_wait(&module_wq, &wait, TASK_INTERRUPTIBLE); |
1064 | return sym; | 1061 | err = use_module(mod, owner); |
1062 | if (likely(!err) || err != -EBUSY || signal_pending(current)) { | ||
1063 | finish_wait(&module_wq, &wait); | ||
1064 | return err ? NULL : sym; | ||
1065 | } | ||
1066 | |||
1067 | /* Module is still loading. Drop lock and wait. */ | ||
1068 | mutex_unlock(&module_mutex); | ||
1069 | timeleft = schedule_timeout(timeleft); | ||
1070 | mutex_lock(&module_mutex); | ||
1071 | finish_wait(&module_wq, &wait); | ||
1072 | |||
1073 | /* Module might be gone entirely, or replaced. Re-lookup. */ | ||
1074 | if (timeleft) | ||
1075 | goto again; | ||
1076 | |||
1077 | printk(KERN_WARNING "%s: gave up waiting for init of module %s.\n", | ||
1078 | mod->name, owner->name); | ||
1079 | return NULL; | ||
1065 | } | 1080 | } |
1066 | 1081 | ||
1067 | /* | 1082 | /* |