linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...)
@ 2012-01-17  5:11 Dan Williams
  2012-01-17  5:11 ` [PATCH v4 01/10] libsas: convert dev->gone to flags Dan Williams
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide

A resend of v4: http://marc.info/?l=linux-scsi&m=132618126007864&w=1

These are all the patches that were modified between v3 and v4.  Previously
only an incremental diff was provided.

---

Dan Williams (10):
      libsas: convert dev->gone to flags
      libsas: prevent domain rediscovery competing with ata error handling
      libsas: fix timeout vs completion race
      libsas: don't mark expanders as gone when a child device is removed
      libsas: check for 'gone' expanders in smp_execute_task()
      libsas: fix sas_find_local_phy(), take phy references
      libsas: don't recover 'gone' devices in sas_ata_hard_reset()
      isci: ->lldd_ata_check_ready handler
      libsas: pre-clean commands that won the eh vs completion race
      libsas: feed the scsi_block_when_processing_errors() meter

 drivers/ata/libata-eh.c             |    1 
 drivers/ata/libata.h                |    1 
 drivers/scsi/aic94xx/aic94xx.h      |    2 
 drivers/scsi/aic94xx/aic94xx_dev.c  |   38 +-
 drivers/scsi/aic94xx/aic94xx_init.c |    2 
 drivers/scsi/aic94xx/aic94xx_tmf.c  |    9 
 drivers/scsi/isci/host.h            |   19 -
 drivers/scsi/isci/init.c            |   12 -
 drivers/scsi/isci/phy.c             |   18 +
 drivers/scsi/isci/phy.h             |    1 
 drivers/scsi/isci/port.c            |  217 ++++++------
 drivers/scsi/isci/port.h            |   11 -
 drivers/scsi/isci/remote_device.c   |   32 --
 drivers/scsi/isci/remote_device.h   |    7 
 drivers/scsi/isci/request.c         |  195 ----------
 drivers/scsi/isci/request.h         |    9 
 drivers/scsi/isci/task.c            |  152 ++------
 drivers/scsi/isci/task.h            |    4 
 drivers/scsi/libsas/sas_ata.c       |  652 +++++++++++++++--------------------
 drivers/scsi/libsas/sas_discover.c  |   93 ++++-
 drivers/scsi/libsas/sas_event.c     |   28 +-
 drivers/scsi/libsas/sas_expander.c  |  103 ++++--
 drivers/scsi/libsas/sas_init.c      |  176 +++++++++
 drivers/scsi/libsas/sas_internal.h  |   22 +
 drivers/scsi/libsas/sas_port.c      |   11 -
 drivers/scsi/libsas/sas_scsi_host.c |  289 +++++++---------
 drivers/scsi/mvsas/mv_sas.c         |    3 
 drivers/scsi/pm8001/pm8001_sas.c    |   19 +
 drivers/scsi/scsi_transport_sas.c   |   59 +++
 include/linux/libata.h              |    1 
 include/scsi/libsas.h               |   48 ++-
 include/scsi/sas_ata.h              |   23 +
 include/scsi/scsi_transport_sas.h   |   12 +
 33 files changed, 1124 insertions(+), 1145 deletions(-)

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

* [PATCH v4 01/10] libsas: convert dev->gone to flags
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 02/10] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide

In preparation for adding tracking of another device state "destroy".

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c       |    2 +-
 drivers/scsi/libsas/sas_expander.c  |    6 +++---
 drivers/scsi/libsas/sas_port.c      |    2 +-
 drivers/scsi/libsas/sas_scsi_host.c |    2 +-
 include/scsi/libsas.h               |    7 +++++--
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 81ce39d..2fc5a39 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -184,7 +184,7 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
 	spin_unlock(ap->lock);
 
 	/* If the device fell off, no sense in issuing commands */
-	if (dev->gone)
+	if (test_bit(SAS_DEV_GONE, &dev->state))
 		goto out;
 
 	task = sas_alloc_task(GFP_ATOMIC);
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 15d2239..f33d0c9 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1750,7 +1750,7 @@ static void sas_unregister_ex_tree(struct asd_sas_port *port, struct domain_devi
 	struct domain_device *child, *n;
 
 	list_for_each_entry_safe(child, n, &ex->children, siblings) {
-		child->gone = 1;
+		set_bit(SAS_DEV_GONE, &child->state);
 		if (child->dev_type == EDGE_DEV ||
 		    child->dev_type == FANOUT_DEV)
 			sas_unregister_ex_tree(port, child);
@@ -1771,7 +1771,7 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
 			&ex_dev->children, siblings) {
 			if (SAS_ADDR(child->sas_addr) ==
 			    SAS_ADDR(phy->attached_sas_addr)) {
-				child->gone = 1;
+				set_bit(SAS_DEV_GONE, &child->state);
 				if (child->dev_type == EDGE_DEV ||
 				    child->dev_type == FANOUT_DEV)
 					sas_unregister_ex_tree(parent->port, child);
@@ -1780,7 +1780,7 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
 				break;
 			}
 		}
-		parent->gone = 1;
+		set_bit(SAS_DEV_GONE, &parent->state);
 		sas_disable_routing(parent, phy->attached_sas_addr);
 	}
 	memset(phy->attached_sas_addr, 0, SAS_ADDR_SIZE);
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index a47c7a7..d88e55f 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -171,7 +171,7 @@ void sas_deform_port(struct asd_sas_phy *phy, int gone)
 
 	if (port->num_phys == 1) {
 		if (dev && gone)
-			dev->gone = 1;
+			set_bit(SAS_DEV_GONE, &dev->state);
 		sas_unregister_domain_devices(port);
 		sas_port_delete(port->port);
 		port->port = NULL;
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index fd60465..15533a1 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -192,7 +192,7 @@ int sas_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	int res = 0;
 
 	/* If the device fell off, no sense in issuing commands */
-	if (dev->gone) {
+	if (test_bit(SAS_DEV_GONE, &dev->state)) {
 		cmd->result = DID_BAD_TARGET << 16;
 		goto out_done;
 	}
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 42900fa..d792b13 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -173,7 +173,10 @@ struct sata_device {
 	struct ata_taskfile tf;
 };
 
-/* ---------- Domain device ---------- */
+enum {
+	SAS_DEV_GONE,
+};
+
 struct domain_device {
         enum sas_dev_type dev_type;
 
@@ -205,7 +208,7 @@ struct domain_device {
         };
 
         void *lldd_dev;
-	int gone;
+	unsigned long state;
 	struct kref kref;
 };
 


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

* [PATCH v4 02/10] libsas: prevent domain rediscovery competing with ata error handling
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
  2012-01-17  5:11 ` [PATCH v4 01/10] libsas: convert dev->gone to flags Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 03/10] libsas: fix timeout vs completion race Dan Williams
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide, Christoph Hellwig

libata error handling provides for a timeout for link recovery.  libsas
must not rescan for previously known devices in this interval otherwise
it may remove a device that is simply waiting for its link to recover.
Let libata-eh make the determination of when the link is stable and
prevent libsas (host workqueue) from taking action while this
determination is pending.

Using a mutex (ha->disco_mutex) to flush and disable revalidation while
eh is running requires any discovery action that may block on eh be
moved to its own context outside the lock.  Probing ATA devices
explicitly waits on ata-eh and the cache-flush-io issued during device
removal may also pend awaiting eh completion.  Essentially any rphy
add/remove activity needs to run outside the lock.

This adds two new cleanup states for sas_unregister_domain_devices()
'allocated-but-not-probed', and 'flagged-for-destruction'.  In the
'allocated-but-not-probed' state  dev->rphy points to a rphy that is
known to have not been through a sas_rphy_add() event.  At domain
teardown check if this device is still pending probe and cleanup
accordingly.  Similarly if a device has already been queued for removal
then sas_unregister_domain_devices has nothing to do.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c      |   55 +++++++++++++++++++++++++++++--
 drivers/scsi/libsas/sas_discover.c |   63 +++++++++++++++++++++++++++++++++---
 drivers/scsi/libsas/sas_event.c    |   26 +++++++++++++++
 drivers/scsi/libsas/sas_expander.c |    5 +--
 drivers/scsi/libsas/sas_init.c     |    2 +
 drivers/scsi/libsas/sas_internal.h |    3 ++
 drivers/scsi/libsas/sas_port.c     |    2 +
 drivers/scsi/scsi_transport_sas.c  |   18 +++++++++-
 include/scsi/libsas.h              |   12 ++++++-
 include/scsi/sas_ata.h             |    5 +++
 include/scsi/scsi_transport_sas.h  |    1 +
 11 files changed, 174 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 2fc5a39..4b6365c64 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -758,6 +758,35 @@ static int sas_discover_sata_pm(struct domain_device *dev)
 	return -ENODEV;
 }
 
+void sas_probe_sata(struct work_struct *work)
+{
+	struct domain_device *dev, *n;
+	struct sas_discovery_event *ev =
+		container_of(work, struct sas_discovery_event, work);
+	struct asd_sas_port *port = ev->port;
+
+	clear_bit(DISCE_PROBE, &port->disc.pending);
+
+	list_for_each_entry_safe(dev, n, &port->disco_list, disco_list_node) {
+		int err;
+
+		spin_lock_irq(&port->dev_list_lock);
+		list_add_tail(&dev->dev_list_node, &port->dev_list);
+		spin_unlock_irq(&port->dev_list_lock);
+
+		err = sas_rphy_add(dev->rphy);
+
+		if (err) {
+			SAS_DPRINTK("%s: for %s device %16llx returned %d\n",
+				    __func__, dev->parent ? "exp-attached" :
+							    "direct-attached",
+				    SAS_ADDR(dev->sas_addr), err);
+			sas_unregister_dev(port, dev);
+		} else
+			list_del_init(&dev->disco_list_node);
+	}
+}
+
 /**
  * sas_discover_sata -- discover an STP/SATA domain device
  * @dev: pointer to struct domain_device of interest
@@ -794,10 +823,15 @@ int sas_discover_sata(struct domain_device *dev)
 		break;
 	}
 	sas_notify_lldd_dev_gone(dev);
-	if (!res) {
-		sas_notify_lldd_dev_found(dev);
-		res = sas_rphy_add(dev->rphy);
-	}
+
+	if (res)
+		return res;
+
+	res = sas_notify_lldd_dev_found(dev);
+	if (res)
+		return res;
+
+	sas_discover_event(dev->port, DISCE_PROBE);
 
 	return res;
 }
@@ -805,6 +839,17 @@ int sas_discover_sata(struct domain_device *dev)
 void sas_ata_strategy_handler(struct Scsi_Host *shost)
 {
 	struct scsi_device *sdev;
+	struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(shost);
+
+	/* it's ok to defer revalidation events during ata eh, these
+	 * disks are in one of three states:
+	 * 1/ present for initial domain discovery, and these
+	 *    resets will cause bcn flutters
+	 * 2/ hot removed, we'll discover that after eh fails
+	 * 3/ hot added after initial discovery, lost the race, and need
+	 *    to catch the next train.
+	 */
+	sas_disable_revalidation(sas_ha);
 
 	shost_for_each_device(sdev, shost) {
 		struct domain_device *ddev = sdev_to_domain_dev(sdev);
@@ -816,6 +861,8 @@ void sas_ata_strategy_handler(struct Scsi_Host *shost)
 		ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error handler");
 		ata_scsi_port_error_handler(shost, ap);
 	}
+
+	sas_enable_revalidation(sas_ha);
 }
 
 int sas_ata_timed_out(struct scsi_cmnd *cmd, struct sas_task *task,
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 32e0117..7e8fdcb 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -148,9 +148,14 @@ static int sas_get_port_device(struct asd_sas_port *port)
 	port->disc.max_level = 0;
 
 	dev->rphy = rphy;
-	spin_lock_irq(&port->dev_list_lock);
-	list_add_tail(&dev->dev_list_node, &port->dev_list);
-	spin_unlock_irq(&port->dev_list_lock);
+
+	if (dev_is_sata(dev))
+		list_add_tail(&dev->disco_list_node, &port->disco_list);
+	else {
+		spin_lock_irq(&port->dev_list_lock);
+		list_add_tail(&dev->dev_list_node, &port->dev_list);
+		spin_unlock_irq(&port->dev_list_lock);
+	}
 
 	return 0;
 }
@@ -255,14 +260,43 @@ static void sas_unregister_common_dev(struct asd_sas_port *port, struct domain_d
 	sas_put_device(dev);
 }
 
-void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
+static void sas_destruct_devices(struct work_struct *work)
 {
-	if (dev->rphy) {
+	struct domain_device *dev, *n;
+	struct sas_discovery_event *ev =
+		container_of(work, struct sas_discovery_event, work);
+	struct asd_sas_port *port = ev->port;
+
+	clear_bit(DISCE_DESTRUCT, &port->disc.pending);
+
+	list_for_each_entry_safe(dev, n, &port->destroy_list, disco_list_node) {
+		list_del_init(&dev->disco_list_node);
+
 		sas_remove_children(&dev->rphy->dev);
 		sas_rphy_delete(dev->rphy);
 		dev->rphy = NULL;
+		sas_unregister_common_dev(port, dev);
+
+		sas_put_device(dev);
+	}
+}
+
+void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
+{
+	if (!test_bit(SAS_DEV_DESTROY, &dev->state) &&
+	    !list_empty(&dev->disco_list_node)) {
+		/* this rphy never saw sas_rphy_add */
+		list_del_init(&dev->disco_list_node);
+		sas_rphy_free(dev->rphy);
+		dev->rphy = NULL;
+		sas_unregister_common_dev(port, dev);
+	}
+
+	if (dev->rphy && !test_and_set_bit(SAS_DEV_DESTROY, &dev->state)) {
+		sas_rphy_unlink(dev->rphy);
+		list_move_tail(&dev->disco_list_node, &port->destroy_list);
+		sas_discover_event(dev->port, DISCE_DESTRUCT);
 	}
-	sas_unregister_common_dev(port, dev);
 }
 
 void sas_unregister_domain_devices(struct asd_sas_port *port)
@@ -271,6 +305,8 @@ void sas_unregister_domain_devices(struct asd_sas_port *port)
 
 	list_for_each_entry_safe_reverse(dev, n, &port->dev_list, dev_list_node)
 		sas_unregister_dev(port, dev);
+	list_for_each_entry_safe(dev, n, &port->disco_list, disco_list_node)
+		sas_unregister_dev(port, dev);
 
 	port->port->rphy = NULL;
 
@@ -335,6 +371,7 @@ static void sas_discover_domain(struct work_struct *work)
 		sas_rphy_free(dev->rphy);
 		dev->rphy = NULL;
 
+		list_del_init(&dev->disco_list_node);
 		spin_lock_irq(&port->dev_list_lock);
 		list_del_init(&dev->dev_list_node);
 		spin_unlock_irq(&port->dev_list_lock);
@@ -353,16 +390,28 @@ static void sas_revalidate_domain(struct work_struct *work)
 	struct sas_discovery_event *ev =
 		container_of(work, struct sas_discovery_event, work);
 	struct asd_sas_port *port = ev->port;
+	struct sas_ha_struct *ha = port->ha;
+
+	/* prevent revalidation from finding sata links in recovery */
+	mutex_lock(&ha->disco_mutex);
+	if (test_bit(SAS_HA_ATA_EH_ACTIVE, &ha->state)) {
+		SAS_DPRINTK("REVALIDATION DEFERRED on port %d, pid:%d\n",
+			    port->id, task_pid_nr(current));
+		goto out;
+	}
 
 	clear_bit(DISCE_REVALIDATE_DOMAIN, &port->disc.pending);
 
 	SAS_DPRINTK("REVALIDATING DOMAIN on port %d, pid:%d\n", port->id,
 		    task_pid_nr(current));
+
 	if (port->port_dev)
 		res = sas_ex_revalidate_domain(port->port_dev);
 
 	SAS_DPRINTK("done REVALIDATING DOMAIN on port %d, pid:%d, res 0x%x\n",
 		    port->id, task_pid_nr(current), res);
+ out:
+	mutex_unlock(&ha->disco_mutex);
 }
 
 /* ---------- Events ---------- */
@@ -414,6 +463,8 @@ void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *port)
 	static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
 		[DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
 		[DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
+		[DISCE_PROBE] = sas_probe_sata,
+		[DISCE_DESTRUCT] = sas_destruct_devices,
 	};
 
 	disc->pending = 0;
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index e5035aa..933d757 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -81,6 +81,32 @@ int sas_drain_work(struct sas_ha_struct *ha)
 }
 EXPORT_SYMBOL_GPL(sas_drain_work);
 
+void sas_disable_revalidation(struct sas_ha_struct *ha)
+{
+	mutex_lock(&ha->disco_mutex);
+	set_bit(SAS_HA_ATA_EH_ACTIVE, &ha->state);
+	mutex_unlock(&ha->disco_mutex);
+}
+
+void sas_enable_revalidation(struct sas_ha_struct *ha)
+{
+	int i;
+
+	mutex_lock(&ha->disco_mutex);
+	clear_bit(SAS_HA_ATA_EH_ACTIVE, &ha->state);
+	for (i = 0; i < ha->num_phys; i++) {
+		struct asd_sas_port *port = ha->sas_port[i];
+		const int ev = DISCE_REVALIDATE_DOMAIN;
+		struct sas_discovery *d = &port->disc;
+
+		if (!test_and_clear_bit(ev, &d->pending))
+			continue;
+
+		sas_queue_event(ev, &d->pending, &d->disc_work[ev].work, ha);
+	}
+	mutex_unlock(&ha->disco_mutex);
+}
+
 static void notify_ha_event(struct sas_ha_struct *sas_ha, enum ha_event event)
 {
 	BUG_ON(event >= HA_NUM_EVENTS);
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index f33d0c9..e45b259 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -704,9 +704,7 @@ static struct domain_device *sas_ex_discover_end_dev(
 
 		child->rphy = rphy;
 
-		spin_lock_irq(&parent->port->dev_list_lock);
-		list_add_tail(&child->dev_list_node, &parent->port->dev_list);
-		spin_unlock_irq(&parent->port->dev_list_lock);
+		list_add_tail(&child->disco_list_node, &parent->port->disco_list);
 
 		res = sas_discover_sata(child);
 		if (res) {
@@ -756,6 +754,7 @@ static struct domain_device *sas_ex_discover_end_dev(
 	sas_rphy_free(child->rphy);
 	child->rphy = NULL;
 
+	list_del(&child->disco_list_node);
 	spin_lock_irq(&parent->port->dev_list_lock);
 	list_del(&child->dev_list_node);
 	spin_unlock_irq(&parent->port->dev_list_lock);
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 572b943..52cd11d 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -104,6 +104,7 @@ int sas_register_ha(struct sas_ha_struct *sas_ha)
 {
 	int error = 0;
 
+	mutex_init(&sas_ha->disco_mutex);
 	spin_lock_init(&sas_ha->phy_port_lock);
 	sas_hash_addr(sas_ha->hashed_sas_addr, sas_ha->sas_addr);
 
@@ -168,6 +169,7 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
 	sas_drain_work(sas_ha);
 
 	sas_unregister_ports(sas_ha);
+	sas_drain_work(sas_ha);
 
 	if (sas_ha->lldd_max_execute_num > 1) {
 		sas_shutdown_queue(sas_ha);
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 948ea64..ebe9b81 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -56,6 +56,8 @@ enum blk_eh_timer_return sas_scsi_timed_out(struct scsi_cmnd *);
 int  sas_init_queue(struct sas_ha_struct *sas_ha);
 int  sas_init_events(struct sas_ha_struct *sas_ha);
 void sas_shutdown_queue(struct sas_ha_struct *sas_ha);
+void sas_disable_revalidation(struct sas_ha_struct *ha);
+void sas_enable_revalidation(struct sas_ha_struct *ha);
 
 void sas_deform_port(struct asd_sas_phy *phy, int gone);
 
@@ -138,6 +140,7 @@ static inline struct domain_device *sas_alloc_device(void)
 	if (dev) {
 		INIT_LIST_HEAD(&dev->siblings);
 		INIT_LIST_HEAD(&dev->dev_list_node);
+		INIT_LIST_HEAD(&dev->disco_list_node);
 		kref_init(&dev->kref);
 	}
 	return dev;
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index d88e55f..2980bde 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -277,6 +277,8 @@ static void sas_init_port(struct asd_sas_port *port,
 	memset(port, 0, sizeof(*port));
 	port->id = i;
 	INIT_LIST_HEAD(&port->dev_list);
+	INIT_LIST_HEAD(&port->disco_list);
+	INIT_LIST_HEAD(&port->destroy_list);
 	spin_lock_init(&port->phy_list_lock);
 	INIT_LIST_HEAD(&port->phy_list);
 	port->ha = sas_ha;
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index 9d9330a..9421bae 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -1603,6 +1603,20 @@ sas_rphy_delete(struct sas_rphy *rphy)
 EXPORT_SYMBOL(sas_rphy_delete);
 
 /**
+ * sas_rphy_unlink  -  unlink SAS remote PHY
+ * @rphy:	SAS remote phy to unlink from its parent port
+ *
+ * Removes port reference to an rphy
+ */
+void sas_rphy_unlink(struct sas_rphy *rphy)
+{
+	struct sas_port *parent = dev_to_sas_port(rphy->dev.parent);
+
+	parent->rphy = NULL;
+}
+EXPORT_SYMBOL(sas_rphy_unlink);
+
+/**
  * sas_rphy_remove  -  remove SAS remote PHY
  * @rphy:	SAS remote phy to remove
  *
@@ -1612,7 +1626,6 @@ void
 sas_rphy_remove(struct sas_rphy *rphy)
 {
 	struct device *dev = &rphy->dev;
-	struct sas_port *parent = dev_to_sas_port(dev->parent);
 
 	switch (rphy->identify.device_type) {
 	case SAS_END_DEVICE:
@@ -1626,10 +1639,9 @@ sas_rphy_remove(struct sas_rphy *rphy)
 		break;
 	}
 
+	sas_rphy_unlink(rphy);
 	transport_remove_device(dev);
 	device_del(dev);
-
-	parent->rphy = NULL;
 }
 EXPORT_SYMBOL(sas_rphy_remove);
 
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index d792b13..bd6e89e 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -86,7 +86,9 @@ enum discover_event {
 	DISCE_DISCOVER_DOMAIN   = 0U,
 	DISCE_REVALIDATE_DOMAIN = 1,
 	DISCE_PORT_GONE         = 2,
-	DISC_NUM_EVENTS 	= 3,
+	DISCE_PROBE		= 3,
+	DISCE_DESTRUCT		= 4,
+	DISC_NUM_EVENTS		= 5,
 };
 
 /* ---------- Expander Devices ---------- */
@@ -175,6 +177,7 @@ struct sata_device {
 
 enum {
 	SAS_DEV_GONE,
+	SAS_DEV_DESTROY,
 };
 
 struct domain_device {
@@ -191,6 +194,7 @@ struct domain_device {
         struct asd_sas_port *port;        /* shortcut to root of the tree */
 
         struct list_head dev_list_node;
+	struct list_head disco_list_node; /* awaiting probe or destruct */
 
         enum sas_protocol    iproto;
         enum sas_protocol    tproto;
@@ -226,7 +230,6 @@ struct sas_discovery {
 	int    max_level;
 };
 
-
 /* The port struct is Class:RW, driver:RO */
 struct asd_sas_port {
 /* private: */
@@ -236,6 +239,8 @@ struct asd_sas_port {
 	struct domain_device *port_dev;
 	spinlock_t dev_list_lock;
 	struct list_head dev_list;
+	struct list_head disco_list;
+	struct list_head destroy_list;
 	enum   sas_linkrate linkrate;
 
 	struct sas_phy *phy;
@@ -334,6 +339,7 @@ struct sas_ha_event {
 enum sas_ha_state {
 	SAS_HA_REGISTERED,
 	SAS_HA_DRAINING,
+	SAS_HA_ATA_EH_ACTIVE,
 };
 
 struct sas_ha_struct {
@@ -346,6 +352,8 @@ struct sas_ha_struct {
 	unsigned long	  state;
 	spinlock_t 	  state_lock;
 
+	struct mutex disco_mutex;
+
 	struct scsi_core core;
 
 /* public: */
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index 7d5013f..557fc9a 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -45,6 +45,7 @@ int sas_ata_timed_out(struct scsi_cmnd *cmd, struct sas_task *task,
 		      enum blk_eh_timer_return *rtn);
 int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 	       struct list_head *done_q);
+void sas_probe_sata(struct work_struct *work);
 
 #else
 
@@ -78,6 +79,10 @@ static inline int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 	return 0;
 }
 
+static inline void sas_probe_sata(struct work_struct *work)
+{
+}
+
 #endif
 
 #endif /* _SAS_ATA_H_ */
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index ffeebc3..6d14daa 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -194,6 +194,7 @@ void sas_rphy_free(struct sas_rphy *);
 extern int sas_rphy_add(struct sas_rphy *);
 extern void sas_rphy_remove(struct sas_rphy *);
 extern void sas_rphy_delete(struct sas_rphy *);
+extern void sas_rphy_unlink(struct sas_rphy *);
 extern int scsi_is_sas_rphy(const struct device *);
 
 struct sas_port *sas_port_alloc(struct device *, int);


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

* [PATCH v4 03/10] libsas: fix timeout vs completion race
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
  2012-01-17  5:11 ` [PATCH v4 01/10] libsas: convert dev->gone to flags Dan Williams
  2012-01-17  5:11 ` [PATCH v4 02/10] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 04/10] libsas: don't mark expanders as gone when a child device is removed Dan Williams
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: Tejun Heo, linux-ide, Christoph Hellwig, Darrick J. Wong

Until we have told the lldd to forget a task a timed out operation can
return from the hardware at any time.  Since completion frees the task
we need to make sure that no tasks run their normal completion handler
once eh has decided to manage the task.  Similar to
ata_scsi_cmd_error_handler() freeze completions to let eh judge the
outcome of the race.

Task collector mode is problematic because it presents a situation where
a task can be timed out and aborted before the lldd has even seen it.
For this case we need to guarantee that a task that an lldd has been
told to forget does not get queued after the lldd says "never seen it".
With sas_scsi_timed_out we achieve this with the ->task_queue_flush
mutex, rather than adding more time.

Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Darrick J. Wong <djwong@us.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c       |   35 ++++--------
 drivers/scsi/libsas/sas_internal.h  |    1 
 drivers/scsi/libsas/sas_scsi_host.c |  104 +++++++++++++++++------------------
 include/scsi/libsas.h               |    3 +
 include/scsi/sas_ata.h              |    8 ---
 5 files changed, 68 insertions(+), 83 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 903bb44..4c2a140 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -93,21 +93,30 @@ static enum ata_completion_errors sas_to_ata_err(struct task_status_struct *ts)
 static void sas_ata_task_done(struct sas_task *task)
 {
 	struct ata_queued_cmd *qc = task->uldd_task;
-	struct domain_device *dev;
+	struct domain_device *dev = task->dev;
 	struct task_status_struct *stat = &task->task_status;
 	struct ata_task_resp *resp = (struct ata_task_resp *)stat->buf;
-	struct sas_ha_struct *sas_ha;
+	struct sas_ha_struct *sas_ha = dev->port->ha;
 	enum ata_completion_errors ac;
 	unsigned long flags;
 	struct ata_link *link;
 	struct ata_port *ap;
 
+	spin_lock_irqsave(&dev->done_lock, flags);
+	if (test_bit(SAS_HA_FROZEN, &sas_ha->state))
+		task = NULL;
+	else if (qc && qc->scsicmd)
+		ASSIGN_SAS_TASK(qc->scsicmd, NULL);
+	spin_unlock_irqrestore(&dev->done_lock, flags);
+
+	/* check if libsas-eh got to the task before us */
+	if (unlikely(!task))
+		return;
+
 	if (!qc)
 		goto qc_already_gone;
 
 	ap = qc->ap;
-	dev = ap->private_data;
-	sas_ha = dev->port->ha;
 	link = &ap->link;
 
 	spin_lock_irqsave(ap->lock, flags);
@@ -156,8 +165,6 @@ static void sas_ata_task_done(struct sas_task *task)
 	}
 
 	qc->lldd_task = NULL;
-	if (qc->scsicmd)
-		ASSIGN_SAS_TASK(qc->scsicmd, NULL);
 	ata_qc_complete(qc);
 	spin_unlock_irqrestore(ap->lock, flags);
 
@@ -633,22 +640,6 @@ void sas_ata_strategy_handler(struct Scsi_Host *shost)
 	sas_enable_revalidation(sas_ha);
 }
 
-int sas_ata_timed_out(struct scsi_cmnd *cmd, struct sas_task *task,
-		      enum blk_eh_timer_return *rtn)
-{
-	struct domain_device *ddev = cmd_to_domain_dev(cmd);
-
-	if (!dev_is_sata(ddev) || task)
-		return 0;
-
-	/* we're a sata device with no task, so this must be a libata
-	 * eh timeout.  Ideally should hook into libata timeout
-	 * handling, but there's no point, it just wants to activate
-	 * the eh thread */
-	*rtn = BLK_EH_NOT_HANDLED;
-	return 1;
-}
-
 int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 	       struct list_head *done_q)
 {
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index ebe9b81..662ffcb 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -142,6 +142,7 @@ static inline struct domain_device *sas_alloc_device(void)
 		INIT_LIST_HEAD(&dev->dev_list_node);
 		INIT_LIST_HEAD(&dev->disco_list_node);
 		kref_init(&dev->kref);
+		spin_lock_init(&dev->done_lock);
 	}
 	return dev;
 }
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 50db8f9..0e3fdba 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -119,9 +119,19 @@ static void sas_end_task(struct scsi_cmnd *sc, struct sas_task *task)
 static void sas_scsi_task_done(struct sas_task *task)
 {
 	struct scsi_cmnd *sc = task->uldd_task;
+	struct domain_device *dev = task->dev;
+	struct sas_ha_struct *ha = dev->port->ha;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->done_lock, flags);
+	if (test_bit(SAS_HA_FROZEN, &ha->state))
+		task = NULL;
+	else
+		ASSIGN_SAS_TASK(sc, NULL);
+	spin_unlock_irqrestore(&dev->done_lock, flags);
 
-	if (unlikely(task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
-		/* Aborted tasks will be completed by the error handler */
+	if (unlikely(!task)) {
+		/* task will be completed by the error handler */
 		SAS_DPRINTK("task done but aborted\n");
 		return;
 	}
@@ -133,7 +143,6 @@ static void sas_scsi_task_done(struct sas_task *task)
 		return;
 	}
 
-	ASSIGN_SAS_TASK(sc, NULL);
 	sas_end_task(sc, task);
 	sc->scsi_done(sc);
 }
@@ -298,6 +307,7 @@ enum task_disposition {
 	TASK_IS_DONE,
 	TASK_IS_ABORTED,
 	TASK_IS_AT_LU,
+	TASK_IS_NOT_AT_HA,
 	TASK_IS_NOT_AT_LU,
 	TASK_ABORT_FAILED,
 };
@@ -314,19 +324,18 @@ static enum task_disposition sas_scsi_find_task(struct sas_task *task)
 		struct scsi_core *core = &ha->core;
 		struct sas_task *t, *n;
 
+		mutex_lock(&core->task_queue_flush);
 		spin_lock_irqsave(&core->task_queue_lock, flags);
-		list_for_each_entry_safe(t, n, &core->task_queue, list) {
+		list_for_each_entry_safe(t, n, &core->task_queue, list)
 			if (task == t) {
 				list_del_init(&t->list);
-				spin_unlock_irqrestore(&core->task_queue_lock,
-						       flags);
-				SAS_DPRINTK("%s: task 0x%p aborted from "
-					    "task_queue\n",
-					    __func__, task);
-				return TASK_IS_ABORTED;
+				break;
 			}
-		}
 		spin_unlock_irqrestore(&core->task_queue_lock, flags);
+		mutex_unlock(&core->task_queue_flush);
+
+		if (task == t)
+			return TASK_IS_NOT_AT_HA;
 	}
 
 	for (i = 0; i < 5; i++) {
@@ -499,8 +508,7 @@ try_bus_reset:
 }
 
 static int sas_eh_handle_sas_errors(struct Scsi_Host *shost,
-				    struct list_head *work_q,
-				    struct list_head *done_q)
+				    struct list_head *work_q)
 {
 	struct scsi_cmnd *cmd, *n;
 	enum task_disposition res = TASK_IS_DONE;
@@ -511,7 +519,16 @@ static int sas_eh_handle_sas_errors(struct Scsi_Host *shost,
 
 Again:
 	list_for_each_entry_safe(cmd, n, work_q, eh_entry) {
-		struct sas_task *task = TO_SAS_TASK(cmd);
+		struct domain_device *dev = cmd_to_domain_dev(cmd);
+		struct sas_task *task;
+
+		spin_lock_irqsave(&dev->done_lock, flags);
+		/* by this point the lldd has either observed
+		 * SAS_HA_FROZEN and is leaving the task alone, or has
+		 * won the race with eh and decided to complete it
+		 */
+		task = TO_SAS_TASK(cmd);
+		spin_unlock_irqrestore(&dev->done_lock, flags);
 
 		if (!task)
 			continue;
@@ -534,6 +551,14 @@ Again:
 		cmd->eh_eflags = 0;
 
 		switch (res) {
+		case TASK_IS_NOT_AT_HA:
+			SAS_DPRINTK("%s: task 0x%p is not at ha: %s\n",
+				    __func__, task,
+				    cmd->retries ? "retry" : "aborted");
+			if (cmd->retries)
+				cmd->retries--;
+			sas_eh_finish_cmd(cmd);
+			continue;
 		case TASK_IS_DONE:
 			SAS_DPRINTK("%s: task 0x%p is done\n", __func__,
 				    task);
@@ -635,7 +660,8 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
 	 * Deal with commands that still have SAS tasks (i.e. they didn't
 	 * complete via the normal sas_task completion mechanism)
 	 */
-	if (sas_eh_handle_sas_errors(shost, &eh_work_q, &ha->eh_done_q))
+	set_bit(SAS_HA_FROZEN, &ha->state);
+	if (sas_eh_handle_sas_errors(shost, &eh_work_q))
 		goto out;
 
 	/*
@@ -649,6 +675,10 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
 			scsi_eh_ready_devs(shost, &eh_work_q, &ha->eh_done_q);
 
 out:
+	clear_bit(SAS_HA_FROZEN, &ha->state);
+	if (ha->lldd_max_execute_num > 1)
+		wake_up_process(ha->core.queue_thread);
+
 	/* now link into libata eh --- if we have any ata devices */
 	sas_ata_strategy_handler(shost);
 
@@ -660,43 +690,7 @@ out:
 
 enum blk_eh_timer_return sas_scsi_timed_out(struct scsi_cmnd *cmd)
 {
-	struct sas_task *task = TO_SAS_TASK(cmd);
-	unsigned long flags;
-	enum blk_eh_timer_return rtn;
-
-	if (sas_ata_timed_out(cmd, task, &rtn))
-		return rtn;
-
-	if (!task) {
-		cmd->request->timeout /= 2;
-		SAS_DPRINTK("command 0x%p, task 0x%p, gone: %s\n",
-			    cmd, task, (cmd->request->timeout ?
-			    "BLK_EH_RESET_TIMER" : "BLK_EH_NOT_HANDLED"));
-		if (!cmd->request->timeout)
-			return BLK_EH_NOT_HANDLED;
-		return BLK_EH_RESET_TIMER;
-	}
-
-	spin_lock_irqsave(&task->task_state_lock, flags);
-	BUG_ON(task->task_state_flags & SAS_TASK_STATE_ABORTED);
-	if (task->task_state_flags & SAS_TASK_STATE_DONE) {
-		spin_unlock_irqrestore(&task->task_state_lock, flags);
-		SAS_DPRINTK("command 0x%p, task 0x%p, timed out: "
-			    "BLK_EH_HANDLED\n", cmd, task);
-		return BLK_EH_HANDLED;
-	}
-	if (!(task->task_state_flags & SAS_TASK_AT_INITIATOR)) {
-		spin_unlock_irqrestore(&task->task_state_lock, flags);
-		SAS_DPRINTK("command 0x%p, task 0x%p, not at initiator: "
-			    "BLK_EH_RESET_TIMER\n",
-			    cmd, task);
-		return BLK_EH_RESET_TIMER;
-	}
-	task->task_state_flags |= SAS_TASK_STATE_ABORTED;
-	spin_unlock_irqrestore(&task->task_state_lock, flags);
-
-	SAS_DPRINTK("command 0x%p, task 0x%p, timed out: BLK_EH_NOT_HANDLED\n",
-		    cmd, task);
+	scmd_printk(KERN_DEBUG, cmd, "command %p timed out\n", cmd);
 
 	return BLK_EH_NOT_HANDLED;
 }
@@ -861,9 +855,11 @@ static void sas_queue(struct sas_ha_struct *sas_ha)
 	int res;
 	struct sas_internal *i = to_sas_internal(core->shost->transportt);
 
+	mutex_lock(&core->task_queue_flush);
 	spin_lock_irqsave(&core->task_queue_lock, flags);
 	while (!kthread_should_stop() &&
-	       !list_empty(&core->task_queue)) {
+	       !list_empty(&core->task_queue) &&
+	       !test_bit(SAS_HA_FROZEN, &sas_ha->state)) {
 
 		can_queue = sas_ha->lldd_queue_size - core->task_queue_size;
 		if (can_queue >= 0) {
@@ -899,6 +895,7 @@ static void sas_queue(struct sas_ha_struct *sas_ha)
 		}
 	}
 	spin_unlock_irqrestore(&core->task_queue_lock, flags);
+	mutex_unlock(&core->task_queue_flush);
 }
 
 /**
@@ -925,6 +922,7 @@ int sas_init_queue(struct sas_ha_struct *sas_ha)
 	struct scsi_core *core = &sas_ha->core;
 
 	spin_lock_init(&core->task_queue_lock);
+	mutex_init(&core->task_queue_flush);
 	core->task_queue_size = 0;
 	INIT_LIST_HEAD(&core->task_queue);
 
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 071041b..aa7192f 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -178,6 +178,7 @@ enum {
 };
 
 struct domain_device {
+	spinlock_t done_lock;
         enum sas_dev_type dev_type;
 
         enum sas_linkrate linkrate;
@@ -321,6 +322,7 @@ struct asd_sas_phy {
 struct scsi_core {
 	struct Scsi_Host *shost;
 
+	struct mutex	  task_queue_flush;
 	spinlock_t        task_queue_lock;
 	struct list_head  task_queue;
 	int               task_queue_size;
@@ -337,6 +339,7 @@ enum sas_ha_state {
 	SAS_HA_REGISTERED,
 	SAS_HA_DRAINING,
 	SAS_HA_ATA_EH_ACTIVE,
+	SAS_HA_FROZEN,
 };
 
 struct sas_ha_struct {
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index 557fc9a..9f7a23d 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -41,8 +41,6 @@ int sas_ata_init_host_and_port(struct domain_device *found_dev,
 
 void sas_ata_task_abort(struct sas_task *task);
 void sas_ata_strategy_handler(struct Scsi_Host *shost);
-int sas_ata_timed_out(struct scsi_cmnd *cmd, struct sas_task *task,
-		      enum blk_eh_timer_return *rtn);
 int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 	       struct list_head *done_q);
 void sas_probe_sata(struct work_struct *work);
@@ -67,12 +65,6 @@ static inline void sas_ata_strategy_handler(struct Scsi_Host *shost)
 {
 }
 
-static inline int sas_ata_timed_out(struct scsi_cmnd *cmd,
-				    struct sas_task *task,
-				    enum blk_eh_timer_return *rtn)
-{
-	return 0;
-}
 static inline int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 			     struct list_head *done_q)
 {


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

* [PATCH v4 04/10] libsas: don't mark expanders as gone when a child device is removed
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
                   ` (2 preceding siblings ...)
  2012-01-17  5:11 ` [PATCH v4 03/10] libsas: fix timeout vs completion race Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 05/10] libsas: check for 'gone' expanders in smp_execute_task() Dan Williams
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide

Commit 56dd2c06 "[SCSI] libsas: Don't issue commands to devices that
have been hot-removed" marked the parent device of an end-device as gone
when all the phys to the end device have been deleted.

The expander device is still present until its parent is removed.  This
is a benign change until the smp_execute_task() path is taught to check
->gone.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_expander.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 32e417e..7701ab5 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1820,7 +1820,6 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
 				break;
 			}
 		}
-		set_bit(SAS_DEV_GONE, &parent->state);
 		sas_disable_routing(parent, phy->attached_sas_addr);
 	}
 	memset(phy->attached_sas_addr, 0, SAS_ADDR_SIZE);


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

* [PATCH v4 05/10] libsas: check for 'gone' expanders in smp_execute_task()
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
                   ` (3 preceding siblings ...)
  2012-01-17  5:11 ` [PATCH v4 04/10] libsas: don't mark expanders as gone when a child device is removed Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 06/10] libsas: fix sas_find_local_phy(), take phy references Dan Williams
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide

No sense in issuing or retrying commands to an expander that has been
removed.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_expander.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 7701ab5..6fb1f3a 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -74,6 +74,11 @@ static int smp_execute_task(struct domain_device *dev, void *req, int req_size,
 
 	mutex_lock(&dev->ex_dev.cmd_mutex);
 	for (retry = 0; retry < 3; retry++) {
+		if (test_bit(SAS_DEV_GONE, &dev->state)) {
+			res = -ECOMM;
+			break;
+		}
+
 		task = sas_alloc_task(GFP_KERNEL);
 		if (!task) {
 			res = -ENOMEM;


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

* [PATCH v4 06/10] libsas: fix sas_find_local_phy(), take phy references
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
                   ` (4 preceding siblings ...)
  2012-01-17  5:11 ` [PATCH v4 05/10] libsas: check for 'gone' expanders in smp_execute_task() Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 07/10] libsas: don't recover 'gone' devices in sas_ata_hard_reset() Dan Williams
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: Xiangliang Yu, linux-ide, Luben Tuikov, Jack Wang

In the direct-attached case this routine returns the phy on which this
device was first discovered.  Which is broken if we want to support
wide-targets, as this phy reference can become stale even though the
port is still active.

In the expander-attached case this routine tries to lookup the phy by
scanning the attached sas addresses of the parent expander, and BUG_ONs
if it can't find it.  However since eh and the libsas workqueue run
independently we can still be attempting device recovery via eh after
libsas has recorded the device as detached.  This is even easier to hit
now that eh is blocked while device domain rediscovery takes place, and
that libata is fed more timed out commands increasing the chances that
it will try to recover the ata device.

Arrange for dev->phy to always point to a last known good phy, it may be
stale after the port is torn down, but it will catch up for wide port
reconfigurations, and never be NULL.

Cc: Jack Wang <jack_wang@usish.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/aic94xx/aic94xx_tmf.c  |    9 ++++++--
 drivers/scsi/isci/task.c            |    9 +++++---
 drivers/scsi/libsas/sas_ata.c       |    7 +++++-
 drivers/scsi/libsas/sas_discover.c  |   24 ++++++++++++++++++++++
 drivers/scsi/libsas/sas_expander.c  |    5 ++++-
 drivers/scsi/libsas/sas_internal.h  |    1 +
 drivers/scsi/libsas/sas_port.c      |    7 +++---
 drivers/scsi/libsas/sas_scsi_host.c |   38 +++++++++++++++++------------------
 drivers/scsi/mvsas/mv_sas.c         |    3 ++-
 drivers/scsi/pm8001/pm8001_sas.c    |   19 +++++++++++-------
 drivers/scsi/scsi_transport_sas.c   |   23 +++++++++++++++++++++
 include/scsi/libsas.h               |    9 ++++++--
 include/scsi/scsi_transport_sas.h   |    6 ++++++
 13 files changed, 116 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c b/drivers/scsi/aic94xx/aic94xx_tmf.c
index 0add73b..50b914f 100644
--- a/drivers/scsi/aic94xx/aic94xx_tmf.c
+++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
@@ -181,7 +181,7 @@ static int asd_clear_nexus_I_T(struct domain_device *dev,
 int asd_I_T_nexus_reset(struct domain_device *dev)
 {
 	int res, tmp_res, i;
-	struct sas_phy *phy = sas_find_local_phy(dev);
+	struct sas_phy *phy = sas_get_local_phy(dev);
 	/* Standard mandates link reset for ATA  (type 0) and
 	 * hard reset for SSP (type 1) */
 	int reset_type = (dev->dev_type == SATA_DEV ||
@@ -201,7 +201,7 @@ int asd_I_T_nexus_reset(struct domain_device *dev)
 	for (i = 0 ; i < 3; i++) {
 		tmp_res = asd_clear_nexus_I_T(dev, NEXUS_PHASE_RESUME);
 		if (tmp_res == TC_RESUME)
-			return res;
+			goto out;
 		msleep(500);
 	}
 
@@ -211,7 +211,10 @@ int asd_I_T_nexus_reset(struct domain_device *dev)
 	dev_printk(KERN_ERR, &phy->dev,
 		   "Failed to resume nexus after reset 0x%x\n", tmp_res);
 
-	return TMF_RESP_FUNC_FAILED;
+	res = TMF_RESP_FUNC_FAILED;
+ out:
+	sas_put_local_phy(phy);
+	return res;
 }
 
 static int asd_clear_nexus_I_T_L(struct domain_device *dev, u8 *lun)
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 5901a0e..a6ab49a 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -1332,7 +1332,7 @@ isci_task_request_complete(struct isci_host *ihost,
 static int isci_reset_device(struct isci_host *ihost,
 			     struct isci_remote_device *idev)
 {
-	struct sas_phy *phy = sas_find_local_phy(idev->domain_dev);
+	struct sas_phy *phy = sas_get_local_phy(idev->domain_dev);
 	enum sci_status status;
 	unsigned long flags;
 	int rc;
@@ -1347,8 +1347,8 @@ static int isci_reset_device(struct isci_host *ihost,
 		dev_dbg(&ihost->pdev->dev,
 			 "%s: sci_remote_device_reset(%p) returned %d!\n",
 			 __func__, idev, status);
-
-		return TMF_RESP_FUNC_FAILED;
+		rc = TMF_RESP_FUNC_FAILED;
+		goto out;
 	}
 	spin_unlock_irqrestore(&ihost->scic_lock, flags);
 
@@ -1369,7 +1369,8 @@ static int isci_reset_device(struct isci_host *ihost,
 	}
 
 	dev_dbg(&ihost->pdev->dev, "%s: idev %p complete.\n", __func__, idev);
-
+ out:
+	sas_put_local_phy(phy);
 	return rc;
 }
 
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 5fdb63a..92f7e78 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -284,9 +284,10 @@ static int smp_ata_check_ready(struct ata_link *link)
 	struct ata_port *ap = link->ap;
 	struct domain_device *dev = ap->private_data;
 	struct domain_device *ex_dev = dev->parent;
-	struct sas_phy *phy = sas_find_local_phy(dev);
+	struct sas_phy *phy = sas_get_local_phy(dev);
 
 	res = sas_get_phy_attached_sas_addr(ex_dev, phy->number, addr);
+	sas_put_local_phy(phy);
 	/* break the wait early if the expander is unreachable,
 	 * otherwise keep polling
 	 */
@@ -319,10 +320,10 @@ static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 			      unsigned long deadline)
 {
 	int ret = 0, res;
+	struct sas_phy *phy;
 	struct ata_port *ap = link->ap;
 	int (*check_ready)(struct ata_link *link);
 	struct domain_device *dev = ap->private_data;
-	struct sas_phy *phy = sas_find_local_phy(dev);
 	struct sas_internal *i = dev_to_sas_internal(dev);
 
 	res = i->dft->lldd_I_T_nexus_reset(dev);
@@ -330,10 +331,12 @@ static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 	if (res != TMF_RESP_FUNC_COMPLETE)
 		SAS_DPRINTK("%s: Unable to reset ata device?\n", __func__);
 
+	phy = sas_get_local_phy(dev);
 	if (scsi_is_sas_phy_local(phy))
 		check_ready = local_ata_check_ready;
 	else
 		check_ready = smp_ata_check_ready;
+	sas_put_local_phy(phy);
 
 	ret = ata_wait_after_reset(link, deadline, check_ready);
 	if (ret && ret != -EAGAIN)
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index cc534c7..dfb0250 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -147,6 +147,7 @@ static int sas_get_port_device(struct asd_sas_port *port)
 	memset(port->disc.eeds_a, 0, SAS_ADDR_SIZE);
 	memset(port->disc.eeds_b, 0, SAS_ADDR_SIZE);
 	port->disc.max_level = 0;
+	sas_device_set_phy(dev, port->port);
 
 	dev->rphy = rphy;
 
@@ -234,6 +235,9 @@ void sas_free_device(struct kref *kref)
 	if (dev->parent)
 		sas_put_device(dev->parent);
 
+	sas_port_put_phy(dev->phy);
+	dev->phy = NULL;
+
 	/* remove the phys and ports, everything else should be gone */
 	if (dev->dev_type == EDGE_DEV || dev->dev_type == FANOUT_DEV)
 		kfree(dev->ex_dev.ex_phy);
@@ -308,6 +312,26 @@ void sas_unregister_domain_devices(struct asd_sas_port *port)
 
 }
 
+void sas_device_set_phy(struct domain_device *dev, struct sas_port *port)
+{
+	struct sas_ha_struct *ha;
+	struct sas_phy *new_phy;
+
+	if (!dev)
+		return;
+
+	ha = dev->port->ha;
+	new_phy = sas_port_get_phy(port);
+
+	/* pin and record last seen phy */
+	spin_lock_irq(&ha->phy_port_lock);
+	if (new_phy) {
+		sas_port_put_phy(dev->phy);
+		dev->phy = new_phy;
+	}
+	spin_unlock_irq(&ha->phy_port_lock);
+}
+
 /* ---------- Discovery and Revalidation ---------- */
 
 /**
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 6fb1f3a..68a80a0 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -723,6 +723,7 @@ static struct domain_device *sas_ex_discover_end_dev(
 		}
 	}
 	sas_ex_get_linkrate(parent, child, phy);
+	sas_device_set_phy(child, phy->port);
 
 #ifdef CONFIG_SCSI_SAS_ATA
 	if ((phy->attached_tproto & SAS_PROTOCOL_STP) || phy->attached_sata_dev) {
@@ -1810,7 +1811,7 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
 {
 	struct expander_device *ex_dev = &parent->ex_dev;
 	struct ex_phy *phy = &ex_dev->ex_phy[phy_id];
-	struct domain_device *child, *n;
+	struct domain_device *child, *n, *found = NULL;
 	if (last) {
 		list_for_each_entry_safe(child, n,
 			&ex_dev->children, siblings) {
@@ -1822,6 +1823,7 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
 					sas_unregister_ex_tree(parent->port, child);
 				else
 					sas_unregister_dev(parent->port, child);
+				found = child;
 				break;
 			}
 		}
@@ -1830,6 +1832,7 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
 	memset(phy->attached_sas_addr, 0, SAS_ADDR_SIZE);
 	if (phy->port) {
 		sas_port_delete_phy(phy->port, phy->phy);
+		sas_device_set_phy(found, phy->port);
 		if (phy->port->num_phys == 0)
 			sas_port_delete(phy->port);
 		phy->port = NULL;
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index a9a3bb9..c8febc7 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -87,6 +87,7 @@ int sas_smp_phy_control(struct domain_device *dev, int phy_id,
 			enum phy_func phy_func, struct sas_phy_linkrates *);
 int sas_smp_get_phy_events(struct sas_phy *phy);
 
+void sas_device_set_phy(struct domain_device *dev, struct sas_port *port);
 struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy);
 struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id);
 int sas_get_phy_attached_sas_addr(struct domain_device *dev, int phy_id,
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index 2980bde..31adcd1 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -108,9 +108,6 @@ static void sas_form_port(struct asd_sas_phy *phy)
 	port->num_phys++;
 	port->phy_mask |= (1U << phy->id);
 
-	if (!port->phy)
-		port->phy = phy->phy;
-
 	if (*(u64 *)port->attached_sas_addr == 0) {
 		port->class = phy->class;
 		memcpy(port->attached_sas_addr, phy->attached_sas_addr,
@@ -175,8 +172,10 @@ void sas_deform_port(struct asd_sas_phy *phy, int gone)
 		sas_unregister_domain_devices(port);
 		sas_port_delete(port->port);
 		port->port = NULL;
-	} else
+	} else {
 		sas_port_delete_phy(port->port, phy->phy);
+		sas_device_set_phy(dev, port->port);
+	}
 
 	if (si->dft->lldd_port_deformed)
 		si->dft->lldd_port_deformed(phy);
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 5cc44fd..94ef763 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -439,30 +439,26 @@ static int sas_recover_I_T(struct domain_device *dev)
 	return res;
 }
 
-/* Find the sas_phy that's attached to this device */
-struct sas_phy *sas_find_local_phy(struct domain_device *dev)
+/* take a reference on the last known good phy for this device */
+struct sas_phy *sas_get_local_phy(struct domain_device *dev)
 {
-	struct domain_device *pdev = dev->parent;
-	struct ex_phy *exphy = NULL;
-	int i;
+	struct sas_ha_struct *ha = dev->port->ha;
+	struct sas_phy *phy;
+	unsigned long flags;
 
-	/* Directly attached device */
-	if (!pdev)
-		return dev->port->phy;
+	/* a published domain device always has a valid phy, it may be
+	 * stale, but it is never NULL
+	 */
+	BUG_ON(!dev->phy);
 
-	/* Otherwise look in the expander */
-	for (i = 0; i < pdev->ex_dev.num_phys; i++)
-		if (!memcmp(dev->sas_addr,
-			    pdev->ex_dev.ex_phy[i].attached_sas_addr,
-			    SAS_ADDR_SIZE)) {
-			exphy = &pdev->ex_dev.ex_phy[i];
-			break;
-		}
+	spin_lock_irqsave(&ha->phy_port_lock, flags);
+	phy = dev->phy;
+	get_device(&phy->dev);
+	spin_unlock_irqrestore(&ha->phy_port_lock, flags);
 
-	BUG_ON(!exphy);
-	return exphy->phy;
+	return phy;
 }
-EXPORT_SYMBOL_GPL(sas_find_local_phy);
+EXPORT_SYMBOL_GPL(sas_get_local_phy);
 
 /* Attempt to send a LUN reset message to a device */
 int sas_eh_device_reset_handler(struct scsi_cmnd *cmd)
@@ -489,7 +485,7 @@ int sas_eh_device_reset_handler(struct scsi_cmnd *cmd)
 int sas_eh_bus_reset_handler(struct scsi_cmnd *cmd)
 {
 	struct domain_device *dev = cmd_to_domain_dev(cmd);
-	struct sas_phy *phy = sas_find_local_phy(dev);
+	struct sas_phy *phy = sas_get_local_phy(dev);
 	int res;
 
 	res = sas_phy_reset(phy, 1);
@@ -497,6 +493,8 @@ int sas_eh_bus_reset_handler(struct scsi_cmnd *cmd)
 		SAS_DPRINTK("Bus reset of %s failed 0x%x\n",
 			    kobject_name(&phy->dev.kobj),
 			    res);
+	sas_put_local_phy(phy);
+
 	if (res == TMF_RESP_FUNC_SUCC || res == TMF_RESP_FUNC_COMPLETE)
 		return SUCCESS;
 
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index cd88223..b68a653 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -1474,10 +1474,11 @@ static int mvs_debug_issue_ssp_tmf(struct domain_device *dev,
 static int mvs_debug_I_T_nexus_reset(struct domain_device *dev)
 {
 	int rc;
-	struct sas_phy *phy = sas_find_local_phy(dev);
+	struct sas_phy *phy = sas_get_local_phy(dev);
 	int reset_type = (dev->dev_type == SATA_DEV ||
 			(dev->tproto & SAS_PROTOCOL_STP)) ? 0 : 1;
 	rc = sas_phy_reset(phy, reset_type);
+	sas_put_local_phy(phy);
 	msleep(2000);
 	return rc;
 }
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 5add18c..b111018 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -873,12 +873,14 @@ int pm8001_I_T_nexus_reset(struct domain_device *dev)
 
 	pm8001_dev = dev->lldd_dev;
 	pm8001_ha = pm8001_find_ha_by_dev(dev);
-	phy = sas_find_local_phy(dev);
+	phy = sas_get_local_phy(dev);
 
 	if (dev_is_sata(dev)) {
 		DECLARE_COMPLETION_ONSTACK(completion_setstate);
-		if (scsi_is_sas_phy_local(phy))
-			return 0;
+		if (scsi_is_sas_phy_local(phy)) {
+			rc = 0;
+			goto out;
+		}
 		rc = sas_phy_reset(phy, 1);
 		msleep(2000);
 		rc = pm8001_exec_internal_task_abort(pm8001_ha, pm8001_dev ,
@@ -887,12 +889,14 @@ int pm8001_I_T_nexus_reset(struct domain_device *dev)
 		rc = PM8001_CHIP_DISP->set_dev_state_req(pm8001_ha,
 			pm8001_dev, 0x01);
 		wait_for_completion(&completion_setstate);
-	} else{
-	rc = sas_phy_reset(phy, 1);
-	msleep(2000);
+	} else {
+		rc = sas_phy_reset(phy, 1);
+		msleep(2000);
 	}
 	PM8001_EH_DBG(pm8001_ha, pm8001_printk(" for device[%x]:rc=%d\n",
 		pm8001_dev->device_id, rc));
+ out:
+	sas_put_local_phy(phy);
 	return rc;
 }
 
@@ -904,10 +908,11 @@ int pm8001_lu_reset(struct domain_device *dev, u8 *lun)
 	struct pm8001_device *pm8001_dev = dev->lldd_dev;
 	struct pm8001_hba_info *pm8001_ha = pm8001_find_ha_by_dev(dev);
 	if (dev_is_sata(dev)) {
-		struct sas_phy *phy = sas_find_local_phy(dev);
+		struct sas_phy *phy = sas_get_local_phy(dev);
 		rc = pm8001_exec_internal_task_abort(pm8001_ha, pm8001_dev ,
 			dev, 1, 0);
 		rc = sas_phy_reset(phy, 1);
+		sas_put_local_phy(phy);
 		rc = PM8001_CHIP_DISP->set_dev_state_req(pm8001_ha,
 			pm8001_dev, 0x01);
 		msleep(2000);
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index ab3bd0b..7d69a25 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -1060,6 +1060,29 @@ int scsi_is_sas_port(const struct device *dev)
 EXPORT_SYMBOL(scsi_is_sas_port);
 
 /**
+ * sas_port_get_phy - try to take a reference on a port member
+ * @port: port to check
+ */
+struct sas_phy *sas_port_get_phy(struct sas_port *port)
+{
+	struct sas_phy *phy;
+
+	mutex_lock(&port->phy_list_mutex);
+	if (list_empty(&port->phy_list))
+		phy = NULL;
+	else {
+		struct list_head *ent = port->phy_list.next;
+
+		phy = list_entry(ent, typeof(*phy), port_siblings);
+		get_device(&phy->dev);
+	}
+	mutex_unlock(&port->phy_list_mutex);
+
+	return phy;
+}
+EXPORT_SYMBOL(sas_port_get_phy);
+
+/**
  * sas_port_add_phy - add another phy to a port to form a wide port
  * @port:	port to add the phy to
  * @phy:	phy to add
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 2079b18..55bab86 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -192,6 +192,7 @@ struct domain_device {
         struct domain_device *parent;
         struct list_head siblings; /* devices on the same level */
         struct asd_sas_port *port;        /* shortcut to root of the tree */
+	struct sas_phy *phy;
 
         struct list_head dev_list_node;
 	struct list_head disco_list_node; /* awaiting probe or destruct */
@@ -243,7 +244,6 @@ struct asd_sas_port {
 	struct list_head destroy_list;
 	enum   sas_linkrate linkrate;
 
-	struct sas_phy *phy;
 	struct work_struct work;
 
 /* public: */
@@ -429,6 +429,11 @@ static inline unsigned int to_sas_gpio_od(int device, int bit)
 	return 3 * device + bit;
 }
 
+static inline void sas_put_local_phy(struct sas_phy *phy)
+{
+	put_device(&phy->dev);
+}
+
 #ifdef CONFIG_SCSI_SAS_HOST_SMP
 int try_test_sas_gpio_gp_bit(unsigned int od, u8 *data, u8 index, u8 count);
 #else
@@ -684,7 +689,7 @@ extern int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 
 extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
 				  struct ssp_response_iu *iu);
-struct sas_phy *sas_find_local_phy(struct domain_device *dev);
+struct sas_phy *sas_get_local_phy(struct domain_device *dev);
 
 int sas_request_addr(struct Scsi_Host *shost, u8 *addr);
 
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 42817fa..98b3a20 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -209,6 +209,12 @@ void sas_port_add_phy(struct sas_port *, struct sas_phy *);
 void sas_port_delete_phy(struct sas_port *, struct sas_phy *);
 void sas_port_mark_backlink(struct sas_port *);
 int scsi_is_sas_port(const struct device *);
+struct sas_phy *sas_port_get_phy(struct sas_port *port);
+static inline void sas_port_put_phy(struct sas_phy *phy)
+{
+	if (phy)
+		put_device(&phy->dev);
+}
 
 extern struct scsi_transport_template *
 sas_attach_transport(struct sas_function_template *);


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

* [PATCH v4 07/10] libsas: don't recover 'gone' devices in sas_ata_hard_reset()
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
                   ` (5 preceding siblings ...)
  2012-01-17  5:11 ` [PATCH v4 06/10] libsas: fix sas_find_local_phy(), take phy references Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 08/10] isci: ->lldd_ata_check_ready handler Dan Williams
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide

The commands that timeout when a disk is forcibly removed may trigger
libata to attempt recovery of the device.  If libsas has decided to
remove the device don't permit ata to continue to issue resets to its
last known phy.

The primary motivation for this patch is hotplug testing by writing 0 to
/sys/class/sas_phy/phyX/enable.  Without this check this test leads to
libata issuing a reset and re-enabling the device that wants to be torn
down.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 92f7e78..0cb538f 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -326,6 +326,9 @@ static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 	struct domain_device *dev = ap->private_data;
 	struct sas_internal *i = dev_to_sas_internal(dev);
 
+	if (test_bit(SAS_DEV_GONE, &dev->state))
+		return -ENODEV;
+
 	res = i->dft->lldd_I_T_nexus_reset(dev);
 
 	if (res != TMF_RESP_FUNC_COMPLETE)


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

* [PATCH v4 08/10] isci: ->lldd_ata_check_ready handler
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
                   ` (6 preceding siblings ...)
  2012-01-17  5:11 ` [PATCH v4 07/10] libsas: don't recover 'gone' devices in sas_ata_hard_reset() Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 09/10] libsas: pre-clean commands that won the eh vs completion race Dan Williams
  2012-01-17  5:11 ` [PATCH v4 10/10] libsas: feed the scsi_block_when_processing_errors() meter Dan Williams
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide, Jack Wang

Report to libata whether the link to the given domain_device is up and the
signature fis has been received.

Cc: Jack Wang <jack_wang@usish.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/isci/init.c |    3 +++
 drivers/scsi/isci/port.c |   25 +++++++++++++++++++++++++
 drivers/scsi/isci/port.h |    1 +
 3 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 59f2ae7..40293b3 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -185,6 +185,9 @@ static struct sas_domain_function_template isci_transport_ops  = {
 	.lldd_lu_reset		= isci_task_lu_reset,
 	.lldd_query_task	= isci_task_query_task,
 
+	/* ata recovery called from ata-eh */
+	.lldd_ata_check_ready	= isci_ata_check_ready,
+
 	/* Port and Adapter management */
 	.lldd_clear_nexus_port	= isci_task_clear_nexus_port,
 	.lldd_clear_nexus_ha	= isci_task_clear_nexus_ha,
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index 7e4a9ee..eca9ba0 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -1663,6 +1663,31 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
 	return ret;
 }
 
+int isci_ata_check_ready(struct domain_device *dev)
+{
+	struct isci_port *iport = dev->port->lldd_port;
+	struct isci_host *ihost = dev_to_ihost(dev);
+	struct isci_remote_device *idev;
+	unsigned long flags;
+	int rc = 0;
+
+	spin_lock_irqsave(&ihost->scic_lock, flags);
+	idev = isci_lookup_device(dev);
+	spin_unlock_irqrestore(&ihost->scic_lock, flags);
+
+	if (!idev)
+		goto out;
+
+	if (test_bit(IPORT_RESET_PENDING, &iport->state))
+		goto out;
+
+	rc = !!iport->active_phy_mask;
+ out:
+	isci_put_device(idev);
+
+	return rc;
+}
+
 void isci_port_deformed(struct asd_sas_phy *phy)
 {
 	struct isci_host *ihost = phy->ha->lldd_ha;
diff --git a/drivers/scsi/isci/port.h b/drivers/scsi/isci/port.h
index 78e1e82..b4a733c 100644
--- a/drivers/scsi/isci/port.h
+++ b/drivers/scsi/isci/port.h
@@ -293,4 +293,5 @@ void isci_port_init(
 
 int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *iport,
 				 struct isci_phy *iphy);
+int isci_ata_check_ready(struct domain_device *dev);
 #endif /* !defined(_ISCI_PORT_H_) */


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

* [PATCH v4 09/10] libsas: pre-clean commands that won the eh vs completion race
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
                   ` (7 preceding siblings ...)
  2012-01-17  5:11 ` [PATCH v4 08/10] isci: ->lldd_ata_check_ready handler Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-01-17  5:11 ` [PATCH v4 10/10] libsas: feed the scsi_block_when_processing_errors() meter Dan Williams
  9 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide

When scrolling forward through the eh list (in a clear_q scenario) it is
possible to encounter commands that won the completion vs eh race.  Rather
than sprinkle more "if (!task)" throughout the handler just make a pass
through the list and delete the race winners before handling the rest.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_scsi_host.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 94ef763..731c892 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -249,8 +249,8 @@ out_done:
 
 static void sas_eh_finish_cmd(struct scsi_cmnd *cmd)
 {
-	struct sas_task *task = TO_SAS_TASK(cmd);
 	struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(cmd->device->host);
+	struct sas_task *task = TO_SAS_TASK(cmd);
 
 	/* At this point, we only get called following an actual abort
 	 * of the task, so we should be guaranteed not to be racing with
@@ -267,9 +267,9 @@ static void sas_eh_finish_cmd(struct scsi_cmnd *cmd)
 
 static void sas_eh_defer_cmd(struct scsi_cmnd *cmd)
 {
-	struct sas_task *task = TO_SAS_TASK(cmd);
-	struct domain_device *dev = task->dev;
+	struct domain_device *dev = cmd_to_domain_dev(cmd);
 	struct sas_ha_struct *ha = dev->port->ha;
+	struct sas_task *task = TO_SAS_TASK(cmd);
 
 	if (!dev_is_sata(dev)) {
 		sas_eh_finish_cmd(cmd);
@@ -530,8 +530,9 @@ static int sas_eh_handle_sas_errors(struct Scsi_Host *shost,
 	struct sas_internal *i = to_sas_internal(shost->transportt);
 	unsigned long flags;
 	struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
+	LIST_HEAD(done);
 
-Again:
+	/* clean out any commands that won the completion vs eh race */
 	list_for_each_entry_safe(cmd, n, work_q, eh_entry) {
 		struct domain_device *dev = cmd_to_domain_dev(cmd);
 		struct sas_task *task;
@@ -545,7 +546,12 @@ Again:
 		spin_unlock_irqrestore(&dev->done_lock, flags);
 
 		if (!task)
-			continue;
+			list_move_tail(&cmd->eh_entry, &done);
+	}
+
+ Again:
+	list_for_each_entry_safe(cmd, n, work_q, eh_entry) {
+		struct sas_task *task = TO_SAS_TASK(cmd);
 
 		list_del_init(&cmd->eh_entry);
 
@@ -649,15 +655,16 @@ Again:
 			goto clear_q;
 		}
 	}
+ out:
+	list_splice_tail(&done, work_q);
 	list_splice_tail_init(&ha->eh_ata_q, work_q);
 	return list_empty(work_q);
-clear_q:
+
+ clear_q:
 	SAS_DPRINTK("--- Exit %s -- clear_q\n", __func__);
 	list_for_each_entry_safe(cmd, n, work_q, eh_entry)
 		sas_eh_finish_cmd(cmd);
-
-	list_splice_tail_init(&ha->eh_ata_q, work_q);
-	return list_empty(work_q);
+	goto out;
 }
 
 void sas_scsi_recover_host(struct Scsi_Host *shost)


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

* [PATCH v4 10/10] libsas: feed the scsi_block_when_processing_errors() meter
  2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
                   ` (8 preceding siblings ...)
  2012-01-17  5:11 ` [PATCH v4 09/10] libsas: pre-clean commands that won the eh vs completion race Dan Williams
@ 2012-01-17  5:11 ` Dan Williams
  2012-02-02  8:04   ` Dan Williams
  9 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2012-01-17  5:11 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide, Abraham Carranza

This is called per-sdev but in the sas-transport case this waits for the
entire domain to recover which is never guaranteed to be less than 120
seconds with libata taking nearly a minute per-device to recover.  Ping
the waitqueue so that the hung task timer knows we're still making
progress.

Reported-by: Abraham Carranza <abrahamx.c.carranza@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 37a9e73..a062adc 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -663,6 +663,11 @@ static void async_sas_ata_eh(void *data, async_cookie_t cookie)
 
 	ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error handler");
 	ata_scsi_port_error_handler(ha->core.shost, ap);
+
+	/* tell scsi_block_when_processing_errors() waiters that we are
+	 * still making forward progress
+	 */
+	wake_up(&ha->core.shost->host_wait);
 }
 
 void sas_ata_strategy_handler(struct Scsi_Host *shost)


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

* Re: [PATCH v4 10/10] libsas: feed the scsi_block_when_processing_errors() meter
  2012-01-17  5:11 ` [PATCH v4 10/10] libsas: feed the scsi_block_when_processing_errors() meter Dan Williams
@ 2012-02-02  8:04   ` Dan Williams
  0 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2012-02-02  8:04 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-ide, Abraham Carranza

On Mon, Jan 16, 2012 at 9:11 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> This is called per-sdev but in the sas-transport case this waits for the
> entire domain to recover which is never guaranteed to be less than 120
> seconds with libata taking nearly a minute per-device to recover.  Ping
> the waitqueue so that the hung task timer knows we're still making
> progress.

I'm now not so sure about this one.  This was implemented before the
change to asynchronously scan ata devices where serial discover could
take inordinate amounts of time.

Jack already reported a scan time reduction of 3 minutes down to 5
seconds with the async code, so I'll drop this patch.

Even if recovery was taking a long time this patch would only move the
hung task timeout backtrace to another location in the kernel that was
dependent on the completion of error handling.

--
Dan

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

end of thread, other threads:[~2012-02-02  8:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-17  5:11 [resend PATCH v4 00/10] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
2012-01-17  5:11 ` [PATCH v4 01/10] libsas: convert dev->gone to flags Dan Williams
2012-01-17  5:11 ` [PATCH v4 02/10] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
2012-01-17  5:11 ` [PATCH v4 03/10] libsas: fix timeout vs completion race Dan Williams
2012-01-17  5:11 ` [PATCH v4 04/10] libsas: don't mark expanders as gone when a child device is removed Dan Williams
2012-01-17  5:11 ` [PATCH v4 05/10] libsas: check for 'gone' expanders in smp_execute_task() Dan Williams
2012-01-17  5:11 ` [PATCH v4 06/10] libsas: fix sas_find_local_phy(), take phy references Dan Williams
2012-01-17  5:11 ` [PATCH v4 07/10] libsas: don't recover 'gone' devices in sas_ata_hard_reset() Dan Williams
2012-01-17  5:11 ` [PATCH v4 08/10] isci: ->lldd_ata_check_ready handler Dan Williams
2012-01-17  5:11 ` [PATCH v4 09/10] libsas: pre-clean commands that won the eh vs completion race Dan Williams
2012-01-17  5:11 ` [PATCH v4 10/10] libsas: feed the scsi_block_when_processing_errors() meter Dan Williams
2012-02-02  8:04   ` Dan Williams

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