From: Bart Van Assche <bvanassche@acm.org>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
James Bottomley <jbottomley@parallels.com>,
Mike Christie <michaelc@cs.wisc.edu>,
Jens Axboe <axboe@kernel.dk>, Tejun Heo <tj@kernel.org>,
Chanho Min <chanho.min@lge.com>, Hannes Reinecke <hare@suse.de>
Subject: [PATCH v6 11/13] Make scsi_remove_host() wait until error handling finished
Date: Wed, 28 Nov 2012 13:52:51 +0100 [thread overview]
Message-ID: <50B60923.3080207@acm.org> (raw)
In-Reply-To: <50B60619.4080406@acm.org>
A SCSI LLD may start cleaning up host resources as soon as
scsi_remove_host() returns. These host resources may be needed by
the LLD in an implementation of one of the eh_* functions. So if
one of the eh_* functions is in progress when scsi_remove_host()
is invoked, wait until the eh_* function has finished. Also, do
not invoke any of the eh_* functions after scsi_remove_host() has
started.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Cc: Tejun Heo <tj@kernel.org>
---
drivers/scsi/hosts.c | 2 +-
drivers/scsi/scsi_error.c | 114 ++++++++++++++++++++++++++++++++++-----------
include/scsi/scsi_host.h | 1 +
3 files changed, 89 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 7bd944e..477b8d6 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -155,7 +155,7 @@ static bool __scsi_remove_host_done(struct Scsi_Host *shost)
{
lockdep_assert_held(shost->host_lock);
- return list_empty(&shost->__devices);
+ return list_empty(&shost->__devices) && !shost->eh_active;
}
/* Test whether scsi_remove_host() may finish, and if so, wake it up. */
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c1b05a8..cab2ac3 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -536,8 +536,52 @@ static void scsi_eh_done(struct scsi_cmnd *scmd)
}
/**
+ * scsi_begin_eh - start host-related error handling
+ *
+ * Must be called before invoking any of the scsi_host_template.eh_* functions
+ * to avoid that scsi_remove_host() returns while one of these callback
+ * functions is in progress.
+ *
+ * Returns true if invoking the eh_* function is allowed and false if not.
+ * If this function returns true then scsi_end_eh() must be called eventually.
+ *
+ * Note: scsi_send_eh_cmnd() calls do not have to be protected by a
+ * scsi_begin_eh() / scsi_end_eh() pair since these operate on an unfinished
+ * block layer request. Since scsi_remove_host() waits until all SCSI devices
+ * have been removed and since blk_cleanup_queue() is invoked during SCSI
+ * device removal scsi_remove_host() won't finish while a scsi_send_eh_cmnd()
+ * call is in progress.
+ */
+static bool scsi_begin_eh(struct Scsi_Host *host)
+{
+ bool res;
+
+ spin_lock_irq(host->host_lock);
+ res = scsi_host_scan_allowed(host);
+ if (res) {
+ WARN_ON_ONCE(host->eh_active < 0 || host->eh_active > 1);
+ host->eh_active++;
+ }
+ spin_unlock_irq(host->host_lock);
+
+ return res;
+}
+
+/**
+ * scsi_end_eh - finish host-related error handling
+ */
+static void scsi_end_eh(struct Scsi_Host *host)
+{
+ spin_lock_irq(host->host_lock);
+ host->eh_active--;
+ WARN_ON_ONCE(host->eh_active < 0 || host->eh_active > 1);
+ __scsi_check_remove_host_done(host);
+ spin_unlock_irq(host->host_lock);
+}
+
+/**
* scsi_try_host_reset - ask host adapter to reset itself
- * @scmd: SCSI cmd to send hsot reset.
+ * @scmd: SCSI cmd to send host reset.
*/
static int scsi_try_host_reset(struct scsi_cmnd *scmd)
{
@@ -552,14 +596,17 @@ static int scsi_try_host_reset(struct scsi_cmnd *scmd)
if (!hostt->eh_host_reset_handler)
return FAILED;
- rtn = hostt->eh_host_reset_handler(scmd);
-
- if (rtn == SUCCESS) {
- if (!hostt->skip_settle_delay)
- ssleep(HOST_RESET_SETTLE_TIME);
- spin_lock_irqsave(host->host_lock, flags);
- scsi_report_bus_reset(host, scmd_channel(scmd));
- spin_unlock_irqrestore(host->host_lock, flags);
+ rtn = FAST_IO_FAIL;
+ if (scsi_begin_eh(host)) {
+ rtn = hostt->eh_host_reset_handler(scmd);
+ if (rtn == SUCCESS) {
+ if (!hostt->skip_settle_delay)
+ ssleep(HOST_RESET_SETTLE_TIME);
+ spin_lock_irqsave(host->host_lock, flags);
+ scsi_report_bus_reset(host, scmd_channel(scmd));
+ spin_unlock_irqrestore(host->host_lock, flags);
+ }
+ scsi_end_eh(host);
}
return rtn;
@@ -582,14 +629,17 @@ static int scsi_try_bus_reset(struct scsi_cmnd *scmd)
if (!hostt->eh_bus_reset_handler)
return FAILED;
- rtn = hostt->eh_bus_reset_handler(scmd);
-
- if (rtn == SUCCESS) {
- if (!hostt->skip_settle_delay)
- ssleep(BUS_RESET_SETTLE_TIME);
- spin_lock_irqsave(host->host_lock, flags);
- scsi_report_bus_reset(host, scmd_channel(scmd));
- spin_unlock_irqrestore(host->host_lock, flags);
+ rtn = FAST_IO_FAIL;
+ if (scsi_begin_eh(host)) {
+ rtn = hostt->eh_bus_reset_handler(scmd);
+ if (rtn == SUCCESS) {
+ if (!hostt->skip_settle_delay)
+ ssleep(BUS_RESET_SETTLE_TIME);
+ spin_lock_irqsave(host->host_lock, flags);
+ scsi_report_bus_reset(host, scmd_channel(scmd));
+ spin_unlock_irqrestore(host->host_lock, flags);
+ }
+ scsi_end_eh(host);
}
return rtn;
@@ -621,12 +671,17 @@ static int scsi_try_target_reset(struct scsi_cmnd *scmd)
if (!hostt->eh_target_reset_handler)
return FAILED;
- rtn = hostt->eh_target_reset_handler(scmd);
- if (rtn == SUCCESS) {
- spin_lock_irqsave(host->host_lock, flags);
- __starget_for_each_device(scsi_target(scmd->device), NULL,
- __scsi_report_device_reset);
- spin_unlock_irqrestore(host->host_lock, flags);
+ rtn = FAST_IO_FAIL;
+ if (scsi_begin_eh(host)) {
+ rtn = hostt->eh_target_reset_handler(scmd);
+ if (rtn == SUCCESS) {
+ spin_lock_irqsave(host->host_lock, flags);
+ __starget_for_each_device(scsi_target(scmd->device),
+ NULL,
+ __scsi_report_device_reset);
+ spin_unlock_irqrestore(host->host_lock, flags);
+ }
+ scsi_end_eh(host);
}
return rtn;
@@ -645,14 +700,19 @@ static int scsi_try_target_reset(struct scsi_cmnd *scmd)
static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
{
int rtn;
- struct scsi_host_template *hostt = scmd->device->host->hostt;
+ struct Scsi_Host *host = scmd->device->host;
+ struct scsi_host_template *hostt = host->hostt;
if (!hostt->eh_device_reset_handler)
return FAILED;
- rtn = hostt->eh_device_reset_handler(scmd);
- if (rtn == SUCCESS)
- __scsi_report_device_reset(scmd->device, NULL);
+ rtn = FAST_IO_FAIL;
+ if (scsi_begin_eh(host)) {
+ rtn = hostt->eh_device_reset_handler(scmd);
+ if (rtn == SUCCESS)
+ __scsi_report_device_reset(scmd->device, NULL);
+ scsi_end_eh(host);
+ }
return rtn;
}
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 1b7fd89..5e2fcd2 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -576,6 +576,7 @@ struct Scsi_Host {
struct task_struct * ehandler; /* Error recovery thread. */
struct completion * eh_action; /* Wait for specific actions on the
host. */
+ int eh_active;
wait_queue_head_t host_wait;
wait_queue_head_t remove_host;
struct scsi_host_template *hostt;
--
1.7.10.4
next prev parent reply other threads:[~2012-11-28 12:52 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-28 12:39 [PATCH 0/13 v6] More device removal fixes Bart Van Assche
2012-11-28 12:42 ` [PATCH v6 01/13] block: Rename queue dead flag Bart Van Assche
2012-11-28 12:43 ` [PATCH v6 02/13] block: Let blk_drain_queue() caller obtain the queue lock Bart Van Assche
2012-11-28 12:44 ` [PATCH v6 03/13] block: Avoid that request_fn is invoked on a dead queue Bart Van Assche
2012-12-02 13:23 ` Tejun Heo
2012-12-02 13:35 ` Bart Van Assche
2012-11-28 12:45 ` [PATCH v6 04/13] block: Avoid scheduling delayed work " Bart Van Assche
2012-12-02 13:26 ` Tejun Heo
2012-12-02 13:41 ` Bart Van Assche
2012-12-02 13:59 ` Tejun Heo
2012-11-28 12:46 ` [PATCH v6 05/13] block: Make blk_cleanup_queue() wait until request_fn finished Bart Van Assche
2012-12-02 13:28 ` Tejun Heo
2012-11-28 12:47 ` [PATCH v6 06/13] bsg: Remove unused function bsg_goose_queue() Bart Van Assche
2012-12-02 13:29 ` Tejun Heo
2012-11-28 12:48 ` [PATCH v6 07/13] Fix race between starved list processing and device removal Bart Van Assche
2012-12-02 13:32 ` Tejun Heo
2012-11-28 12:48 ` [PATCH v6 08/13] Remove get_device() / put_device() pair from scsi_request_fn() Bart Van Assche
2012-12-02 13:34 ` Tejun Heo
2012-11-28 12:50 ` [PATCH v6 09/13] Avoid saving/restoring interrupt state inside scsi_remove_host() Bart Van Assche
2012-12-02 13:35 ` Tejun Heo
2012-11-28 12:51 ` [PATCH v6 10/13] Make scsi_remove_host() wait for device removal Bart Van Assche
2012-12-02 13:45 ` Tejun Heo
2012-12-02 13:48 ` Tejun Heo
2012-12-03 8:23 ` Bart Van Assche
2012-12-03 16:15 ` Tejun Heo
2012-12-03 16:38 ` Bart Van Assche
2012-12-03 16:42 ` Tejun Heo
2012-12-07 7:41 ` Bart Van Assche
2012-11-28 12:52 ` Bart Van Assche [this message]
2012-12-02 13:51 ` [PATCH v6 11/13] Make scsi_remove_host() wait until error handling finished Tejun Heo
2012-11-28 12:53 ` [PATCH v6 12/13] Avoid that scsi_device_set_state() triggers a race Bart Van Assche
2012-12-02 13:53 ` Tejun Heo
2012-11-28 12:53 ` [PATCH v6 13/13] Do not queue new I/O after scsi_remove_host() started Bart Van Assche
2012-12-02 13:58 ` Tejun Heo
2012-12-02 14:02 ` [PATCH 0/13 v6] More device removal fixes Tejun Heo
2012-12-06 13:33 ` Jens Axboe
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=50B60923.3080207@acm.org \
--to=bvanassche@acm.org \
--cc=axboe@kernel.dk \
--cc=chanho.min@lge.com \
--cc=hare@suse.de \
--cc=jbottomley@parallels.com \
--cc=linux-scsi@vger.kernel.org \
--cc=michaelc@cs.wisc.edu \
--cc=tj@kernel.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).