From: Tejun Heo <tj@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>,
Ming Lei <ming.lei@canonical.com>,
Alex Riesen <raa.lkml@gmail.com>,
Alan Stern <stern@rowland.harvard.edu>,
Jens Axboe <axboe@kernel.dk>,
USB list <linux-usb@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Rusty Russell <rusty@rustcorp.com.au>
Subject: [PATCH v2 2/2] block: don't request module during elevator init
Date: Tue, 22 Jan 2013 16:51:42 -0800 [thread overview]
Message-ID: <20130123005142.GD5359@htj.dyndns.org> (raw)
In-Reply-To: <20130116213104.GT2668@htj.dyndns.org>
>From 21c3c5d2800733b7a276725b8e1ae49a694adc1a Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Tue, 22 Jan 2013 16:48:03 -0800
Block layer allows selecting an elevator which is built as a module to
be selected as system default via kernel param "elevator=". This is
achieved by automatically invoking request_module() whenever a new
block device is initialized and the elevator is not available.
This led to an interesting deadlock problem involving async and module
init. Block device probing running off an async job invokes
request_module(). While the module is being loaded, it performs
async_synchronize_full() which ends up waiting for the async job which
is already waiting for request_module() to finish, leading to
deadlock.
Invoking request_module() from deep in block device init path is
already nasty in itself. It seems best to avoid these situations from
the beginning by moving on-demand module loading out of block init
path.
The previous patch made sure that the default elevator module is
loaded early during boot if available. This patch removes on-demand
loading of the default elevator from elevator init path. As the
module would have been loaded during boot, userland-visible behavior
difference should be minimal.
For more details, please refer to the following thread.
http://thread.gmane.org/gmane.linux.kernel/1420814
v2: The bool parameter was named @request_module which conflicted with
request_module(). This built okay w/ CONFIG_MODULES because
request_module() was defined as a macro. W/o CONFIG_MODULES, it
causes build breakage. Rename the parameter to @try_loading.
Reported by Fengguang.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alex Riesen <raa.lkml@gmail.com>
Cc: Fengguang Wu <fengguang.wu@intel.com>
---
Minor revision to fix build breakage on !CONFIG_MODULES reported by
Fengguang. The patch has been queued to
wq/for-3.9-async-deadlock-fixes.
Thanks.
block/elevator.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/block/elevator.c b/block/elevator.c
index c2d61d5..603b2c1 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -100,14 +100,14 @@ static void elevator_put(struct elevator_type *e)
module_put(e->elevator_owner);
}
-static struct elevator_type *elevator_get(const char *name)
+static struct elevator_type *elevator_get(const char *name, bool try_loading)
{
struct elevator_type *e;
spin_lock(&elv_list_lock);
e = elevator_find(name);
- if (!e) {
+ if (!e && try_loading) {
spin_unlock(&elv_list_lock);
request_module("%s-iosched", name);
spin_lock(&elv_list_lock);
@@ -207,25 +207,30 @@ int elevator_init(struct request_queue *q, char *name)
q->boundary_rq = NULL;
if (name) {
- e = elevator_get(name);
+ e = elevator_get(name, true);
if (!e)
return -EINVAL;
}
+ /*
+ * Use the default elevator specified by config boot param or
+ * config option. Don't try to load modules as we could be running
+ * off async and request_module() isn't allowed from async.
+ */
if (!e && *chosen_elevator) {
- e = elevator_get(chosen_elevator);
+ e = elevator_get(chosen_elevator, false);
if (!e)
printk(KERN_ERR "I/O scheduler %s not found\n",
chosen_elevator);
}
if (!e) {
- e = elevator_get(CONFIG_DEFAULT_IOSCHED);
+ e = elevator_get(CONFIG_DEFAULT_IOSCHED, false);
if (!e) {
printk(KERN_ERR
"Default I/O scheduler not found. " \
"Using noop.\n");
- e = elevator_get("noop");
+ e = elevator_get("noop", false);
}
}
@@ -967,7 +972,7 @@ int elevator_change(struct request_queue *q, const char *name)
return -ENXIO;
strlcpy(elevator_name, name, sizeof(elevator_name));
- e = elevator_get(strstrip(elevator_name));
+ e = elevator_get(strstrip(elevator_name), true);
if (!e) {
printk(KERN_ERR "elevator: type %s not found\n", elevator_name);
return -EINVAL;
--
1.8.1
next prev parent reply other threads:[~2013-01-23 0:51 UTC|newest]
Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-11 21:04 USB device cannot be reconnected and khubd "blocked for more than 120 seconds" Alex Riesen
2013-01-12 7:48 ` Alex Riesen
2013-01-12 9:18 ` Lan Tianyu
2013-01-12 17:37 ` Alan Stern
2013-01-12 19:39 ` Alex Riesen
2013-01-12 20:33 ` Alex Riesen
2013-01-12 22:52 ` Alan Stern
2013-01-13 12:09 ` Alex Riesen
2013-01-13 16:56 ` Alan Stern
2013-01-13 17:42 ` Alex Riesen
2013-01-13 19:16 ` Oliver Neukum
2013-01-14 2:39 ` Alan Stern
2013-01-14 16:43 ` Alex Riesen
2013-01-14 3:47 ` Ming Lei
2013-01-14 7:15 ` Ming Lei
2013-01-14 17:30 ` Linus Torvalds
2013-01-14 18:04 ` Alan Stern
2013-01-14 18:34 ` Linus Torvalds
2013-01-15 1:53 ` Ming Lei
2013-01-15 6:23 ` Ming Lei
2013-01-15 17:36 ` Linus Torvalds
2013-01-15 18:18 ` Linus Torvalds
2013-01-15 23:17 ` Tejun Heo
2013-01-15 18:20 ` Alan Stern
2013-01-15 18:39 ` Tejun Heo
2013-01-15 18:32 ` Tejun Heo
2013-01-15 20:18 ` Linus Torvalds
2013-01-15 23:50 ` Tejun Heo
2013-01-16 0:25 ` Arjan van de Ven
2013-01-16 0:35 ` Tejun Heo
2013-01-16 4:01 ` Alan Stern
2013-01-16 16:12 ` Tejun Heo
2013-01-16 17:01 ` Alan Stern
2013-01-16 17:37 ` Tejun Heo
2013-01-16 17:51 ` Alan Stern
2013-01-16 0:36 ` Linus Torvalds
2013-01-16 0:40 ` Linus Torvalds
2013-01-16 2:52 ` [PATCH] module, async: async_synchronize_full() on module init iff async is used Tejun Heo
2013-01-16 3:00 ` Linus Torvalds
2013-01-16 3:25 ` Tejun Heo
2013-01-16 3:37 ` Linus Torvalds
2013-01-16 16:22 ` Arjan van de Ven
2013-01-16 16:48 ` Tejun Heo
2013-01-16 17:03 ` Arjan van de Ven
2013-01-16 17:06 ` Linus Torvalds
2013-01-16 21:30 ` [PATCH 1/2] init, block: try to load default elevator module early during boot Tejun Heo
2013-01-17 18:05 ` Linus Torvalds
2013-01-17 18:38 ` Tejun Heo
2013-01-17 18:46 ` Linus Torvalds
2013-01-17 18:59 ` Tejun Heo
2013-01-17 19:00 ` Linus Torvalds
2013-01-18 1:24 ` [PATCH 1/3] workqueue: set PF_WQ_WORKER on rescuers Tejun Heo
2013-01-18 1:25 ` [PATCH 2/3] workqueue, async: implement work/async_current_func() Tejun Heo
2013-01-18 2:47 ` Linus Torvalds
2013-01-18 2:59 ` Tejun Heo
2013-01-18 3:04 ` Tejun Heo
2013-01-18 3:18 ` Linus Torvalds
2013-01-18 3:47 ` Tejun Heo
2013-01-18 22:08 ` [PATCH 1/5] workqueue: set PF_WQ_WORKER on rescuers Tejun Heo
2013-01-18 22:10 ` [PATCH 2/5] workqueue: rename kernel/workqueue_sched.h to kernel/workqueue_internal.h Tejun Heo
2013-01-18 22:11 ` [PATCH 3/5] workqueue: move struct worker definition to workqueue_internal.h Tejun Heo
2013-01-18 22:11 ` [PATCH 4/5] workqueue: implement current_is_async() Tejun Heo
2013-01-18 22:12 ` [PATCH 5/5] async, kmod: warn on synchronous request_module() from async workers Tejun Heo
2022-06-23 5:25 ` Saravana Kannan
2013-01-18 1:27 ` [PATCH 3/3] " Tejun Heo
2013-01-23 0:53 ` [PATCH v2 1/2] init, block: try to load default elevator module early during boot Tejun Heo
2013-01-16 21:31 ` [PATCH 2/2] block: don't request module during elevator init Tejun Heo
2013-01-23 0:51 ` Tejun Heo [this message]
2013-01-16 3:30 ` [PATCH] module, async: async_synchronize_full() on module init iff async is used Ming Lei
2013-01-16 4:24 ` Rusty Russell
2013-01-16 11:36 ` Alex Riesen
2013-08-12 7:04 ` [3.8-rc3 -> 3.8-rc4 regression] " Jonathan Nieder
2013-08-12 15:09 ` Tejun Heo
2013-11-26 21:29 ` Josh Hunt
2013-11-26 21:53 ` Linus Torvalds
2013-11-26 22:12 ` Josh Hunt
2013-11-26 22:29 ` Tejun Heo
2013-12-03 14:28 ` Josh Hunt
2013-12-03 15:19 ` Tejun Heo
2013-12-04 23:01 ` Josh Hunt
2013-12-04 23:12 ` Tejun Heo
2013-11-26 22:30 ` Linus Torvalds
2013-01-16 0:44 ` USB device cannot be reconnected and khubd "blocked for more than 120 seconds" Tejun Heo
2013-01-16 17:19 ` [PATCH] async: fix __lowest_in_progress() Tejun Heo
2013-01-17 18:16 ` Linus Torvalds
2013-01-17 18:50 ` Tejun Heo
2013-01-23 0:15 ` [PATCH v2] " Tejun Heo
2013-01-23 0:22 ` Linus Torvalds
2013-01-16 3:05 ` USB device cannot be reconnected and khubd "blocked for more than 120 seconds" Ming Lei
2013-01-16 4:14 ` Linus Torvalds
2013-01-14 8:22 ` Oliver Neukum
2013-01-14 8:40 ` Ming Lei
2013-01-12 19:56 ` Alex Riesen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130123005142.GD5359@htj.dyndns.org \
--to=tj@kernel.org \
--cc=arjan@linux.intel.com \
--cc=axboe@kernel.dk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=ming.lei@canonical.com \
--cc=raa.lkml@gmail.com \
--cc=rusty@rustcorp.com.au \
--cc=stern@rowland.harvard.edu \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).