mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: linux1394-devel@lists.sourceforge.net
Cc: Ben Collins <bcollins@ubuntu.com>, Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2.6.18-rc4-mm1 8/8] ieee1394: sbp2: prevent rare deadlock in shutdown
Date: Mon, 14 Aug 2006 19:53:18 +0200 (CEST)	[thread overview]
Message-ID: <tkrat.fee9f136ea5d8e54@s5r6.in-berlin.de> (raw)
In-Reply-To: <tkrat.9b22090d7dd50b12@s5r6.in-berlin.de>

Scsi_remove_device() may go into uninterruptible sleep if blocked.
Therefore sbp2_remove() unblocks the Scsi_Host before the device is
requested to be removed.  But there could be another 1394 bus reset
after that which would block the host again.  The 1394 subsystem won't
call sbp2_update() concurrently to sbp2_remove(), which is why there is
no chance for sbp2_remove() to be unblocked by sbp2_update().

The fix is to tell sbp2's bus reset handler when a device is to be shut
down so that it skips scsi_block_requests() on that host.  As before,
any new commands after a reset without reconnect will be failed quickly
by sbp2scsi_queuecommand().

In the long term, means to go without scsi_block_requests() should be
found.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/sbp2.c |   21 +++++++++++----------
 drivers/ieee1394/sbp2.h |    9 ++++++++-
 2 files changed, 19 insertions(+), 11 deletions(-)

Index: linux/drivers/ieee1394/sbp2.h
===================================================================
--- linux.orig/drivers/ieee1394/sbp2.h	2006-08-14 12:46:13.000000000 +0200
+++ linux/drivers/ieee1394/sbp2.h	2006-08-14 17:21:32.000000000 +0200
@@ -348,10 +348,17 @@ struct scsi_id_instance_data {
 	/* Device specific workarounds/brokeness */
 	unsigned workarounds;
 
-	atomic_t unfinished_reset;
+	atomic_t state;
 	struct work_struct protocol_work;
 };
 
+/* For use in scsi_id_instance_data.state */
+enum sbp2lu_state_types {
+	SBP2LU_STATE_RUNNING,		/* all normal */
+	SBP2LU_STATE_IN_RESET,		/* between bus reset and reconnect */
+	SBP2LU_STATE_IN_SHUTDOWN	/* when sbp2_remove was called */
+};
+
 /* Sbp2 host data structure (one per IEEE1394 host) */
 struct sbp2scsi_host_info {
 	struct hpsb_host *host;		/* IEEE1394 host */
Index: linux/drivers/ieee1394/sbp2.c
===================================================================
--- linux.orig/drivers/ieee1394/sbp2.c	2006-08-14 12:46:15.000000000 +0200
+++ linux/drivers/ieee1394/sbp2.c	2006-08-14 12:46:16.000000000 +0200
@@ -488,7 +488,7 @@ static void sbp2util_notify_fetch_agent(
 	 * There is a small window after a bus reset within which the node
 	 * entry's generation is current but the reconnect wasn't completed.
 	 */
-	if (atomic_read(&scsi_id->unfinished_reset))
+	if (unlikely(atomic_read(&scsi_id->state) == SBP2LU_STATE_IN_RESET))
 		return;
 
 	if (hpsb_node_write(scsi_id->ne,
@@ -499,7 +499,7 @@ static void sbp2util_notify_fetch_agent(
 	 * Now accept new SCSI commands, unless a bus reset happended during
 	 * hpsb_node_write.
 	 */
-	if (!atomic_read(&scsi_id->unfinished_reset))
+	if (likely(atomic_read(&scsi_id->state) != SBP2LU_STATE_IN_RESET))
 		scsi_unblock_requests(scsi_id->scsi_host);
 }
 
@@ -766,7 +766,7 @@ static int sbp2_remove(struct device *de
 			sbp2scsi_complete_all_commands(scsi_id, DID_NO_CONNECT);
 		/* scsi_remove_device() will trigger shutdown functions of SCSI
 		 * highlevel drivers which would deadlock if blocked. */
-		atomic_set(&scsi_id->unfinished_reset, 0);
+		atomic_set(&scsi_id->state, SBP2LU_STATE_IN_SHUTDOWN);
 		scsi_unblock_requests(scsi_id->scsi_host);
 	}
 	sdev = scsi_id->sdev;
@@ -821,7 +821,7 @@ static int sbp2_update(struct unit_direc
 	/* Accept new commands unless there was another bus reset in the
 	 * meantime. */
 	if (hpsb_node_entry_valid(scsi_id->ne)) {
-		atomic_set(&scsi_id->unfinished_reset, 0);
+		atomic_set(&scsi_id->state, SBP2LU_STATE_RUNNING);
 		scsi_unblock_requests(scsi_id->scsi_host);
 	}
 	return 0;
@@ -852,7 +852,7 @@ static struct scsi_id_instance_data *sbp
 	INIT_LIST_HEAD(&scsi_id->sbp2_command_orb_completed);
 	INIT_LIST_HEAD(&scsi_id->scsi_list);
 	spin_lock_init(&scsi_id->sbp2_command_orb_lock);
-	atomic_set(&scsi_id->unfinished_reset, 0);
+	atomic_set(&scsi_id->state, SBP2LU_STATE_RUNNING);
 	INIT_WORK(&scsi_id->protocol_work, NULL, NULL);
 
 	ud->device.driver_data = scsi_id;
@@ -936,13 +936,14 @@ static void sbp2_host_reset(struct hpsb_
 	struct scsi_id_instance_data *scsi_id;
 
 	hi = hpsb_get_hostinfo(&sbp2_highlevel, host);
-
-	if (hi) {
-		list_for_each_entry(scsi_id, &hi->scsi_ids, scsi_list) {
-			atomic_set(&scsi_id->unfinished_reset, 1);
+	if (!hi)
+		return;
+	list_for_each_entry(scsi_id, &hi->scsi_ids, scsi_list)
+		if (likely(atomic_read(&scsi_id->state) !=
+			   SBP2LU_STATE_IN_SHUTDOWN)) {
+			atomic_set(&scsi_id->state, SBP2LU_STATE_IN_RESET);
 			scsi_block_requests(scsi_id->scsi_host);
 		}
-	}
 }
 
 /*



      parent reply	other threads:[~2006-08-14 17:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-14 17:40 [PATCH 2.6.18-rc4-mm1 0/8] ieee1394: fixes and touch-ups for sbp2 Stefan Richter
2006-08-14 17:42 ` [PATCH 2.6.18-rc4-mm1 1/8] the scheduled removal of drivers/ieee1394/sbp2.c:force_inquiry_hack Stefan Richter
2006-08-14 17:45 ` [PATCH 2.6.18-rc4-mm1 2/8] ieee1394: sbp2: handle "sbp2util_node_write_no_wait failed" Stefan Richter
2006-08-14 17:47 ` [PATCH 2.6.18-rc4-mm1 3/8] ieee1394: sbp2: safer agent reset in error handlers Stefan Richter
2006-08-14 17:48 ` [PATCH 2.6.18-rc4-mm1 4/8] ieee1394: sbp2: recheck node generation in sbp2_update Stefan Richter
2006-08-14 17:49 ` [PATCH 2.6.18-rc4-mm1 5/8] ieee1394: sbp2: better handling of transport errors Stefan Richter
2006-08-14 17:50 ` [PATCH 2.6.18-rc4-mm1 6/8] ieee1394: sbp2: select SCSI in Kconfig Stefan Richter
2006-08-14 17:51 ` [PATCH 2.6.18-rc4-mm1 7/8] ieee1394: sbp2: update includes Stefan Richter
2006-08-14 17:53 ` Stefan Richter [this message]

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=tkrat.fee9f136ea5d8e54@s5r6.in-berlin.de \
    --to=stefanr@s5r6.in-berlin.de \
    --cc=akpm@osdl.org \
    --cc=bcollins@ubuntu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    /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

Powered by JetHome