Use workqueue for call_usermodehelper
PATCH 1/1 Use workqueue for call_usermodehelper
[ Vatsa, this should solve your NUMA+HOTPLUG_CPU deadlock too, I think ]
This uses the create_singlethread_workqueue() function presented in the last patch, although it could just as easily use create_workqueue().
Name: call_usermodehelper To Use Own Workqueue
Status: Tested on 2.6.6-rc1-bk3
Depends: Misc/workqueue-singlethread.patch.gz
call_usermodehelper uses keventd to create a thread, guaranteeing a nice, clean kernel thread. Unfortunately, there is a case where call_usermodehelper is called with &bus->subsys.rwsem held (via bus_add_driver()), but keventd could be running bus_add_device(), which is blocked on the same lock. The result is deadlock, and it comes from using keventd for both.
In this case, it can be fixed by using a completely independent thread for call_usermodehelper, or an independent workqueue. Workqueues have the infrastructure we need, so we use one.
Move EXPORT_SYMBOL while we're there, too.
kernel/kmod.c +15 -18
@@ -35,11 +35,13 @@#include <linux/security.h> #include <linux/mount.h> #include <linux/kernel.h> +#include <linux/init.h> #include <asm/uaccess.h> extern int max_threads;#ifdef CONFIG_KMOD +static struct workqueue_struct *khelper_wq;/* modprobe_path is set via /proc/sys.
@@ -109,6 +111,7 @@ int request_module(const char *fmt, ...)atomic_dec (&kmod_concurrent); return ret; } +EXPORT_SYMBOL (request_module);#endif /* CONFIG_KMOD */ #ifdef CONFIG_HOTPLUG
@@ -197,9 +200,7 @@ static int wait_for_helper(void *data)return 0 ; } -/* -* This is run by keventd. -*/ +/* This is run by khelper thread */ static void __call_usermodehelper (void *data) {struct subprocess_info *sub_info = data;
@@ -249,26 +250,22 @@ int call_usermodehelper(char *path, char };DECLARE_WORK (work, __call_usermodehelper, &sub_info); -if (system_state !=SYSTEM_RUNNING ) +if (!khelper_wq)return -EBUSY ;if (path[0 ] =='\0' ) - goto out; +return 0 ; -if (current_is_keventd ()) { -/* We can't wait on keventd! */ -__call_usermodehelper (&sub_info); - }else { -schedule_work (&work); -wait_for_completion (&done); - } -out: +queue_work (khelper_wq, &work); +wait_for_completion (&done);return sub_info.retval; } -EXPORT_SYMBOL (call_usermodehelper); -#ifdef CONFIG_KMOD -EXPORT_SYMBOL (request_module); -#endif - +static __init int usermodehelper_init (void ) +{ + khelper_wq =create_singlethread_workqueue ("khelper" ); +BUG_ON (!khelper_wq); +return 0 ; +} +__initcall (usermodehelper_init);
Signature · 1 line
Anyone who quotes me in their signature is an idiot -- Rusty Russell
Quoted 4 lines · * Rusty Russell ([email protected]) wrote:
[ Vatsa, this should solve your NUMA+HOTPLUG_CPU deadlock too, I think ]
This uses the
create_singlethread_workqueue()function presented in the last patch, although it could just as easily usecreate_workqueue().
Nice, this seems like it should fixup the problem Brian was seeing too, and maintain return code from kernel_thread(), etc. instead of the async option. Brian, did you give these changes a whirl? (they're in latest -mm tree)
thanks,
-chris
Signature · 1 line
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
Quoted 4 lines · Chris Wright
Quoted 2 lines · * Rusty Russell ([email protected]) wrote:
[ Vatsa, this should solve your NUMA+HOTPLUG_CPU deadlock too, I think ]
Quoted 8 lines
Quoted 2 lines
This uses the
create_singlethread_workqueue()function presented in the last patch, although it could just as easily usecreate_workqueue().Nice, this seems like it should fixup the problem Brian was seeing too, and maintain return code from
kernel_thread(), etc. instead of the async option. Brian, did you give these changes a whirl? (they're in latest -mm tree)
I tried out the fix on my system and it seemed to work fine. However, I am having trouble recreating the deadlock with old code now that my system has been reconfigured...
-Brian
Signature · 3 lines
Brian King eServer Storage I/O IBM Linux Technology Center