[PATCH v5 0/7] libsas eh reworks: new + regression fixes

[PATCH v5 0/7] libsas eh reworks: new + regression fixes

[Dan Williams]

The following are incremental to v4: http://marc.info/?l=linux-scsi&m=132618126007864&w=2

1/ patch 1, 2, 3, 4 new fixes to clean up eh vs device remove 
2/ patch 5 regression fix for the sas_drain_work implementation
3/ patch 6 regression fix for domain_device reference counting
4/ patch 7 RFC implementation for hardening sata device discovery.

Patch 7 is still undergoing test, but looking for feedback from other
libsas users, particularly Jack, about the approach and whether it
addresses the sata discovery problems that have been discussed on the
list.

[PATCH 1/7] libsas: mark all domain devices gone if root port disappears
[PATCH 2/7] libsas: close scsi_remove_target() vs libata-eh race
[PATCH 3/7] libsas: fix mixed topology recovery
[PATCH 4/7] libsas: route local link resets through ata-eh
[PATCH 5/7] libsas: fix sas_unregister_ports vs sas_drain_work
[PATCH 6/7] libsas: kill spurious sas_put_device
[RFC PATCH 7/7] libsas: let libata recover links that fail to transmit initial sig-fis

 drivers/scsi/libsas/sas_ata.c       |   84 ++++++++++++++++++++----
 drivers/scsi/libsas/sas_discover.c  |   16 +++-
 drivers/scsi/libsas/sas_expander.c  |  125 ++++++++++++++++-------------------
 drivers/scsi/libsas/sas_host_smp.c  |   11 +++
 drivers/scsi/libsas/sas_init.c      |   53 +++++++++------
 drivers/scsi/libsas/sas_internal.h  |    7 +-
 drivers/scsi/libsas/sas_port.c      |    4 -
 drivers/scsi/libsas/sas_scsi_host.c |   16 ++--
 include/scsi/libsas.h               |    2 -
 include/scsi/sas.h                  |    4 +
 include/scsi/sas_ata.h              |   17 +++--
 11 files changed, 210 insertions(+), 129 deletions(-)

[PATCH v5 1/7] libsas: mark all domain devices gone if root port disappears

[Dan Williams]

If the top level expander is hot removed, mark all child devices as gone
before unregistration to short circuit futile recovery.

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

diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index dfb0250..3ef7741 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -299,12 +299,16 @@ void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
 	}
 }
 
-void sas_unregister_domain_devices(struct asd_sas_port *port)
+void sas_unregister_domain_devices(struct asd_sas_port *port, int gone)
 {
 	struct domain_device *dev, *n;
 
-	list_for_each_entry_safe_reverse(dev, n, &port->dev_list, dev_list_node)
+	list_for_each_entry_safe_reverse(dev, n, &port->dev_list, dev_list_node) {
+		if (gone)
+			set_bit(SAS_DEV_GONE, &dev->state);
 		sas_unregister_dev(port, dev);
+	}
+
 	list_for_each_entry_safe(dev, n, &port->disco_list, disco_list_node)
 		sas_unregister_dev(port, dev);
 
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index 31adcd1..59ee8a0 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -167,9 +167,7 @@ void sas_deform_port(struct asd_sas_phy *phy, int gone)
 		dev->pathways--;
 
 	if (port->num_phys == 1) {
-		if (dev && gone)
-			set_bit(SAS_DEV_GONE, &dev->state);
-		sas_unregister_domain_devices(port);
+		sas_unregister_domain_devices(port, gone);
 		sas_port_delete(port->port);
 		port->port = NULL;
 	} else {
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 55bab86..4a42be3 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -664,7 +664,7 @@ void sas_init_ex_attr(void);
 
 int  sas_ex_revalidate_domain(struct domain_device *);
 
-void sas_unregister_domain_devices(struct asd_sas_port *port);
+void sas_unregister_domain_devices(struct asd_sas_port *port, int gone);
 void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *);
 int  sas_discover_event(struct asd_sas_port *, enum discover_event ev);
 

[PATCH v5 2/7] libsas: close scsi_remove_target() vs libata-eh race

[Dan Williams]

ata_port lifetime in libata follows the host.  In libsas it follows the
scsi_target.  Once scsi_remove_device() has caused all commands to be
completed it allows scsi_remove_target() to immediately proceed to
freeing the ata_port causing bug reports like:

[  848.393333] BUG: spinlock bad magic on CPU#4, kworker/u:2/5107
[  848.400262] general protection fault: 0000 [#1] SMP
[  848.406244] CPU 4
[  848.408310] Modules linked in: nls_utf8 ipv6 uinput i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support ioatdma dca sg sd_mod sr_mod cdrom ahci libahci isci libsas libata scsi_transport_sas [last unloaded: scsi_wait_scan]
[  848.432060]
[  848.434137] Pid: 5107, comm: kworker/u:2 Not tainted 3.2.0-isci+ #8 Intel Corporation S2600CP/S2600CP
[  848.445310] RIP: 0010:[<ffffffff8126a68c>]  [<ffffffff8126a68c>] spin_dump+0x5e/0x8c
[  848.454787] RSP: 0018:ffff8807f868dca0  EFLAGS: 00010002
[  848.461137] RAX: 0000000000000048 RBX: ffff8807fe86a630 RCX: ffffffff817d0be0
[  848.469520] RDX: 0000000000000000 RSI: ffffffff814af1cf RDI: 0000000000000002
[  848.477959] RBP: ffff8807f868dcb0 R08: 00000000ffffffff R09: 000000006b6b6b6b
[  848.486327] R10: 000000000003fb8c R11: ffffffff81a19448 R12: 6b6b6b6b6b6b6b6b
[  848.494699] R13: ffff8808027dc520 R14: 0000000000000000 R15: 000000000000001e
[  848.503067] FS:  0000000000000000(0000) GS:ffff88083fd00000(0000) knlGS:0000000000000000
[  848.512899] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  848.519710] CR2: 00007ff77d001000 CR3: 00000007f7a5d000 CR4: 00000000000406e0
[  848.528072] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  848.536446] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  848.544831] Process kworker/u:2 (pid: 5107, threadinfo ffff8807f868c000, task ffff8807ff348000)
[  848.555327] Stack:
[  848.557959]  ffff8807fe86a630 ffff8807fe86a630 ffff8807f868dcd0 ffffffff8126a6e0
[  848.567072]  ffffffff817c142f ffff8807fe86a630 ffff8807f868dcf0 ffffffff8126a703
[  848.576190]  ffff8808027dc520 0000000000000286 ffff8807f868dd10 ffffffff814af1bb
[  848.585281] Call Trace:
[  848.588409]  [<ffffffff8126a6e0>] spin_bug+0x26/0x28
[  848.594357]  [<ffffffff8126a703>] do_raw_spin_unlock+0x21/0x88
[  848.601283]  [<ffffffff814af1bb>] _raw_spin_unlock_irqrestore+0x2c/0x65
[  848.609089]  [<ffffffffa001c103>] ata_scsi_port_error_handler+0x548/0x557 [libata]
[  848.618331]  [<ffffffff81061813>] ? async_schedule+0x17/0x17
[  848.625060]  [<ffffffffa004f30f>] async_sas_ata_eh+0x45/0x69 [libsas]
[  848.632655]  [<ffffffff810618aa>] async_run_entry_fn+0x97/0x125
[  848.639670]  [<ffffffff81057439>] process_one_work+0x207/0x38d
[  848.646577]  [<ffffffff8105738c>] ? process_one_work+0x15a/0x38d
[  848.653681]  [<ffffffff810576f7>] worker_thread+0x138/0x21c
[  848.660305]  [<ffffffff810575bf>] ? process_one_work+0x38d/0x38d
[  848.667493]  [<ffffffff8105b098>] kthread+0x9d/0xa5
[  848.673382]  [<ffffffff8106e1bd>] ? trace_hardirqs_on_caller+0x12f/0x166
[  848.681304]  [<ffffffff814b7704>] kernel_thread_helper+0x4/0x10
[  848.688324]  [<ffffffff814af534>] ? retint_restore_args+0x13/0x13
[  848.695530]  [<ffffffff8105affb>] ? __init_kthread_worker+0x5b/0x5b
[  848.702929]  [<ffffffff814b7700>] ? gs_change+0x13/0x13
[  848.709155] Code: 00 00 48 8d 88 38 04 00 00 44 8b 80 84 02 00 00 31 c0 e8 cf 1b 24 00 41 83 c8 ff 44 8b 4b 08 48 c7 c1 e0 0b 7d 81 4d 85 e4 74 10 <45> 8b 84 24 84 02 00 00 49 8d 8c 24 38 04 00 00 8b 53 04 48 89
[  848.732467] RIP  [<ffffffff8126a68c>] spin_dump+0x5e/0x8c
[  848.738905]  RSP <ffff8807f868dca0>
[  848.743743] ---[ end trace 143161646eee8caa ]---

...so arrange for the ata_port to have the same end of life as the domain
device.

Reported-by: Marcin Tomczak <marcin.tomczak@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c       |    5 +++++
 drivers/scsi/libsas/sas_discover.c  |    5 +++++
 drivers/scsi/libsas/sas_scsi_host.c |    3 ---
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index a062adc..2c00a38 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -661,8 +661,13 @@ static void async_sas_ata_eh(void *data, async_cookie_t cookie)
 	struct ata_port *ap = dev->sata_dev.ap;
 	struct sas_ha_struct *ha = dev->port->ha;
 
+	/* hold a reference over eh since we may be racing with final
+	 * remove once all commands are completed
+	 */
+	kref_get(&dev->kref);
 	ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error handler");
 	ata_scsi_port_error_handler(ha->core.shost, ap);
+	sas_put_device(dev);
 
 	/* tell scsi_block_when_processing_errors() waiters that we are
 	 * still making forward progress
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 3ef7741..9367101 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -242,6 +242,11 @@ void sas_free_device(struct kref *kref)
 	if (dev->dev_type == EDGE_DEV || dev->dev_type == FANOUT_DEV)
 		kfree(dev->ex_dev.ex_phy);
 
+	if (dev_is_sata(dev) && dev->sata_dev.ap) {
+		ata_sas_port_destroy(dev->sata_dev.ap);
+		dev->sata_dev.ap = NULL;
+	}
+
 	kfree(dev);
 }
 
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 731c892..b563ff2 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -1028,9 +1028,6 @@ void sas_target_destroy(struct scsi_target *starget)
 	if (!found_dev)
 		return;
 
-	if (dev_is_sata(found_dev))
-		ata_sas_port_destroy(found_dev->sata_dev.ap);
-
 	starget->hostdata = NULL;
 	sas_put_device(found_dev);
 }

[PATCH v5 3/7] libsas: fix mixed topology recovery

[Dan Williams]

If we have a domain with sas and sata devices there may still be sas
recovery actions to take after peeling off the commands to send to
libata.

Reported-by: Andrzej Jakowski <andrzej.jakowski@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c       |    8 ++------
 drivers/scsi/libsas/sas_scsi_host.c |   13 +++++++------
 include/scsi/sas_ata.h              |    9 ++++-----
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 2c00a38..0ee6831 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -704,10 +704,9 @@ void sas_ata_strategy_handler(struct Scsi_Host *shost)
 	sas_enable_revalidation(sas_ha);
 }
 
-int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
-	       struct list_head *done_q)
+void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
+		struct list_head *done_q)
 {
-	int rtn = 0;
 	struct scsi_cmnd *cmd, *n;
 	struct ata_port *ap;
 
@@ -724,7 +723,6 @@ int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 			if (ap && ap != ddev->sata_dev.ap)
 				continue;
 			ap = ddev->sata_dev.ap;
-			rtn = 1;
 			list_move(&cmd->eh_entry, &sata_q);
 		}
 
@@ -746,8 +744,6 @@ int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 				list_del_init(sata_q.next);
 		}
 	} while (ap);
-
-	return rtn;
 }
 
 void sas_ata_schedule_reset(struct domain_device *dev)
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index b563ff2..e58ca50 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -678,7 +678,8 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
 	shost->host_eh_scheduled = 0;
 	spin_unlock_irqrestore(shost->host_lock, flags);
 
-	SAS_DPRINTK("Enter %s\n", __func__);
+	SAS_DPRINTK("Enter %s busy: %d failed: %d\n",
+		    __func__, shost->host_busy, shost->host_failed);
 	/*
 	 * Deal with commands that still have SAS tasks (i.e. they didn't
 	 * complete via the normal sas_task completion mechanism)
@@ -693,9 +694,9 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
 	 * scsi_unjam_host does, but we skip scsi_eh_abort_cmds because any
 	 * command we see here has no sas_task and is thus unknown to the HA.
 	 */
-	if (!sas_ata_eh(shost, &eh_work_q, &ha->eh_done_q))
-		if (!scsi_eh_get_sense(&eh_work_q, &ha->eh_done_q))
-			scsi_eh_ready_devs(shost, &eh_work_q, &ha->eh_done_q);
+	sas_ata_eh(shost, &eh_work_q, &ha->eh_done_q);
+	if (!scsi_eh_get_sense(&eh_work_q, &ha->eh_done_q))
+		scsi_eh_ready_devs(shost, &eh_work_q, &ha->eh_done_q);
 
 out:
 	clear_bit(SAS_HA_FROZEN, &ha->state);
@@ -707,8 +708,8 @@ out:
 
 	scsi_eh_flush_done_q(&ha->eh_done_q);
 
-	SAS_DPRINTK("--- Exit %s\n", __func__);
-	return;
+	SAS_DPRINTK("--- Exit %s: busy: %d failed: %d\n",
+		    __func__, shost->host_busy, shost->host_failed);
 }
 
 enum blk_eh_timer_return sas_scsi_timed_out(struct scsi_cmnd *cmd)
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index da3f377..cb724fd 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -41,8 +41,8 @@ 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_eh(struct Scsi_Host *shost, struct list_head *work_q,
-	       struct list_head *done_q);
+void 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);
 void sas_ata_schedule_reset(struct domain_device *dev);
 void sas_ata_wait_eh(struct domain_device *dev);
@@ -66,10 +66,9 @@ static inline void sas_ata_strategy_handler(struct Scsi_Host *shost)
 {
 }
 
-static inline int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
-			     struct list_head *done_q)
+static inline void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
+			      struct list_head *done_q)
 {
-	return 0;
 }
 
 static inline void sas_probe_sata(struct work_struct *work)

[PATCH v5 4/7] libsas: route local link resets through ata-eh

[Dan Williams]

Similar to the conversion of the transport-class reset we want bsg
initiated resets to be managed by libata.

Reported-by: Jacek Danecki <jacek.danecki@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_host_smp.c |   11 ++++++++-
 drivers/scsi/libsas/sas_init.c     |   45 +++++++++++++++++++++---------------
 drivers/scsi/libsas/sas_internal.h |    1 +
 3 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/libsas/sas_host_smp.c b/drivers/scsi/libsas/sas_host_smp.c
index bb8f492..e921e53 100644
--- a/drivers/scsi/libsas/sas_host_smp.c
+++ b/drivers/scsi/libsas/sas_host_smp.c
@@ -187,11 +187,14 @@ static void sas_phy_control(struct sas_ha_struct *sas_ha, u8 phy_id,
 	struct sas_internal *i =
 		to_sas_internal(sas_ha->core.shost->transportt);
 	struct sas_phy_linkrates rates;
+	struct asd_sas_phy *asd_phy;
 
 	if (phy_id >= sas_ha->num_phys) {
 		resp_data[2] = SMP_RESP_NO_PHY;
 		return;
 	}
+
+	asd_phy = sas_ha->sas_phy[phy_id];
 	switch (phy_op) {
 	case PHY_FUNC_NOP:
 	case PHY_FUNC_LINK_RESET:
@@ -210,7 +213,13 @@ static void sas_phy_control(struct sas_ha_struct *sas_ha, u8 phy_id,
 	rates.minimum_linkrate = min;
 	rates.maximum_linkrate = max;
 
-	if (i->dft->lldd_control_phy(sas_ha->sas_phy[phy_id], phy_op, &rates))
+	/* filter reset requests through libata eh */
+	if (phy_op == PHY_FUNC_LINK_RESET && sas_try_ata_reset(asd_phy) == 0) {
+		resp_data[2] = SMP_RESP_FUNC_ACC;
+		return;
+	}
+
+	if (i->dft->lldd_control_phy(asd_phy, phy_op, &rates))
 		resp_data[2] = SMP_RESP_FUNC_FAILED;
 	else
 		resp_data[2] = SMP_RESP_FUNC_ACC;
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index cf1b532..dc93e118 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -196,6 +196,27 @@ static int sas_get_linkerrors(struct sas_phy *phy)
 	return sas_smp_get_phy_events(phy);
 }
 
+int sas_try_ata_reset(struct asd_sas_phy *asd_phy)
+{
+	struct domain_device *dev = NULL;
+
+	/* try to route user requested link resets through libata */
+	if (asd_phy->port)
+		dev = asd_phy->port->port_dev;
+
+	/* validate that dev has been probed */
+	if (dev)
+		dev = sas_find_dev_by_rphy(dev->rphy);
+
+	if (dev && dev_is_sata(dev)) {
+		sas_ata_schedule_reset(dev);
+		sas_ata_wait_eh(dev);
+		return 0;
+	}
+
+	return -ENODEV;
+}
+
 /**
  * transport_sas_phy_reset - reset a phy and permit libata to manage the link
  *
@@ -204,7 +225,6 @@ static int sas_get_linkerrors(struct sas_phy *phy)
  */
 static int transport_sas_phy_reset(struct sas_phy *phy, int hard_reset)
 {
-	int ret;
 	enum phy_func reset_type;
 
 	if (hard_reset)
@@ -218,21 +238,10 @@ static int transport_sas_phy_reset(struct sas_phy *phy, int hard_reset)
 		struct asd_sas_phy *asd_phy = sas_ha->sas_phy[phy->number];
 		struct sas_internal *i =
 			to_sas_internal(sas_ha->core.shost->transportt);
-		struct domain_device *dev = NULL;
-
-		if (asd_phy->port)
-			dev = asd_phy->port->port_dev;
-
-		/* validate that dev has been probed */
-		if (dev)
-			dev = sas_find_dev_by_rphy(dev->rphy);
 
-		if (dev && dev_is_sata(dev) && !hard_reset) {
-			sas_ata_schedule_reset(dev);
-			sas_ata_wait_eh(dev);
-			ret = 0;
-		} else
-			ret = i->dft->lldd_control_phy(asd_phy, reset_type, NULL);
+		if (!hard_reset && sas_try_ata_reset(asd_phy) == 0)
+			return 0;
+		return i->dft->lldd_control_phy(asd_phy, reset_type, NULL);
 	} else {
 		struct sas_rphy *rphy = dev_to_rphy(phy->dev.parent);
 		struct domain_device *ddev = sas_find_dev_by_rphy(rphy);
@@ -241,12 +250,10 @@ static int transport_sas_phy_reset(struct sas_phy *phy, int hard_reset)
 		if (ata_dev && !hard_reset) {
 			sas_ata_schedule_reset(ata_dev);
 			sas_ata_wait_eh(ata_dev);
-			ret = 0;
+			return 0;
 		} else
-			ret = sas_smp_phy_control(ddev, phy->number, reset_type, NULL);
+			return sas_smp_phy_control(ddev, phy->number, reset_type, NULL);
 	}
-
-	return ret;
 }
 
 static int sas_phy_enable(struct sas_phy *phy, int enable)
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index c8febc7..4157f6e 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -92,6 +92,7 @@ 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,
 				  u8 *attached_sas_addr);
+int sas_try_ata_reset(struct asd_sas_phy *phy);
 void sas_hae_reset(struct work_struct *work);
 
 void sas_free_device(struct kref *kref);

[PATCH v5 5/7] libsas: fix sas_unregister_ports vs sas_drain_work

[Dan Williams]

We need to hold drain_mutex across the unregistration as port down events
queue device removal as chained events, so we need to make sure no other
drainers are active.

[ 1118.673968] WARNING: at kernel/workqueue.c:996 __queue_work+0x11a/0x326()
[ 1118.681982] Hardware name: S2600CP
[ 1118.686193] Modules linked in: isci(-) libsas scsi_transport_sas nls_utf8
ipv6 uinput sg iTCO_wdt iTCO_vendor_support i2c_i801 i2c_core ioatdma dca
sd_mod sr_mod cdrom ahci libahci libata [last unloaded: scsi_transport_sas]
[ 1118.709893] Pid: 6831, comm: rmmod Not tainted 3.2.0-isci+ #1
[ 1118.716727] Call Trace:
[ 1118.719867]  [<ffffffff8103e9f5>] warn_slowpath_common+0x85/0x9d
[ 1118.727000]  [<ffffffff8103ea27>] warn_slowpath_null+0x1a/0x1c
[ 1118.733942]  [<ffffffff81056d44>] __queue_work+0x11a/0x326
[ 1118.740481]  [<ffffffff81056f99>] queue_work_on+0x1b/0x22
[ 1118.746925]  [<ffffffff81057106>] queue_work+0x37/0x3e
[ 1118.753105]  [<ffffffffa0120e05>] ? sas_discover_event+0x55/0x82 [libsas]
[ 1118.761094]  [<ffffffff813217c3>] scsi_queue_work+0x42/0x44
[ 1118.767717]  [<ffffffffa0120e19>] sas_discover_event+0x69/0x82 [libsas]
[ 1118.775509]  [<ffffffffa0120f5b>] sas_unregister_dev+0xc3/0xcc [libsas]
[ 1118.783319]  [<ffffffffa0120fae>] sas_unregister_domain_devices+0x4a/0xc8 [libsas]
[ 1118.792731]  [<ffffffffa0120071>] sas_deform_port+0x60/0x1a6 [libsas]
[ 1118.800339]  [<ffffffffa01201ea>] sas_unregister_ports+0x33/0x44 [libsas]
[ 1118.808342]  [<ffffffffa011f7e5>] sas_unregister_ha+0x41/0x6b [libsas]
[ 1118.816055]  [<ffffffffa0134055>] isci_unregister+0x22/0x4d [isci]
[ 1118.823384]  [<ffffffffa0143040>] isci_pci_remove+0x2e/0x60 [isci]

Reported-by: Jacek Danecki <jacek.danecki@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_init.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index dc93e118..1e13cf4 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -163,14 +163,18 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
 	unsigned long flags;
 
 	/* Set the state to unregistered to avoid further unchained
-	 * events to be queued
+	 * events to be queued, but since sas_unregister_ports()
+	 * generates unchained events relative to any in progress drains
+	 * we need to hold drain_mutex.
 	 */
+	mutex_lock(&sas_ha->drain_mutex);
 	spin_lock_irqsave(&sas_ha->state_lock, flags);
 	clear_bit(SAS_HA_REGISTERED, &sas_ha->state);
 	spin_unlock_irqrestore(&sas_ha->state_lock, flags);
-	sas_drain_work(sas_ha);
 
 	sas_unregister_ports(sas_ha);
+	mutex_unlock(&sas_ha->drain_mutex);
+
 	sas_drain_work(sas_ha);
 
 	if (sas_ha->lldd_max_execute_num > 1) {

[PATCH v5 6/7] libsas: kill spurious sas_put_device

[Dan Williams]

From: Maciej Trela <maciej.trela@intel.com>

Holdover from a patch rework, prior to the addition of SAS_DEV_DESTROY
we were holding a reference while the destruct was pending in case the
domain was torn down before the desctruct event ran.  That case is
covered by SAS_DEV_DESTROY, and the sas_put_device() just corrupts freed
memory, or worse frees the memory while another agent holds a reference.

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

diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 9367101..c6681b4 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -281,8 +281,6 @@ static void sas_destruct_devices(struct work_struct *work)
 		sas_rphy_delete(dev->rphy);
 		dev->rphy = NULL;
 		sas_unregister_common_dev(port, dev);
-
-		sas_put_device(dev);
 	}
 }
 

[RFC PATCH v5 7/7] libsas: let libata recover links that fail to transmit initial sig-fis

[Dan Williams]

libsas fails to discover all sata devices in the domain.  If a device fails
negotiation and does not transmit a signature fis the link needs recovery.
libata already understands how to manage slow to come up links, so treat these
conditions as ata device attach events for the purposes of creating an
ata_port.  This allows libata to manage retrying link bring up.

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/libsas/sas_ata.c      |   71 +++++++++++++++++++-
 drivers/scsi/libsas/sas_discover.c |    1 
 drivers/scsi/libsas/sas_expander.c |  125 ++++++++++++++++--------------------
 drivers/scsi/libsas/sas_internal.h |    6 +-
 include/scsi/sas.h                 |    4 +
 include/scsi/sas_ata.h             |    8 ++
 6 files changed, 136 insertions(+), 79 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 0ee6831..d14af22 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -278,26 +278,84 @@ static struct sas_internal *dev_to_sas_internal(struct domain_device *dev)
 	return to_sas_internal(dev->port->ha->core.shost->transportt);
 }
 
+static void sas_get_ata_command_set(struct domain_device *dev);
+
+int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
+{
+	if (phy->attached_tproto & SAS_PROTOCOL_STP)
+		dev->tproto = phy->attached_tproto;
+	if (phy->attached_sata_dev)
+		dev->tproto |= SATA_DEV;
+
+	if (phy->attached_dev_type == SATA_PENDING)
+		dev->dev_type = SATA_PENDING;
+	else {
+		int res;
+
+		dev->dev_type = SATA_DEV;
+		res = sas_get_report_phy_sata(dev->parent, phy->phy_id,
+					      &dev->sata_dev.rps_resp);
+		if (res) {
+			SAS_DPRINTK("report phy sata to %016llx:0x%x returned "
+				    "0x%x\n", SAS_ADDR(dev->parent->sas_addr),
+				    phy->phy_id, res);
+			return res;
+		}
+		memcpy(dev->frame_rcvd, &dev->sata_dev.rps_resp.rps.fis,
+		       sizeof(struct dev_to_host_fis));
+		/* TODO switch to ata_dev_classify() */
+		sas_get_ata_command_set(dev);
+	}
+	return 0;
+}
+
+static int sas_ata_clear_pending(struct domain_device *dev, struct ex_phy *phy)
+{
+	int res;
+
+	/* we weren't pending, so successfully end the reset sequence now */
+	if (dev->dev_type != SATA_PENDING)
+		return 1;
+
+	/* hmmm, if this succeeds do we need to repost the domain_device to the
+	 * lldd so it can pick up new parameters?
+	 */
+	res = sas_get_ata_info(dev, phy);
+	if (res)
+		return 0; /* retry */
+	else
+		return 1;
+}
+
 static int smp_ata_check_ready(struct ata_link *link)
 {
 	int res;
-	u8 addr[8];
 	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_get_local_phy(dev);
+	struct ex_phy *ex_phy = &ex_dev->ex_dev.ex_phy[phy->number];
 
-	res = sas_get_phy_attached_sas_addr(ex_dev, phy->number, addr);
+	res = sas_ex_phy_discover(ex_dev, phy->number);
 	sas_put_local_phy(phy);
+
 	/* break the wait early if the expander is unreachable,
 	 * otherwise keep polling
 	 */
 	if (res == -ECOMM)
 		return res;
-	if (res != SMP_RESP_FUNC_ACC || SAS_ADDR(addr) == 0)
+	if (res != SMP_RESP_FUNC_ACC)
 		return 0;
-	else
-		return 1;
+
+	switch (ex_phy->attached_dev_type) {
+	case SATA_PENDING:
+		return 0;
+	case SAS_END_DEV:
+		if (ex_phy->attached_sata_dev)
+			return sas_ata_clear_pending(dev, ex_phy);
+	default:
+		return -ENODEV;
+	}
 }
 
 static int local_ata_check_ready(struct ata_link *link)
@@ -562,6 +620,9 @@ static void sas_get_ata_command_set(struct domain_device *dev)
 	struct dev_to_host_fis *fis =
 		(struct dev_to_host_fis *) dev->frame_rcvd;
 
+	if (dev->dev_type == SATA_PENDING)
+		return;
+
 	if ((fis->sector_count == 1 && /* ATA */
 	     fis->lbal         == 1 &&
 	     fis->lbam         == 0 &&
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index c6681b4..dc8baef 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -48,6 +48,7 @@ void sas_init_dev(struct domain_device *dev)
         case SATA_DEV:
         case SATA_PM:
         case SATA_PM_PORT:
+	case SATA_PENDING:
                 INIT_LIST_HEAD(&dev->sata_dev.children);
                 break;
         default:
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 68a80a0..18aef30 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -166,17 +166,15 @@ static inline void *alloc_smp_resp(int size)
 	return kzalloc(size, GFP_KERNEL);
 }
 
-/* ---------- Expander configuration ---------- */
-
-static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
-			   void *disc_resp)
+static void sas_set_ex_phy(struct domain_device *dev, int phy_id, void *rsp)
 {
+	struct smp_resp *resp = rsp;
+	struct discover_resp *dr = &resp->disc;
 	struct expander_device *ex = &dev->ex_dev;
 	struct ex_phy *phy = &ex->ex_phy[phy_id];
-	struct smp_resp *resp = disc_resp;
-	struct discover_resp *dr = &resp->disc;
 	struct sas_rphy *rphy = dev->rphy;
-	int rediscover = (phy->phy != NULL);
+	int rediscover = !!phy->phy;
+	char *type;
 
 	if (!rediscover) {
 		phy->phy = sas_phy_alloc(&rphy->dev, phy_id);
@@ -197,8 +195,14 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
 		break;
 	}
 
+	/* This is detecting a failure to transmit initial dev to host
+	 * FIS as described in section J.5 of sas-2 r16
+	 */
+	if (dr->attached_dev_type == NO_DEVICE && dr->attached_sata_dev)
+		phy->attached_dev_type = SATA_PENDING;
+	else
+		phy->attached_dev_type = dr->attached_dev_type;
 	phy->phy_id = phy_id;
-	phy->attached_dev_type = dr->attached_dev_type;
 	phy->linkrate = dr->linkrate;
 	phy->attached_sata_host = dr->attached_sata_host;
 	phy->attached_sata_dev  = dr->attached_sata_dev;
@@ -213,7 +217,7 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
 	phy->last_da_index = -1;
 
 	phy->phy->identify.sas_address = SAS_ADDR(phy->attached_sas_addr);
-	phy->phy->identify.device_type = phy->attached_dev_type;
+	phy->phy->identify.device_type = dr->attached_dev_type;
 	phy->phy->identify.initiator_port_protocols = phy->attached_iproto;
 	phy->phy->identify.target_port_protocols = phy->attached_tproto;
 	phy->phy->identify.phy_identifier = phy_id;
@@ -229,14 +233,33 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
 			return;
 		}
 
-	SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx\n",
+	switch (phy->attached_dev_type) {
+	case SATA_PENDING:
+		type = "stp pending";
+		break;
+	case NO_DEVICE:
+		type = "no device";
+		break;
+	case SAS_END_DEV:
+		if (dr->attached_sata_dev)
+			type = "stp";
+		else
+			type = "ssp";
+		break;
+	case EDGE_DEV:
+	case FANOUT_DEV:
+		type = "smp";
+		break;
+	default:
+		type = "unknown";
+	}
+
+	SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx (%s)\n",
 		    SAS_ADDR(dev->sas_addr), phy->phy_id,
 		    phy->routing_attr == TABLE_ROUTING ? 'T' :
 		    phy->routing_attr == DIRECT_ROUTING ? 'D' :
 		    phy->routing_attr == SUBTRACTIVE_ROUTING ? 'S' : '?',
-		    SAS_ADDR(phy->attached_sas_addr));
-
-	return;
+		    SAS_ADDR(phy->attached_sas_addr), type);
 }
 
 /* check if we have an existing attached ata device on this expander phy */
@@ -267,50 +290,25 @@ struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
 static int sas_ex_phy_discover_helper(struct domain_device *dev, u8 *disc_req,
 				      u8 *disc_resp, int single)
 {
-	struct domain_device *ata_dev = sas_ex_to_ata(dev, single);
-	int i, res;
+	struct discover_resp *dr;
+	int res;
 
 	disc_req[9] = single;
-	for (i = 1 ; i < 3; i++) {
-		struct discover_resp *dr;
 
-		res = smp_execute_task(dev, disc_req, DISCOVER_REQ_SIZE,
-				       disc_resp, DISCOVER_RESP_SIZE);
-		if (res)
-			return res;
-		dr = &((struct smp_resp *)disc_resp)->disc;
-		if (memcmp(dev->sas_addr, dr->attached_sas_addr,
-			  SAS_ADDR_SIZE) == 0) {
-			sas_printk("Found loopback topology, just ignore it!\n");
-			return 0;
-		}
-
-		/* This is detecting a failure to transmit initial
-		 * dev to host FIS as described in section J.5 of
-		 * sas-2 r16
-		 */
-		if (!(dr->attached_dev_type == 0 &&
-		      dr->attached_sata_dev))
-			break;
-
-		/* In order to generate the dev to host FIS, we send a
-		 * link reset to the expander port.  If a device was
-		 * previously detected on this port we ask libata to
-		 * manage the reset and link recovery.
-		 */
-		if (ata_dev) {
-			sas_ata_schedule_reset(ata_dev);
-			break;
-		}
-		sas_smp_phy_control(dev, single, PHY_FUNC_LINK_RESET, NULL);
-		/* Wait for the reset to trigger the negotiation */
-		msleep(500);
+	res = smp_execute_task(dev, disc_req, DISCOVER_REQ_SIZE,
+			       disc_resp, DISCOVER_RESP_SIZE);
+	if (res)
+		return res;
+	dr = &((struct smp_resp *)disc_resp)->disc;
+	if (memcmp(dev->sas_addr, dr->attached_sas_addr, SAS_ADDR_SIZE) == 0) {
+		sas_printk("Found loopback topology, just ignore it!\n");
+		return 0;
 	}
 	sas_set_ex_phy(dev, single, disc_resp);
 	return 0;
 }
 
-static int sas_ex_phy_discover(struct domain_device *dev, int single)
+int sas_ex_phy_discover(struct domain_device *dev, int single)
 {
 	struct expander_device *ex = &dev->ex_dev;
 	int  res = 0;
@@ -615,9 +613,8 @@ int sas_smp_get_phy_events(struct sas_phy *phy)
 #define RPS_REQ_SIZE  16
 #define RPS_RESP_SIZE 60
 
-static int sas_get_report_phy_sata(struct domain_device *dev,
-					  int phy_id,
-					  struct smp_resp *rps_resp)
+int sas_get_report_phy_sata(struct domain_device *dev, int phy_id,
+			    struct smp_resp *rps_resp)
 {
 	int res;
 	u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE);
@@ -727,21 +724,9 @@ static struct domain_device *sas_ex_discover_end_dev(
 
 #ifdef CONFIG_SCSI_SAS_ATA
 	if ((phy->attached_tproto & SAS_PROTOCOL_STP) || phy->attached_sata_dev) {
-		child->dev_type = SATA_DEV;
-		if (phy->attached_tproto & SAS_PROTOCOL_STP)
-			child->tproto = phy->attached_tproto;
-		if (phy->attached_sata_dev)
-			child->tproto |= SATA_DEV;
-		res = sas_get_report_phy_sata(parent, phy_id,
-					      &child->sata_dev.rps_resp);
-		if (res) {
-			SAS_DPRINTK("report phy sata to %016llx:0x%x returned "
-				    "0x%x\n", SAS_ADDR(parent->sas_addr),
-				    phy_id, res);
+		res = sas_get_ata_info(child, phy);
+		if (res)
 			goto out_free;
-		}
-		memcpy(child->frame_rcvd, &child->sata_dev.rps_resp.rps.fis,
-		       sizeof(struct dev_to_host_fis));
 
 		rphy = sas_end_device_alloc(phy->port);
 		if (unlikely(!rphy))
@@ -956,7 +941,8 @@ static int sas_ex_discover_dev(struct domain_device *dev, int phy_id)
 
 	if (ex_phy->attached_dev_type != SAS_END_DEV &&
 	    ex_phy->attached_dev_type != FANOUT_DEV &&
-	    ex_phy->attached_dev_type != EDGE_DEV) {
+	    ex_phy->attached_dev_type != EDGE_DEV &&
+	    ex_phy->attached_dev_type != SATA_PENDING) {
 		SAS_DPRINTK("unknown device type(0x%x) attached to ex %016llx "
 			    "phy 0x%x\n", ex_phy->attached_dev_type,
 			    SAS_ADDR(dev->sas_addr),
@@ -982,6 +968,7 @@ static int sas_ex_discover_dev(struct domain_device *dev, int phy_id)
 
 	switch (ex_phy->attached_dev_type) {
 	case SAS_END_DEV:
+	case SATA_PENDING:
 		child = sas_ex_discover_end_dev(dev, phy_id);
 		break;
 	case FANOUT_DEV:
@@ -1658,8 +1645,8 @@ static int sas_get_phy_change_count(struct domain_device *dev,
 	return res;
 }
 
-int sas_get_phy_attached_sas_addr(struct domain_device *dev, int phy_id,
-				  u8 *attached_sas_addr)
+static int sas_get_phy_attached_sas_addr(struct domain_device *dev, int phy_id,
+					 u8 *attached_sas_addr)
 {
 	int res;
 	struct smp_resp *disc_resp;
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 4157f6e..a37a5bd 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -90,8 +90,9 @@ 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,
-				  u8 *attached_sas_addr);
+int sas_ex_phy_discover(struct domain_device *dev, int single);
+int sas_get_report_phy_sata(struct domain_device *dev, int phy_id,
+			    struct smp_resp *rps_resp);
 int sas_try_ata_reset(struct asd_sas_phy *phy);
 void sas_hae_reset(struct work_struct *work);
 
@@ -121,6 +122,7 @@ static inline void sas_fill_in_rphy(struct domain_device *dev,
 	case SATA_DEV:
 		/* FIXME: need sata device type */
 	case SAS_END_DEV:
+	case SATA_PENDING:
 		rphy->identify.device_type = SAS_END_DEVICE;
 		break;
 	case EDGE_DEV:
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index 3673d68..a577a83 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -89,8 +89,7 @@ enum sas_oob_mode {
 	SAS_OOB_MODE
 };
 
-/* See sas_discover.c if you plan on changing these.
- */
+/* See sas_discover.c if you plan on changing these */
 enum sas_dev_type {
 	NO_DEVICE   = 0,	  /* protocol */
 	SAS_END_DEV = 1,	  /* protocol */
@@ -100,6 +99,7 @@ enum sas_dev_type {
 	SATA_DEV    = 5,
 	SATA_PM     = 7,
 	SATA_PM_PORT= 8,
+	SATA_PENDING  = 9,
 };
 
 enum sas_protocol {
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index cb724fd..0ca2f8a 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -33,9 +33,10 @@
 static inline int dev_is_sata(struct domain_device *dev)
 {
 	return dev->dev_type == SATA_DEV || dev->dev_type == SATA_PM ||
-	       dev->dev_type == SATA_PM_PORT;
+	       dev->dev_type == SATA_PM_PORT || dev->dev_type == SATA_PENDING;
 }
 
+int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy);
 int sas_ata_init_host_and_port(struct domain_device *found_dev,
 			       struct scsi_target *starget);
 
@@ -82,6 +83,11 @@ static inline void sas_ata_schedule_reset(struct domain_device *dev)
 static inline void sas_ata_wait_eh(struct domain_device *dev)
 {
 }
+
+static inline int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
+{
+	return 0;
+}
 #endif
 
 #endif /* _SAS_ATA_H_ */

Re: [RFC PATCH v5 7/7] libsas: let libata recover links that fail to transmit initial sig-fis

[Dan Williams]

On Sat, Jan 14, 2012 at 10:10 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> libsas fails to discover all sata devices in the domain.  If a device fails
> negotiation and does not transmit a signature fis the link needs recovery.
> libata already understands how to manage slow to come up links, so treat these
> conditions as ata device attach events for the purposes of creating an
> ata_port.  This allows libata to manage retrying link bring up.
>
> 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>
> ---
[..]
>  static int smp_ata_check_ready(struct ata_link *link)
>  {
>        int res;
> -       u8 addr[8];
>        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_get_local_phy(dev);
> +       struct ex_phy *ex_phy = &ex_dev->ex_dev.ex_phy[phy->number];
>
> -       res = sas_get_phy_attached_sas_addr(ex_dev, phy->number, addr);
> +       res = sas_ex_phy_discover(ex_dev, phy->number);
>        sas_put_local_phy(phy);
> +
>        /* break the wait early if the expander is unreachable,
>         * otherwise keep polling
>         */
>        if (res == -ECOMM)
>                return res;
> -       if (res != SMP_RESP_FUNC_ACC || SAS_ADDR(addr) == 0)
> +       if (res != SMP_RESP_FUNC_ACC)
>                return 0;
> -       else
> -               return 1;
> +
> +       switch (ex_phy->attached_dev_type) {
> +       case SATA_PENDING:
> +               return 0;
> +       case SAS_END_DEV:
> +               if (ex_phy->attached_sata_dev)
> +                       return sas_ata_clear_pending(dev, ex_phy);
> +       default:
> +               return -ENODEV;
> +       }
>  }

Side comment, it strikes me as deadlock prone that we need to do
GFP_KERNEL allocations to recover a domain_device.  Since we're now
enforcing one in-flight smp command per expander, we might as well
pre-allocate the request/response buffers for managing remote phys.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v5 7/7] libsas: let libata recover links that fail to transmit initial sig-fis

[jack_wang]

Thanks your work to make life easier. Looks good to me.


--------------
jack_wang
>libsas fails to discover all sata devices in the domain.  If a device fails
>negotiation and does not transmit a signature fis the link needs recovery.
>libata already understands how to manage slow to come up links, so treat these
>conditions as ata device attach events for the purposes of creating an
>ata_port.  This allows libata to manage retrying link bring up.
>
>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/libsas/sas_ata.c      |   71 +++++++++++++++++++-
> drivers/scsi/libsas/sas_discover.c |    1 
> drivers/scsi/libsas/sas_expander.c |  125 ++++++++++++++++--------------------
> drivers/scsi/libsas/sas_internal.h |    6 +-
> include/scsi/sas.h                 |    4 +
> include/scsi/sas_ata.h             |    8 ++
> 6 files changed, 136 insertions(+), 79 deletions(-)
>
>diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
>index 0ee6831..d14af22 100644
>--- a/drivers/scsi/libsas/sas_ata.c
>+++ b/drivers/scsi/libsas/sas_ata.c
>@@ -278,26 +278,84 @@ static struct sas_internal *dev_to_sas_internal(struct domain_device *dev)
> 	return to_sas_internal(dev->port->ha->core.shost->transportt);
> }
> 
>+static void sas_get_ata_command_set(struct domain_device *dev);
>+
>+int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
>+{
>+	if (phy->attached_tproto & SAS_PROTOCOL_STP)
>+		dev->tproto = phy->attached_tproto;
>+	if (phy->attached_sata_dev)
>+		dev->tproto |= SATA_DEV;
>+
>+	if (phy->attached_dev_type == SATA_PENDING)
>+		dev->dev_type = SATA_PENDING;
>+	else {
>+		int res;
>+
>+		dev->dev_type = SATA_DEV;
>+		res = sas_get_report_phy_sata(dev->parent, phy->phy_id,
>+					      &dev->sata_dev.rps_resp);
>+		if (res) {
>+			SAS_DPRINTK("report phy sata to %016llx:0x%x returned "
>+				    "0x%x\n", SAS_ADDR(dev->parent->sas_addr),
>+				    phy->phy_id, res);
>+			return res;
>+		}
>+		memcpy(dev->frame_rcvd, &dev->sata_dev.rps_resp.rps.fis,
>+		       sizeof(struct dev_to_host_fis));
>+		/* TODO switch to ata_dev_classify() */
>+		sas_get_ata_command_set(dev);
>+	}
>+	return 0;
>+}
>+
>+static int sas_ata_clear_pending(struct domain_device *dev, struct ex_phy *phy)
>+{
>+	int res;
>+
>+	/* we weren't pending, so successfully end the reset sequence now */
>+	if (dev->dev_type != SATA_PENDING)
>+		return 1;
>+
>+	/* hmmm, if this succeeds do we need to repost the domain_device to the
>+	 * lldd so it can pick up new parameters?
>+	 */
>+	res = sas_get_ata_info(dev, phy);
>+	if (res)
>+		return 0; /* retry */
>+	else
>+		return 1;
>+}
>+
> static int smp_ata_check_ready(struct ata_link *link)
> {
> 	int res;
>-	u8 addr[8];
> 	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_get_local_phy(dev);
>+	struct ex_phy *ex_phy = &ex_dev->ex_dev.ex_phy[phy->number];
> 
>-	res = sas_get_phy_attached_sas_addr(ex_dev, phy->number, addr);
>+	res = sas_ex_phy_discover(ex_dev, phy->number);
> 	sas_put_local_phy(phy);
>+
> 	/* break the wait early if the expander is unreachable,
> 	 * otherwise keep polling
> 	 */
> 	if (res == -ECOMM)
> 		return res;
>-	if (res != SMP_RESP_FUNC_ACC || SAS_ADDR(addr) == 0)
>+	if (res != SMP_RESP_FUNC_ACC)
> 		return 0;
>-	else
>-		return 1;
>+
>+	switch (ex_phy->attached_dev_type) {
>+	case SATA_PENDING:
>+		return 0;
>+	case SAS_END_DEV:
>+		if (ex_phy->attached_sata_dev)
>+			return sas_ata_clear_pending(dev, ex_phy);
>+	default:
>+		return -ENODEV;
>+	}
> }
> 
> static int local_ata_check_ready(struct ata_link *link)
>@@ -562,6 +620,9 @@ static void sas_get_ata_command_set(struct domain_device *dev)
> 	struct dev_to_host_fis *fis =
> 		(struct dev_to_host_fis *) dev->frame_rcvd;
> 
>+	if (dev->dev_type == SATA_PENDING)
>+		return;
>+
> 	if ((fis->sector_count == 1 && /* ATA */
> 	     fis->lbal         == 1 &&
> 	     fis->lbam         == 0 &&
>diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
>index c6681b4..dc8baef 100644
>--- a/drivers/scsi/libsas/sas_discover.c
>+++ b/drivers/scsi/libsas/sas_discover.c
>@@ -48,6 +48,7 @@ void sas_init_dev(struct domain_device *dev)
>         case SATA_DEV:
>         case SATA_PM:
>         case SATA_PM_PORT:
>+	case SATA_PENDING:
>                 INIT_LIST_HEAD(&dev->sata_dev.children);
>                 break;
>         default:
>diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
>index 68a80a0..18aef30 100644
>--- a/drivers/scsi/libsas/sas_expander.c
>+++ b/drivers/scsi/libsas/sas_expander.c
>@@ -166,17 +166,15 @@ static inline void *alloc_smp_resp(int size)
> 	return kzalloc(size, GFP_KERNEL);
> }
> 
>-/* ---------- Expander configuration ---------- */
>-
>-static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
>-			   void *disc_resp)
>+static void sas_set_ex_phy(struct domain_device *dev, int phy_id, void *rsp)
> {
>+	struct smp_resp *resp = rsp;
>+	struct discover_resp *dr = &resp->disc;
> 	struct expander_device *ex = &dev->ex_dev;
> 	struct ex_phy *phy = &ex->ex_phy[phy_id];
>-	struct smp_resp *resp = disc_resp;
>-	struct discover_resp *dr = &resp->disc;
> 	struct sas_rphy *rphy = dev->rphy;
>-	int rediscover = (phy->phy != NULL);
>+	int rediscover = !!phy->phy;
>+	char *type;
> 
> 	if (!rediscover) {
> 		phy->phy = sas_phy_alloc(&rphy->dev, phy_id);
>@@ -197,8 +195,14 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
> 		break;
> 	}
> 
>+	/* This is detecting a failure to transmit initial dev to host
>+	 * FIS as described in section J.5 of sas-2 r16
>+	 */
>+	if (dr->attached_dev_type == NO_DEVICE && dr->attached_sata_dev)
>+		phy->attached_dev_type = SATA_PENDING;
>+	else
>+		phy->attached_dev_type = dr->attached_dev_type;
> 	phy->phy_id = phy_id;
>-	phy->attached_dev_type = dr->attached_dev_type;
> 	phy->linkrate = dr->linkrate;
> 	phy->attached_sata_host = dr->attached_sata_host;
> 	phy->attached_sata_dev  = dr->attached_sata_dev;
>@@ -213,7 +217,7 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
> 	phy->last_da_index = -1;
> 
> 	phy->phy->identify.sas_address = SAS_ADDR(phy->attached_sas_addr);
>-	phy->phy->identify.device_type = phy->attached_dev_type;
>+	phy->phy->identify.device_type = dr->attached_dev_type;
> 	phy->phy->identify.initiator_port_protocols = phy->attached_iproto;
> 	phy->phy->identify.target_port_protocols = phy->attached_tproto;
> 	phy->phy->identify.phy_identifier = phy_id;
>@@ -229,14 +233,33 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
> 			return;
> 		}
> 
>-	SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx\n",
>+	switch (phy->attached_dev_type) {
>+	case SATA_PENDING:
>+		type = "stp pending";
>+		break;
>+	case NO_DEVICE:
>+		type = "no device";
>+		break;
>+	case SAS_END_DEV:
>+		if (dr->attached_sata_dev)
>+			type = "stp";
>+		else
>+			type = "ssp";
>+		break;
>+	case EDGE_DEV:
>+	case FANOUT_DEV:
>+		type = "smp";
>+		break;
>+	default:
>+		type = "unknown";
>+	}
>+
>+	SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx (%s)\n",
> 		    SAS_ADDR(dev->sas_addr), phy->phy_id,
> 		    phy->routing_attr == TABLE_ROUTING ? 'T' :
> 		    phy->routing_attr == DIRECT_ROUTING ? 'D' :
> 		    phy->routing_attr == SUBTRACTIVE_ROUTING ? 'S' : '?',
>-		    SAS_ADDR(phy->attached_sas_addr));
>-
>-	return;
>+		    SAS_ADDR(phy->attached_sas_addr), type);
> }
> 
> /* check if we have an existing attached ata device on this expander phy */
>@@ -267,50 +290,25 @@ struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
> static int sas_ex_phy_discover_helper(struct domain_device *dev, u8 *disc_req,
> 				      u8 *disc_resp, int single)
> {
>-	struct domain_device *ata_dev = sas_ex_to_ata(dev, single);
>-	int i, res;
>+	struct discover_resp *dr;
>+	int res;
> 
> 	disc_req[9] = single;
>-	for (i = 1 ; i < 3; i++) {
>-		struct discover_resp *dr;
> 
>-		res = smp_execute_task(dev, disc_req, DISCOVER_REQ_SIZE,
>-				       disc_resp, DISCOVER_RESP_SIZE);
>-		if (res)
>-			return res;
>-		dr = &((struct smp_resp *)disc_resp)->disc;
>-		if (memcmp(dev->sas_addr, dr->attached_sas_addr,
>-			  SAS_ADDR_SIZE) == 0) {
>-			sas_printk("Found loopback topology, just ignore it!\n");
>-			return 0;
>-		}
>-
>-		/* This is detecting a failure to transmit initial
>-		 * dev to host FIS as described in section J.5 of
>-		 * sas-2 r16
>-		 */
>-		if (!(dr->attached_dev_type == 0 &&
>-		      dr->attached_sata_dev))
>-			break;
>-
>-		/* In order to generate the dev to host FIS, we send a
>-		 * link reset to the expander port.  If a device was
>-		 * previously detected on this port we ask libata to
>-		 * manage the reset and link recovery.
>-		 */
>-		if (ata_dev) {
>-			sas_ata_schedule_reset(ata_dev);
>-			break;
>-		}
>-		sas_smp_phy_control(dev, single, PHY_FUNC_LINK_RESET, NULL);
>-		/* Wait for the reset to trigger the negotiation */
>-		msleep(500);
>+	res = smp_execute_task(dev, disc_req, DISCOVER_REQ_SIZE,
>+			       disc_resp, DISCOVER_RESP_SIZE);
>+	if (res)
>+		return res;
>+	dr = &((struct smp_resp *)disc_resp)->disc;
>+	if (memcmp(dev->sas_addr, dr->attached_sas_addr, SAS_ADDR_SIZE) == 0) {
>+		sas_printk("Found loopback topology, just ignore it!\n");
>+		return 0;
> 	}
> 	sas_set_ex_phy(dev, single, disc_resp);
> 	return 0;
> }
> 
>-static int sas_ex_phy_discover(struct domain_device *dev, int single)
>+int sas_ex_phy_discover(struct domain_device *dev, int single)
> {
> 	struct expander_device *ex = &dev->ex_dev;
> 	int  res = 0;
>@@ -615,9 +613,8 @@ int sas_smp_get_phy_events(struct sas_phy *phy)
> #define RPS_REQ_SIZE  16
> #define RPS_RESP_SIZE 60
> 
>-static int sas_get_report_phy_sata(struct domain_device *dev,
>-					  int phy_id,
>-					  struct smp_resp *rps_resp)
>+int sas_get_report_phy_sata(struct domain_device *dev, int phy_id,
>+			    struct smp_resp *rps_resp)
> {
> 	int res;
> 	u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE);
>@@ -727,21 +724,9 @@ static struct domain_device *sas_ex_discover_end_dev(
> 
> #ifdef CONFIG_SCSI_SAS_ATA
> 	if ((phy->attached_tproto & SAS_PROTOCOL_STP) || phy->attached_sata_dev) {
>-		child->dev_type = SATA_DEV;
>-		if (phy->attached_tproto & SAS_PROTOCOL_STP)
>-			child->tproto = phy->attached_tproto;
>-		if (phy->attached_sata_dev)
>-			child->tproto |= SATA_DEV;
>-		res = sas_get_report_phy_sata(parent, phy_id,
>-					      &child->sata_dev.rps_resp);
>-		if (res) {
>-			SAS_DPRINTK("report phy sata to %016llx:0x%x returned "
>-				    "0x%x\n", SAS_ADDR(parent->sas_addr),
>-				    phy_id, res);
>+		res = sas_get_ata_info(child, phy);
>+		if (res)
> 			goto out_free;
>-		}
>-		memcpy(child->frame_rcvd, &child->sata_dev.rps_resp.rps.fis,
>-		       sizeof(struct dev_to_host_fis));
> 
> 		rphy = sas_end_device_alloc(phy->port);
> 		if (unlikely(!rphy))
>@@ -956,7 +941,8 @@ static int sas_ex_discover_dev(struct domain_device *dev, int phy_id)
> 
> 	if (ex_phy->attached_dev_type != SAS_END_DEV &&
> 	    ex_phy->attached_dev_type != FANOUT_DEV &&
>-	    ex_phy->attached_dev_type != EDGE_DEV) {
>+	    ex_phy->attached_dev_type != EDGE_DEV &&
>+	    ex_phy->attached_dev_type != SATA_PENDING) {
> 		SAS_DPRINTK("unknown device type(0x%x) attached to ex %016llx "
> 			    "phy 0x%x\n", ex_phy->attached_dev_type,
> 			    SAS_ADDR(dev->sas_addr),
>@@ -982,6 +968,7 @@ static int sas_ex_discover_dev(struct domain_device *dev, int phy_id)
> 
> 	switch (ex_phy->attached_dev_type) {
> 	case SAS_END_DEV:
>+	case SATA_PENDING:
> 		child = sas_ex_discover_end_dev(dev, phy_id);
> 		break;
> 	case FANOUT_DEV:
>@@ -1658,8 +1645,8 @@ static int sas_get_phy_change_count(struct domain_device *dev,
> 	return res;
> }
> 
>-int sas_get_phy_attached_sas_addr(struct domain_device *dev, int phy_id,
>-				  u8 *attached_sas_addr)
>+static int sas_get_phy_attached_sas_addr(struct domain_device *dev, int phy_id,
>+					 u8 *attached_sas_addr)
> {
> 	int res;
> 	struct smp_resp *disc_resp;
>diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
>index 4157f6e..a37a5bd 100644
>--- a/drivers/scsi/libsas/sas_internal.h
>+++ b/drivers/scsi/libsas/sas_internal.h
>@@ -90,8 +90,9 @@ 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,
>-				  u8 *attached_sas_addr);
>+int sas_ex_phy_discover(struct domain_device *dev, int single);
>+int sas_get_report_phy_sata(struct domain_device *dev, int phy_id,
>+			    struct smp_resp *rps_resp);
> int sas_try_ata_reset(struct asd_sas_phy *phy);
> void sas_hae_reset(struct work_struct *work);
> 
>@@ -121,6 +122,7 @@ static inline void sas_fill_in_rphy(struct domain_device *dev,
> 	case SATA_DEV:
> 		/* FIXME: need sata device type */
> 	case SAS_END_DEV:
>+	case SATA_PENDING:
> 		rphy->identify.device_type = SAS_END_DEVICE;
> 		break;
> 	case EDGE_DEV:
>diff --git a/include/scsi/sas.h b/include/scsi/sas.h
>index 3673d68..a577a83 100644
>--- a/include/scsi/sas.h
>+++ b/include/scsi/sas.h
>@@ -89,8 +89,7 @@ enum sas_oob_mode {
> 	SAS_OOB_MODE
> };
> 
>-/* See sas_discover.c if you plan on changing these.
>- */
>+/* See sas_discover.c if you plan on changing these */
> enum sas_dev_type {
> 	NO_DEVICE   = 0,	  /* protocol */
> 	SAS_END_DEV = 1,	  /* protocol */
>@@ -100,6 +99,7 @@ enum sas_dev_type {
> 	SATA_DEV    = 5,
> 	SATA_PM     = 7,
> 	SATA_PM_PORT= 8,
>+	SATA_PENDING  = 9,
> };
> 
> enum sas_protocol {
>diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
>index cb724fd..0ca2f8a 100644
>--- a/include/scsi/sas_ata.h
>+++ b/include/scsi/sas_ata.h
>@@ -33,9 +33,10 @@
> static inline int dev_is_sata(struct domain_device *dev)
> {
> 	return dev->dev_type == SATA_DEV || dev->dev_type == SATA_PM ||
>-	       dev->dev_type == SATA_PM_PORT;
>+	       dev->dev_type == SATA_PM_PORT || dev->dev_type == SATA_PENDING;
> }
> 
>+int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy);
> int sas_ata_init_host_and_port(struct domain_device *found_dev,
> 			       struct scsi_target *starget);
> 
>@@ -82,6 +83,11 @@ static inline void sas_ata_schedule_reset(struct domain_device *dev)
> static inline void sas_ata_wait_eh(struct domain_device *dev)
> {
> }
>+
>+static inline int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
>+{
>+	return 0;
>+}
> #endif
> 
> #endif /* _SAS_ATA_H_ */
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>__________ Information from ESET NOD32 Antivirus, version of virus signature database 5659 (20101129) __________
>
>The message was checked by ESET NOD32 Antivirus.
>
>http://www.eset.com
>
>
>

Re: [PATCH v5 5/7] libsas: fix sas_unregister_ports vs sas_drain_work

[Dan Williams]

On Sat, Jan 14, 2012 at 10:10 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> We need to hold drain_mutex across the unregistration as port down events
> queue device removal as chained events, so we need to make sure no other
> drainers are active.
>
> [ 1118.673968] WARNING: at kernel/workqueue.c:996 __queue_work+0x11a/0x326()
> [ 1118.681982] Hardware name: S2600CP
> [ 1118.686193] Modules linked in: isci(-) libsas scsi_transport_sas nls_utf8
> ipv6 uinput sg iTCO_wdt iTCO_vendor_support i2c_i801 i2c_core ioatdma dca
> sd_mod sr_mod cdrom ahci libahci libata [last unloaded: scsi_transport_sas]
> [ 1118.709893] Pid: 6831, comm: rmmod Not tainted 3.2.0-isci+ #1
> [ 1118.716727] Call Trace:
> [ 1118.719867]  [<ffffffff8103e9f5>] warn_slowpath_common+0x85/0x9d
> [ 1118.727000]  [<ffffffff8103ea27>] warn_slowpath_null+0x1a/0x1c
> [ 1118.733942]  [<ffffffff81056d44>] __queue_work+0x11a/0x326
> [ 1118.740481]  [<ffffffff81056f99>] queue_work_on+0x1b/0x22
> [ 1118.746925]  [<ffffffff81057106>] queue_work+0x37/0x3e
> [ 1118.753105]  [<ffffffffa0120e05>] ? sas_discover_event+0x55/0x82 [libsas]
> [ 1118.761094]  [<ffffffff813217c3>] scsi_queue_work+0x42/0x44
> [ 1118.767717]  [<ffffffffa0120e19>] sas_discover_event+0x69/0x82 [libsas]
> [ 1118.775509]  [<ffffffffa0120f5b>] sas_unregister_dev+0xc3/0xcc [libsas]
> [ 1118.783319]  [<ffffffffa0120fae>] sas_unregister_domain_devices+0x4a/0xc8 [libsas]
> [ 1118.792731]  [<ffffffffa0120071>] sas_deform_port+0x60/0x1a6 [libsas]
> [ 1118.800339]  [<ffffffffa01201ea>] sas_unregister_ports+0x33/0x44 [libsas]
> [ 1118.808342]  [<ffffffffa011f7e5>] sas_unregister_ha+0x41/0x6b [libsas]
> [ 1118.816055]  [<ffffffffa0134055>] isci_unregister+0x22/0x4d [isci]
> [ 1118.823384]  [<ffffffffa0143040>] isci_pci_remove+0x2e/0x60 [isci]
>
> Reported-by: Jacek Danecki <jacek.danecki@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/scsi/libsas/sas_init.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
> index dc93e118..1e13cf4 100644
> --- a/drivers/scsi/libsas/sas_init.c
> +++ b/drivers/scsi/libsas/sas_init.c
> @@ -163,14 +163,18 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
>        unsigned long flags;
>
>        /* Set the state to unregistered to avoid further unchained
> -        * events to be queued
> +        * events to be queued, but since sas_unregister_ports()
> +        * generates unchained events relative to any in progress drains
> +        * we need to hold drain_mutex.
>         */
> +       mutex_lock(&sas_ha->drain_mutex);
>        spin_lock_irqsave(&sas_ha->state_lock, flags);
>        clear_bit(SAS_HA_REGISTERED, &sas_ha->state);
>        spin_unlock_irqrestore(&sas_ha->state_lock, flags);
> -       sas_drain_work(sas_ha);
>
>        sas_unregister_ports(sas_ha);
> +       mutex_unlock(&sas_ha->drain_mutex);

lockdep correctly points out this won't work as we can't hold the
mutex over transport actions since, transport actions also sometimes
acquire the mutex.