public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] module: create a request_module_nowait()
@ 2009-02-08 18:42 Arjan van de Ven
  2009-02-08 18:42 ` [PATCH 2/3] use the new request_module_nowait() in the hid driver Arjan van de Ven
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Arjan van de Ven @ 2009-02-08 18:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: rusty, jkosina, mchehab

>From 48cd2d5171ae8fb40ef6092a566b4889b9ac72b7 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 1 Feb 2009 12:02:39 -0800
Subject: [PATCH] module: create a request_module_nowait()

There seems to be a common pattern in the kernel where drivers want to
call request_module() from inside a module_init() function. Currently
this would deadlock.

As a result, several drivers go through hoops like scheduling things via
kevent, or creating custom work queues (because kevent can deadlock on them).

This patch changes this to use a request_module_nowait() function macro instead,
which just fires the modprobe off but doesn't wait for it, and thus avoids the
original deadlock entirely.

On my laptop this already results in one less kernel thread running..

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 include/linux/kmod.h |    9 ++++++---
 kernel/kmod.c        |   10 ++++++----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 92213a9..73b2b56 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -29,10 +29,13 @@
 #ifdef CONFIG_MODULES
 /* modprobe exit status on success, -ve on error.  Return value
  * usually useless though. */
-extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2)));
-#define try_then_request_module(x, mod...) ((x) ?: (request_module(mod), (x)))
+extern int __request_module(int wait, const char *name, ...) __attribute__ ((format (printf, 2, 3)));
+#define request_module(mod...) __request_module(1, mod)
+#define request_module_nowait(mod...) __request_module(0, mod)
+#define try_then_request_module(x, mod...) ((x) ?: (__request_module(0, mod), (x)))
 #else
-static inline int request_module(const char * name, ...) { return -ENOSYS; }
+static inline int request_module(const char *name, ...) { return -ENOSYS; }
+static inline int request_module_nowait(const char *name, ...) { return -ENOSYS; }
 #define try_then_request_module(x, mod...) (x)
 #endif
 
diff --git a/kernel/kmod.c b/kernel/kmod.c
index a27a5f6..3b90ea5 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -50,7 +50,8 @@ static struct workqueue_struct *khelper_wq;
 char modprobe_path[KMOD_PATH_LEN] = "/sbin/modprobe";
 
 /**
- * request_module - try to load a kernel module
+ * __request_module - try to load a kernel module
+ * @wait: wait (or not) for the operation to complete
  * @fmt: printf style format string for the name of the module
  * @...: arguments as specified in the format string
  *
@@ -63,7 +64,8 @@ char modprobe_path[KMOD_PATH_LEN] = "/sbin/modprobe";
  * If module auto-loading support is disabled then this function
  * becomes a no-operation.
  */
-int request_module(const char *fmt, ...)
+
+int __request_module(int wait, const char *fmt, ...)
 {
 	va_list args;
 	char module_name[MODULE_NAME_LEN];
@@ -108,11 +110,11 @@ int request_module(const char *fmt, ...)
 		return -ENOMEM;
 	}
 
-	ret = call_usermodehelper(modprobe_path, argv, envp, 1);
+	ret = call_usermodehelper(modprobe_path, argv, envp, wait);
 	atomic_dec(&kmod_concurrent);
 	return ret;
 }
-EXPORT_SYMBOL(request_module);
+EXPORT_SYMBOL(__request_module);
 #endif /* CONFIG_MODULES */
 
 struct subprocess_info {
-- 
1.6.0.6



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] use the new request_module_nowait() in the hid driver
  2009-02-08 18:42 [PATCH 1/3] module: create a request_module_nowait() Arjan van de Ven
@ 2009-02-08 18:42 ` Arjan van de Ven
  2009-02-10 17:23   ` Jiri Kosina
  2009-02-08 18:43 ` [PATCH 3/3] use the new request_module_nowait() in drivers/media Arjan van de Ven
  2009-02-09  6:32 ` [PATCH 1/3] module: create a request_module_nowait() Rusty Russell
  2 siblings, 1 reply; 12+ messages in thread
From: Arjan van de Ven @ 2009-02-08 18:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Arjan van de Ven, rusty, jkosina, mchehab


>From b8c36ec88e272ba2e755e6104579f1e751ee81f8 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 1 Feb 2009 12:02:39 -0800
Subject: [PATCH] use the new request_module_nowait() in the hid driver

Now that there is a request_module_nowait(), use it in the hid driver.
This gets rid of a kernel thread, and also greatly simplifies the code...

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/hid/hid-core.c |   19 +------------------
 1 files changed, 1 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 6cad69e..d51331f 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1812,15 +1812,6 @@ void hid_unregister_driver(struct hid_driver *hdrv)
 }
 EXPORT_SYMBOL_GPL(hid_unregister_driver);
 
-#ifdef CONFIG_HID_COMPAT
-static void hid_compat_load(struct work_struct *ws)
-{
-	request_module("hid-dummy");
-}
-static DECLARE_WORK(hid_compat_work, hid_compat_load);
-static struct workqueue_struct *hid_compat_wq;
-#endif
-
 static int __init hid_init(void)
 {
 	int ret;
@@ -1836,12 +1827,7 @@ static int __init hid_init(void)
 		goto err_bus;
 
 #ifdef CONFIG_HID_COMPAT
-	hid_compat_wq = create_singlethread_workqueue("hid_compat");
-	if (!hid_compat_wq) {
-		hidraw_exit();
-		goto err;
-	}
-	queue_work(hid_compat_wq, &hid_compat_work);
+	request_module_nowait("hid-dummy");
 #endif
 
 	return 0;
@@ -1853,9 +1839,6 @@ err:
 
 static void __exit hid_exit(void)
 {
-#ifdef CONFIG_HID_COMPAT
-	destroy_workqueue(hid_compat_wq);
-#endif
 	hidraw_exit();
 	bus_unregister(&hid_bus_type);
 }
-- 
1.6.0.6


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] use the new request_module_nowait() in drivers/media
  2009-02-08 18:42 [PATCH 1/3] module: create a request_module_nowait() Arjan van de Ven
  2009-02-08 18:42 ` [PATCH 2/3] use the new request_module_nowait() in the hid driver Arjan van de Ven
@ 2009-02-08 18:43 ` Arjan van de Ven
  2009-02-08 19:00   ` Arjan van de Ven
  2009-02-09  6:32 ` [PATCH 1/3] module: create a request_module_nowait() Rusty Russell
  2 siblings, 1 reply; 12+ messages in thread
From: Arjan van de Ven @ 2009-02-08 18:43 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, rusty, jkosina, mchehab

>From 2311993ceed96ec0bb023419120d9aeada205242 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 1 Feb 2009 12:02:39 -0800
Subject: [PATCH] use the new request_module_nowait() in drivers/media

Several drivers/media/video drivers use keventd to load modules
to avoid the "load a module from the module init code" deadlock.

Now that we have request_module_nowait() this can be simplified
greatly.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/media/video/bt8xx/bttv-driver.c    |    8 +-------
 drivers/media/video/bt8xx/bttvp.h          |    3 ---
 drivers/media/video/cx88/cx88-mpeg.c       |   14 +++-----------
 drivers/media/video/cx88/cx88.h            |    1 -
 drivers/media/video/em28xx/em28xx-cards.c  |   17 ++++-------------
 drivers/media/video/em28xx/em28xx.h        |    2 --
 drivers/media/video/saa7134/saa7134-core.c |   19 ++++++-------------
 drivers/media/video/saa7134/saa7134.h      |    2 --
 8 files changed, 14 insertions(+), 52 deletions(-)

diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c
index c71f394..f470318 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -175,15 +175,9 @@ static DEVICE_ATTR(card, S_IRUGO, show_card, NULL);
 /* ----------------------------------------------------------------------- */
 /* dvb auto-load setup                                                     */
 #if defined(CONFIG_MODULES) && defined(MODULE)
-static void request_module_async(struct work_struct *work)
-{
-	request_module("dvb-bt8xx");
-}
-
 static void request_modules(struct bttv *dev)
 {
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+	request_module_async("dvb-bt8xx");
 }
 #else
 #define request_modules(dev)
diff --git a/drivers/media/video/bt8xx/bttvp.h b/drivers/media/video/bt8xx/bttvp.h
index 199a4d2..767d483 100644
--- a/drivers/media/video/bt8xx/bttvp.h
+++ b/drivers/media/video/bt8xx/bttvp.h
@@ -439,9 +439,6 @@ struct bttv {
 	unsigned int users;
 	struct bttv_fh init;
 
-	/* used to make dvb-bt8xx autoloadable */
-	struct work_struct request_module_wk;
-
 	/* Default (0) and current (1) video capturing and overlay
 	   cropping parameters in bttv_tvnorm.cropcap units. Protected
 	   by bttv.lock. */
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index b295b76..7215344 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -50,20 +50,12 @@ MODULE_PARM_DESC(debug,"enable debug messages [mpeg]");
 	printk(KERN_DEBUG "%s/2-mpeg: " fmt, core->name, ## arg)
 
 #if defined(CONFIG_MODULES) && defined(MODULE)
-static void request_module_async(struct work_struct *work)
+static void request_modules(struct cx8802_dev *dev)
 {
-	struct cx8802_dev *dev=container_of(work, struct cx8802_dev, request_module_wk);
-
 	if (dev->core->board.mpeg & CX88_MPEG_DVB)
-		request_module("cx88-dvb");
+		request_module_async("cx88-dvb");
 	if (dev->core->board.mpeg & CX88_MPEG_BLACKBIRD)
-		request_module("cx88-blackbird");
-}
-
-static void request_modules(struct cx8802_dev *dev)
-{
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+		request_module_async("cx88-blackbird");
 }
 #else
 #define request_modules(dev)
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index 6025fdd..242cd62 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -510,7 +510,6 @@ struct cx8802_dev {
 
 	/* List of attached drivers */
 	struct list_head	   drvlist;
-	struct work_struct	   request_module_wk;
 };
 
 /* ----------------------------------------------------------- */
diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c
index 3b3ca3f..890f209 100644
--- a/drivers/media/video/em28xx/em28xx-cards.c
+++ b/drivers/media/video/em28xx/em28xx-cards.c
@@ -1809,24 +1809,15 @@ void em28xx_card_setup(struct em28xx *dev)
 
 
 #if defined(CONFIG_MODULES) && defined(MODULE)
-static void request_module_async(struct work_struct *work)
+static void request_modules(struct em28xx *dev)
 {
-	struct em28xx *dev = container_of(work,
-			     struct em28xx, request_module_wk);
-
 	if (dev->has_audio_class)
-		request_module("snd-usb-audio");
+		request_module_async("snd-usb-audio");
 	else if (dev->has_alsa_audio)
-		request_module("em28xx-alsa");
+		request_module_async("em28xx-alsa");
 
 	if (dev->board.has_dvb)
-		request_module("em28xx-dvb");
-}
-
-static void request_modules(struct em28xx *dev)
-{
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+		request_module_async("em28xx-dvb");
 }
 #else
 #define request_modules(dev)
diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h
index dd2cd36..164a543 100644
--- a/drivers/media/video/em28xx/em28xx.h
+++ b/drivers/media/video/em28xx/em28xx.h
@@ -498,8 +498,6 @@ struct em28xx {
 	enum em28xx_dev_state state;
 	enum em28xx_io_method io;
 
-	struct work_struct         request_module_wk;
-
 	/* locks */
 	struct mutex lock;
 	struct mutex ctrl_urb_lock;	/* protects urb_buf */
diff --git a/drivers/media/video/saa7134/saa7134-core.c b/drivers/media/video/saa7134/saa7134-core.c
index 99221d7..753836c 100644
--- a/drivers/media/video/saa7134/saa7134-core.c
+++ b/drivers/media/video/saa7134/saa7134-core.c
@@ -149,23 +149,16 @@ void saa7134_set_gpio(struct saa7134_dev *dev, int bit_no, int value)
 /* delayed request_module                                      */
 
 #if defined(CONFIG_MODULES) && defined(MODULE)
-
-static void request_module_async(struct work_struct *work){
-	struct saa7134_dev* dev = container_of(work, struct saa7134_dev, request_module_wk);
+static void request_submodules(struct saa7134_dev *dev)
+{
 	if (card_is_empress(dev))
-		request_module("saa7134-empress");
+		request_module_async("saa7134-empress");
 	if (card_is_dvb(dev))
-		request_module("saa7134-dvb");
+		request_module_async("saa7134-dvb");
 	if (alsa)
-		request_module("saa7134-alsa");
+		request_module_async("saa7134-alsa");
 	if (oss)
-		request_module("saa7134-oss");
-}
-
-static void request_submodules(struct saa7134_dev *dev)
-{
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+		request_module_async("saa7134-oss");
 }
 
 #else
diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h
index 14ee265..d13d16f 100644
--- a/drivers/media/video/saa7134/saa7134.h
+++ b/drivers/media/video/saa7134/saa7134.h
@@ -482,8 +482,6 @@ struct saa7134_dev {
 	struct mutex               lock;
 	spinlock_t                 slock;
 	struct v4l2_prio_state     prio;
-	/* workstruct for loading modules */
-	struct work_struct request_module_wk;
 
 	/* insmod option/autodetected */
 	int                        autodetected;
-- 
1.6.0.6



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] use the new request_module_nowait() in drivers/media
  2009-02-08 18:43 ` [PATCH 3/3] use the new request_module_nowait() in drivers/media Arjan van de Ven
@ 2009-02-08 19:00   ` Arjan van de Ven
  2009-02-09 16:34     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 12+ messages in thread
From: Arjan van de Ven @ 2009-02-08 19:00 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, rusty, jkosina, mchehab

On Sun, 8 Feb 2009 10:43:14 -0800
Arjan van de Ven <arjan@infradead.org> wrote:

blargh I sent an older version of this patch
I meant to send this one

>From 2311993ceed96ec0bb023419120d9aeada205242 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Subject: [PATCH] use the new request_module_nowait() in drivers/media

Several drivers/media/video drivers use keventd to load modules
to avoid the "load a module from the module init code" deadlock.

Now that we have request_module_nowait() this can be simplified
greatly.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/media/video/bt8xx/bttv-driver.c    |    8 +-------
 drivers/media/video/bt8xx/bttvp.h          |    3 ---
 drivers/media/video/cx88/cx88-mpeg.c       |   14 +++-----------
 drivers/media/video/cx88/cx88.h            |    1 -
 drivers/media/video/em28xx/em28xx-cards.c  |   17 ++++-------------
 drivers/media/video/em28xx/em28xx.h        |    2 --
 drivers/media/video/saa7134/saa7134-core.c |   19 ++++++-------------
 drivers/media/video/saa7134/saa7134.h      |    2 --
 8 files changed, 14 insertions(+), 52 deletions(-)

diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c
index c71f394..f470318 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -175,15 +175,9 @@ static DEVICE_ATTR(card, S_IRUGO, show_card, NULL);
 /* ----------------------------------------------------------------------- */
 /* dvb auto-load setup                                                     */
 #if defined(CONFIG_MODULES) && defined(MODULE)
-static void request_module_async(struct work_struct *work)
-{
-	request_module("dvb-bt8xx");
-}
-
 static void request_modules(struct bttv *dev)
 {
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+	request_module_nowait("dvb-bt8xx");
 }
 #else
 #define request_modules(dev)
diff --git a/drivers/media/video/bt8xx/bttvp.h b/drivers/media/video/bt8xx/bttvp.h
index 199a4d2..767d483 100644
--- a/drivers/media/video/bt8xx/bttvp.h
+++ b/drivers/media/video/bt8xx/bttvp.h
@@ -439,9 +439,6 @@ struct bttv {
 	unsigned int users;
 	struct bttv_fh init;
 
-	/* used to make dvb-bt8xx autoloadable */
-	struct work_struct request_module_wk;
-
 	/* Default (0) and current (1) video capturing and overlay
 	   cropping parameters in bttv_tvnorm.cropcap units. Protected
 	   by bttv.lock. */
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index b295b76..7215344 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -50,20 +50,12 @@ MODULE_PARM_DESC(debug,"enable debug messages [mpeg]");
 	printk(KERN_DEBUG "%s/2-mpeg: " fmt, core->name, ## arg)
 
 #if defined(CONFIG_MODULES) && defined(MODULE)
-static void request_module_async(struct work_struct *work)
+static void request_modules(struct cx8802_dev *dev)
 {
-	struct cx8802_dev *dev=container_of(work, struct cx8802_dev, request_module_wk);
-
 	if (dev->core->board.mpeg & CX88_MPEG_DVB)
-		request_module("cx88-dvb");
+		request_module_nowait("cx88-dvb");
 	if (dev->core->board.mpeg & CX88_MPEG_BLACKBIRD)
-		request_module("cx88-blackbird");
-}
-
-static void request_modules(struct cx8802_dev *dev)
-{
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+		request_module_nowait("cx88-blackbird");
 }
 #else
 #define request_modules(dev)
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index 6025fdd..242cd62 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -510,7 +510,6 @@ struct cx8802_dev {
 
 	/* List of attached drivers */
 	struct list_head	   drvlist;
-	struct work_struct	   request_module_wk;
 };
 
 /* ----------------------------------------------------------- */
diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c
index 3b3ca3f..890f209 100644
--- a/drivers/media/video/em28xx/em28xx-cards.c
+++ b/drivers/media/video/em28xx/em28xx-cards.c
@@ -1809,24 +1809,15 @@ void em28xx_card_setup(struct em28xx *dev)
 
 
 #if defined(CONFIG_MODULES) && defined(MODULE)
-static void request_module_async(struct work_struct *work)
+static void request_modules(struct em28xx *dev)
 {
-	struct em28xx *dev = container_of(work,
-			     struct em28xx, request_module_wk);
-
 	if (dev->has_audio_class)
-		request_module("snd-usb-audio");
+		request_module_nowait("snd-usb-audio");
 	else if (dev->has_alsa_audio)
-		request_module("em28xx-alsa");
+		request_module_nowait("em28xx-alsa");
 
 	if (dev->board.has_dvb)
-		request_module("em28xx-dvb");
-}
-
-static void request_modules(struct em28xx *dev)
-{
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+		request_module_nowait("em28xx-dvb");
 }
 #else
 #define request_modules(dev)
diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h
index dd2cd36..164a543 100644
--- a/drivers/media/video/em28xx/em28xx.h
+++ b/drivers/media/video/em28xx/em28xx.h
@@ -498,8 +498,6 @@ struct em28xx {
 	enum em28xx_dev_state state;
 	enum em28xx_io_method io;
 
-	struct work_struct         request_module_wk;
-
 	/* locks */
 	struct mutex lock;
 	struct mutex ctrl_urb_lock;	/* protects urb_buf */
diff --git a/drivers/media/video/saa7134/saa7134-core.c b/drivers/media/video/saa7134/saa7134-core.c
index 99221d7..753836c 100644
--- a/drivers/media/video/saa7134/saa7134-core.c
+++ b/drivers/media/video/saa7134/saa7134-core.c
@@ -149,23 +149,16 @@ void saa7134_set_gpio(struct saa7134_dev *dev, int bit_no, int value)
 /* delayed request_module                                      */
 
 #if defined(CONFIG_MODULES) && defined(MODULE)
-
-static void request_module_async(struct work_struct *work){
-	struct saa7134_dev* dev = container_of(work, struct saa7134_dev, request_module_wk);
+static void request_submodules(struct saa7134_dev *dev)
+{
 	if (card_is_empress(dev))
-		request_module("saa7134-empress");
+		request_module_nowait("saa7134-empress");
 	if (card_is_dvb(dev))
-		request_module("saa7134-dvb");
+		request_module_nowait("saa7134-dvb");
 	if (alsa)
-		request_module("saa7134-alsa");
+		request_module_nowait("saa7134-alsa");
 	if (oss)
-		request_module("saa7134-oss");
-}
-
-static void request_submodules(struct saa7134_dev *dev)
-{
-	INIT_WORK(&dev->request_module_wk, request_module_async);
-	schedule_work(&dev->request_module_wk);
+		request_module_nowait("saa7134-oss");
 }
 
 #else
diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h
index 14ee265..d13d16f 100644
--- a/drivers/media/video/saa7134/saa7134.h
+++ b/drivers/media/video/saa7134/saa7134.h
@@ -482,8 +482,6 @@ struct saa7134_dev {
 	struct mutex               lock;
 	spinlock_t                 slock;
 	struct v4l2_prio_state     prio;
-	/* workstruct for loading modules */
-	struct work_struct request_module_wk;
 
 	/* insmod option/autodetected */
 	int                        autodetected;
-- 
1.6.0.6



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] module: create a request_module_nowait()
  2009-02-08 18:42 [PATCH 1/3] module: create a request_module_nowait() Arjan van de Ven
  2009-02-08 18:42 ` [PATCH 2/3] use the new request_module_nowait() in the hid driver Arjan van de Ven
  2009-02-08 18:43 ` [PATCH 3/3] use the new request_module_nowait() in drivers/media Arjan van de Ven
@ 2009-02-09  6:32 ` Rusty Russell
  2009-02-09 15:25   ` Arjan van de Ven
  2 siblings, 1 reply; 12+ messages in thread
From: Rusty Russell @ 2009-02-09  6:32 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, jkosina, mchehab

On Monday 09 February 2009 05:12:01 Arjan van de Ven wrote:
> This patch changes this to use a request_module_nowait() function macro instead,
> which just fires the modprobe off but doesn't wait for it, and thus avoids the
> original deadlock entirely.

Nice, applied.  Made one change; I've taken to using the new-fangled bool
type for flag args, for slight extra clarity.

(Which uncovered another interesting issue, which lead to several more
 patches).

Thanks,
Rusty.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] module: create a request_module_nowait()
  2009-02-09  6:32 ` [PATCH 1/3] module: create a request_module_nowait() Rusty Russell
@ 2009-02-09 15:25   ` Arjan van de Ven
  2009-02-10  3:11     ` Rusty Russell
  0 siblings, 1 reply; 12+ messages in thread
From: Arjan van de Ven @ 2009-02-09 15:25 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, jkosina, mchehab

On Mon, 9 Feb 2009 17:02:17 +1030
Rusty Russell <rusty@rustcorp.com.au> wrote:

> On Monday 09 February 2009 05:12:01 Arjan van de Ven wrote:
> > This patch changes this to use a request_module_nowait() function
> > macro instead, which just fires the modprobe off but doesn't wait
> > for it, and thus avoids the original deadlock entirely.
> 
> Nice, applied.  Made one change; I've taken to using the new-fangled
> bool type for flag args, for slight extra clarity.
> 
> (Which uncovered another interesting issue, which lead to several more
>  patches).
> 

for this case that's a bit unfortunate though.

the call usermode helper code takes 3 values, -1, 0 and 1.
Right now the code only uses 0 and 1, but I've been looking at
using -1 as well in this exact codepath in case there is a problem with
the 0 option (the 0 option waits for the exec but not the result, the
-1 waits for nothing)

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] use the new request_module_nowait() in drivers/media
  2009-02-08 19:00   ` Arjan van de Ven
@ 2009-02-09 16:34     ` Mauro Carvalho Chehab
  2009-02-10 13:32       ` Rusty Russell
  0 siblings, 1 reply; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2009-02-09 16:34 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, rusty, jkosina, linux-media

On Sun, 8 Feb 2009 11:00:52 -0800
Arjan van de Ven <arjan@infradead.org> wrote:

> On Sun, 8 Feb 2009 10:43:14 -0800
> Arjan van de Ven <arjan@infradead.org> wrote:
> 
> blargh I sent an older version of this patch
> I meant to send this one
> 
> From 2311993ceed96ec0bb023419120d9aeada205242 Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@linux.intel.com>
> Subject: [PATCH] use the new request_module_nowait() in drivers/media
> 
> Several drivers/media/video drivers use keventd to load modules
> to avoid the "load a module from the module init code" deadlock.
> 
> Now that we have request_module_nowait() this can be simplified
> greatly.
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>

Cheers,
Mauro

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] module: create a request_module_nowait()
  2009-02-09 15:25   ` Arjan van de Ven
@ 2009-02-10  3:11     ` Rusty Russell
  0 siblings, 0 replies; 12+ messages in thread
From: Rusty Russell @ 2009-02-10  3:11 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, jkosina, mchehab

On Tuesday 10 February 2009 01:55:31 Arjan van de Ven wrote:
> On Mon, 9 Feb 2009 17:02:17 +1030
> Rusty Russell <rusty@rustcorp.com.au> wrote:
> > Nice, applied.  Made one change; I've taken to using the new-fangled
> > bool type for flag args, for slight extra clarity.
> 
> for this case that's a bit unfortunate though.
> 
> the call usermode helper code takes 3 values, -1, 0 and 1.

Yes, we could re-use the umh enum if we want to change it.

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] use the new request_module_nowait() in drivers/media
  2009-02-09 16:34     ` Mauro Carvalho Chehab
@ 2009-02-10 13:32       ` Rusty Russell
  2009-02-10 18:37         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 12+ messages in thread
From: Rusty Russell @ 2009-02-10 13:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Arjan van de Ven, linux-kernel, jkosina, linux-media

On Tuesday 10 February 2009 03:04:56 Mauro Carvalho Chehab wrote:
> On Sun, 8 Feb 2009 11:00:52 -0800
> Arjan van de Ven <arjan@infradead.org> wrote:
> > From: Arjan van de Ven <arjan@linux.intel.com>
> > Subject: [PATCH] use the new request_module_nowait() in drivers/media
...
> Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> 
> Cheers,
> Mauro

Thanks Mauro.  Since I have the prereq, I've taken this into my tree too.

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] use the new request_module_nowait() in the hid driver
  2009-02-08 18:42 ` [PATCH 2/3] use the new request_module_nowait() in the hid driver Arjan van de Ven
@ 2009-02-10 17:23   ` Jiri Kosina
  2009-02-10 22:18     ` Rusty Russell
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Kosina @ 2009-02-10 17:23 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, rusty, mchehab

On Sun, 8 Feb 2009, Arjan van de Ven wrote:

> From b8c36ec88e272ba2e755e6104579f1e751ee81f8 Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Sun, 1 Feb 2009 12:02:39 -0800
> Subject: [PATCH] use the new request_module_nowait() in the hid driver
> 
> Now that there is a request_module_nowait(), use it in the hid driver.
> This gets rid of a kernel thread, and also greatly simplifies the code...
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

Acked-by: Jiri Kosina <jkosina@suse.cz>

Rusty, could you please take this altogether with the patch introducing 
request_module_nowait() itself?

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] use the new request_module_nowait() in drivers/media
  2009-02-10 13:32       ` Rusty Russell
@ 2009-02-10 18:37         ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2009-02-10 18:37 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Arjan van de Ven, linux-kernel, jkosina, linux-media

On Wed, 11 Feb 2009 00:02:15 +1030
Rusty Russell <rusty@rustcorp.com.au> wrote:

> On Tuesday 10 February 2009 03:04:56 Mauro Carvalho Chehab wrote:
> > On Sun, 8 Feb 2009 11:00:52 -0800
> > Arjan van de Ven <arjan@infradead.org> wrote:
> > > From: Arjan van de Ven <arjan@linux.intel.com>
> > > Subject: [PATCH] use the new request_module_nowait() in drivers/media
> ...
> > Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> > 
> > Cheers,
> > Mauro
> 
> Thanks Mauro.  Since I have the prereq, I've taken this into my tree too.

OK.

Cheers,
Mauro

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] use the new request_module_nowait() in the hid driver
  2009-02-10 17:23   ` Jiri Kosina
@ 2009-02-10 22:18     ` Rusty Russell
  0 siblings, 0 replies; 12+ messages in thread
From: Rusty Russell @ 2009-02-10 22:18 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Arjan van de Ven, linux-kernel, mchehab

On Wednesday 11 February 2009 03:53:29 Jiri Kosina wrote:
> Acked-by: Jiri Kosina <jkosina@suse.cz>
> 
> Rusty, could you please take this altogether with the patch introducing 
> request_module_nowait() itself?

Yep, done!

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2009-02-10 22:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-08 18:42 [PATCH 1/3] module: create a request_module_nowait() Arjan van de Ven
2009-02-08 18:42 ` [PATCH 2/3] use the new request_module_nowait() in the hid driver Arjan van de Ven
2009-02-10 17:23   ` Jiri Kosina
2009-02-10 22:18     ` Rusty Russell
2009-02-08 18:43 ` [PATCH 3/3] use the new request_module_nowait() in drivers/media Arjan van de Ven
2009-02-08 19:00   ` Arjan van de Ven
2009-02-09 16:34     ` Mauro Carvalho Chehab
2009-02-10 13:32       ` Rusty Russell
2009-02-10 18:37         ` Mauro Carvalho Chehab
2009-02-09  6:32 ` [PATCH 1/3] module: create a request_module_nowait() Rusty Russell
2009-02-09 15:25   ` Arjan van de Ven
2009-02-10  3:11     ` Rusty Russell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox