LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: linux1394-devel@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH 9/9] firewire: fw-sbp2: fix I/O errors during reconnect
Date: Sun, 3 Feb 2008 23:13:13 +0100 (CET)	[thread overview]
Message-ID: <tkrat.8110272bacc3f714@s5r6.in-berlin.de> (raw)
In-Reply-To: <tkrat.f34e34fd91a0695f@s5r6.in-berlin.de>

While fw-sbp2 takes the necessary time to reconnect to a logical unit
after bus reset, the SCSI core keeps sending new commands.  They are all
immediately completed with host busy status, and application clients or
filesystems will break quickly.  The SCSI device might even be taken
offline:  http://bugzilla.kernel.org/show_bug.cgi?id=9734

The only remedy seems to be to block the SCSI device until reconnect.
Alas the SCSI core has no useful API to block only one logical unit i.e.
the scsi_device, therefore we block the entire Scsi_Host.  This
currently corresponds to an SBP-2 target.  In case of targets with
multiple logical units, we need to satisfy the dependencies between
logical units by carefully tracking the blocking state of the target and
its units.  We block all logical units of a target as soon as one of
them needs to be blocked, and keep them blocked until all of them are
ready to be unblocked.

Furthermore, as the history of the old sbp2 driver has shown, the
scsi_block_requests() API is a minefield with high potential of
deadlocks.  We therefore take extra measures to keep logical units
unblocked during __scsi_add_device() and during shutdown.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-sbp2.c |   71 +++++++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 2 deletions(-)

Index: linux/drivers/firewire/fw-sbp2.c
===================================================================
--- linux.orig/drivers/firewire/fw-sbp2.c
+++ linux/drivers/firewire/fw-sbp2.c
@@ -41,6 +41,7 @@
 #include <linux/stringify.h>
 #include <linux/timer.h>
 #include <linux/workqueue.h>
+#include <asm/atomic.h>
 #include <asm/system.h>
 
 #include <scsi/scsi.h>
@@ -139,6 +140,7 @@ struct sbp2_logical_unit {
 	int generation;
 	int retries;
 	struct delayed_work work;
+	atomic_t blocked;
 };
 
 /*
@@ -157,6 +159,9 @@ struct sbp2_target {
 	int address_high;
 	unsigned int workarounds;
 	unsigned int mgt_orb_timeout;
+
+	atomic_t dont_block;
+	atomic_t blocked;
 };
 
 /*
@@ -646,6 +651,53 @@ static void sbp2_agent_reset_no_wait(str
 			&z, sizeof(z), complete_agent_reset_write_no_wait, t);
 }
 
+/*
+ * Blocks lu->tgt if all of the following conditions are met:
+ *   - Login, INQUIRY, and high-level SCSI setup of all logical units of the
+ *     target have been successfully finished (indicated by dont_block == 0).
+ *   - The lu->generation is stale.  sbp2_reconnect will unblock lu later.
+ */
+static void sbp2_conditionally_block(struct sbp2_logical_unit *lu)
+{
+	struct fw_card *card = fw_device(lu->tgt->unit->device.parent)->card;
+
+	if (!atomic_read(&lu->tgt->dont_block) &&
+	    lu->generation != card->generation &&
+	    atomic_cmpxchg(&lu->blocked, 0, 1) == 0) {
+
+		/* raise the block count of the target */
+		if (atomic_inc_return(&lu->tgt->blocked) == 1) {
+			scsi_block_requests(lu->sdev->host);
+			fw_notify("blocked %s\n", lu->tgt->bus_id);
+		}
+	}
+}
+
+/* Unblocks lu->tgt as soon as all its logical units can be unblocked. */
+static void sbp2_conditionally_unblock(struct sbp2_logical_unit *lu)
+{
+	if (atomic_cmpxchg(&lu->blocked, 1, 0) == 1) {
+
+		/* lower the block count of the target */
+		if (atomic_dec_and_test(&lu->tgt->blocked)) {
+			scsi_unblock_requests(lu->sdev->host);
+			fw_notify("unblocked %s\n", lu->tgt->bus_id);
+		}
+	}
+}
+
+
+/* Prevents future blocking of tgt and then unblocks it. */
+static void sbp2_unblock(struct sbp2_target *tgt)
+{
+	struct Scsi_Host *shost =
+		container_of((void *)tgt, struct Scsi_Host, hostdata[0]);
+
+	atomic_inc(&tgt->dont_block);
+	smp_wmb();
+	scsi_unblock_requests(shost);
+}
+
 static void sbp2_release_target(struct kref *kref)
 {
 	struct sbp2_target *tgt = container_of(kref, struct sbp2_target, kref);
@@ -653,6 +705,12 @@ static void sbp2_release_target(struct k
 	struct Scsi_Host *shost =
 		container_of((void *)tgt, struct Scsi_Host, hostdata[0]);
 
+	/*
+	 * Make sure that the target is unblocked and won't be blocked anymore
+	 * before scsi_remove_device() is called.  Else it will deadlock.
+	 */
+	sbp2_unblock(tgt);
+
 	list_for_each_entry_safe(lu, next, &tgt->lu_list, link) {
 		if (lu->sdev)
 			scsi_remove_device(lu->sdev);
@@ -717,11 +775,14 @@ static void sbp2_login(struct work_struc
 
 	if (sbp2_send_management_orb(lu, node_id, generation,
 				SBP2_LOGIN_REQUEST, lu->lun, &response) < 0) {
-		if (lu->retries++ < 5)
+		if (lu->retries++ < 5) {
 			sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5));
-		else
+		} else {
 			fw_error("%s: failed to login to LUN %04x\n",
 				 tgt->bus_id, lu->lun);
+			/* Let any waiting I/O fail from now on. */
+			sbp2_unblock(lu->tgt);
+		}
 		goto out;
 	}
 
@@ -749,6 +810,7 @@ static void sbp2_login(struct work_struc
 	/* This was a re-login. */
 	if (lu->sdev) {
 		sbp2_cancel_orbs(lu);
+		sbp2_conditionally_unblock(lu);
 		goto out;
 	}
 
@@ -786,6 +848,8 @@ static void sbp2_login(struct work_struc
 		 * Can you believe it?  Everything went well.
 		 */
 		lu->sdev = sdev;
+		smp_wmb();  /* We need lu->sdev when we want to block lu. */
+		atomic_dec(&lu->tgt->dont_block);
 		scsi_device_put(sdev);
 		goto out;
 	}
@@ -828,6 +892,7 @@ static int sbp2_add_logical_unit(struct 
 	lu->sdev = NULL;
 	lu->lun  = lun_entry & 0xffff;
 	lu->retries = 0;
+	atomic_inc(&tgt->dont_block);
 	INIT_LIST_HEAD(&lu->orb_list);
 	INIT_DELAYED_WORK(&lu->work, sbp2_login);
 
@@ -1053,6 +1118,7 @@ static void sbp2_reconnect(struct work_s
 
 	sbp2_agent_reset(lu);
 	sbp2_cancel_orbs(lu);
+	sbp2_conditionally_unblock(lu);
  out:
 	sbp2_target_put(tgt);
 }
@@ -1172,6 +1238,7 @@ complete_command_orb(struct sbp2_orb *ba
 		 * or when sending the write (less likely).
 		 */
 		result = DID_BUS_BUSY << 16;
+		sbp2_conditionally_block(orb->lu);
 	}
 
 	dma_unmap_single(device->card->device, orb->base.request_bus,

-- 
Stefan Richter
-=====-==--- --=- ---==
http://arcgraph.de/sr/


  parent reply	other threads:[~2008-02-03 22:13 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-03 22:00 [PATCH 0/9] firewire-sbp2: misc hotplug related patches Stefan Richter
2008-02-03 22:03 ` [PATCH 1/9] firewire: log GUID of new devices Stefan Richter
2008-02-04  8:14   ` Stefan Richter
2008-02-11 16:53   ` Jarod Wilson
2008-02-03 22:04 ` [PATCH 2/9] firewire: fw-sbp2: add INQUIRY delay workaround Stefan Richter
2008-02-11 17:01   ` Jarod Wilson
2008-02-03 22:07 ` [PATCH 3/9] ieee1394: sbp2: " Stefan Richter
2008-02-11 17:03   ` Jarod Wilson
2008-02-03 22:08 ` [PATCH 4/9] firewire: fw-sbp2: wait for completion of fetch agent reset Stefan Richter
2008-02-04  8:11   ` Stefan Richter
2008-02-03 22:09 ` [PATCH 5/9] firewire: fw-sbp2: log bus_id at management request failures Stefan Richter
2008-02-11 17:16   ` Jarod Wilson
2008-02-03 22:10 ` [PATCH 6/9] firewire: fw-sbp2: don't add scsi_device twice Stefan Richter
2008-02-11 17:19   ` Jarod Wilson
2008-02-11 19:42     ` Stefan Richter
2008-02-12  8:55       ` Stefan Richter
2008-02-03 22:11 ` [PATCH 7/9] firewire: fw-sbp2: logout and login after failed reconnect Stefan Richter
2008-02-11 17:32   ` Jarod Wilson
2008-02-03 22:12 ` [PATCH 8/9] firewire: fw-sbp2: sort includes Stefan Richter
2008-02-03 22:13 ` Stefan Richter [this message]
2008-02-11 18:09   ` [PATCH 9/9] firewire: fw-sbp2: fix I/O errors during reconnect Jarod Wilson
2008-02-11 20:21     ` Stefan Richter
2008-02-12  5:07       ` Jarod Wilson
2008-02-12  8:01         ` Stefan Richter
2008-02-16 15:37       ` Stefan Richter
2008-02-16 15:51         ` Stefan Richter
2008-02-04 15:54 ` [PATCH 0/9] firewire-sbp2: misc hotplug related patches John Stoffel
2008-02-04 17:48   ` Stefan Richter
2008-02-04 18:51     ` John Stoffel
2008-02-06  5:17 ` Jarod Wilson
2008-02-06 18:27   ` Stefan Richter
2008-02-06 21:09     ` [PATCH 11/9] firewire: fw-sbp2: enforce a retry of __scsi_add_device if bus generation changed Stefan Richter
2008-02-08 18:54       ` Jarod Wilson
2008-02-08 19:58         ` Stefan Richter
2008-02-08 21:33           ` [PATCH 11/9 update] " Stefan Richter
2008-02-10 18:36             ` Jarod Wilson
2008-02-16 15:01               ` Stefan Richter
2008-02-06 21:07 ` [PATCH 10/9] firewire: fw-sbp2: preemptively block sdev Stefan Richter

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.8110272bacc3f714@s5r6.in-berlin.de \
    --to=stefanr@s5r6.in-berlin.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    --subject='Re: [PATCH 9/9] firewire: fw-sbp2: fix I/O errors during reconnect' \
    /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

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).