* [PATCH v3 01/14] libsas: introduce sas_drain_work()
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 02/14] libsas: remove ata_port.lock management duties from lldds Dan Williams
` (13 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: Tejun Heo, linux-ide
When an lldd invokes ->notify_port_event() it can trigger a chain of libsas
events to:
1/ form the port and find the direct attached device
2/ if the attached device is an expander perform domain discovery
A call to flush_workqueue() will only flush the initial port formation work.
Currently libsas users need to call scsi_flush_work() up to the max depth of
chain (which will grow from 2 to 3 when ata discovery is moved to its own
discovery event). Instead of open coding multiple calls switch to use
drain_workqueue() to flush sas work.
drain_workqueue() does not handle new work submitted during the drain so
libsas needs a bit of infrastructure to hold off unchained work submissions
while a drain is in flight. A lldd ->notify() event is considered 'unchained'
while a sas_discover_event() is 'chained'. As Tejun notes:
"For now, I think it would be best to add private wrapper in libsas to
support deferring unchained work items while draining."
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/aic94xx/aic94xx_init.c | 2 +
drivers/scsi/isci/host.c | 8 ++---
drivers/scsi/libsas/sas_discover.c | 21 +++++++++++++
drivers/scsi/libsas/sas_event.c | 55 +++++++++++++++++++++++++++++++++++
drivers/scsi/libsas/sas_init.c | 9 ++++--
drivers/scsi/libsas/sas_internal.h | 14 ---------
drivers/scsi/mvsas/mv_sas.c | 2 +
drivers/scsi/pm8001/pm8001_sas.c | 4 ++-
include/scsi/libsas.h | 4 +++
9 files changed, 93 insertions(+), 26 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index 8db4e72..2b3717f 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -971,7 +971,7 @@ static int asd_scan_finished(struct Scsi_Host *shost, unsigned long time)
if (time < HZ)
return 0;
/* Wait for discovery to finish */
- scsi_flush_work(shost);
+ sas_drain_work(SHOST_TO_SAS_HA(shost));
return 1;
}
diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index e7fe9c4..e7e5d06 100644
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -650,15 +650,13 @@ static void isci_host_start_complete(struct isci_host *ihost, enum sci_status co
int isci_host_scan_finished(struct Scsi_Host *shost, unsigned long time)
{
- struct isci_host *ihost = SHOST_TO_SAS_HA(shost)->lldd_ha;
+ struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
+ struct isci_host *ihost = ha->lldd_ha;
if (test_bit(IHOST_START_PENDING, &ihost->flags))
return 0;
- /* todo: use sas_flush_discovery once it is upstream */
- scsi_flush_work(shost);
-
- scsi_flush_work(shost);
+ sas_drain_work(ha);
dev_dbg(&ihost->pdev->dev,
"%s: ihost->status = %d, time = %ld\n",
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index ed04118..32e0117 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -367,6 +367,25 @@ static void sas_revalidate_domain(struct work_struct *work)
/* ---------- Events ---------- */
+static void sas_chain_work(struct sas_ha_struct *ha, struct work_struct *work)
+{
+ /* chained work is not subject to SA_HA_DRAINING or SAS_HA_REGISTERED */
+ scsi_queue_work(ha->core.shost, work);
+}
+
+static void sas_chain_event(int event, unsigned long *pending,
+ struct work_struct *work,
+ struct sas_ha_struct *ha)
+{
+ if (!test_and_set_bit(event, pending)) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&ha->state_lock, flags);
+ sas_chain_work(ha, work);
+ spin_unlock_irqrestore(&ha->state_lock, flags);
+ }
+}
+
int sas_discover_event(struct asd_sas_port *port, enum discover_event ev)
{
struct sas_discovery *disc;
@@ -377,7 +396,7 @@ int sas_discover_event(struct asd_sas_port *port, enum discover_event ev)
BUG_ON(ev >= DISC_NUM_EVENTS);
- sas_queue_event(ev, &disc->pending, &disc->disc_work[ev].work, port->ha);
+ sas_chain_event(ev, &disc->pending, &disc->disc_work[ev].work, port->ha);
return 0;
}
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index 9c084bc..e5035aa 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -22,10 +22,65 @@
*
*/
+#include <linux/export.h>
#include <scsi/scsi_host.h>
#include "sas_internal.h"
#include "sas_dump.h"
+static void sas_queue_work(struct sas_ha_struct *ha, struct work_struct *work)
+{
+ if (!test_bit(SAS_HA_REGISTERED, &ha->state))
+ return;
+
+ if (test_bit(SAS_HA_DRAINING, &ha->state))
+ list_add(&work->entry, &ha->defer_q);
+ else
+ scsi_queue_work(ha->core.shost, work);
+}
+
+static void sas_queue_event(int event, unsigned long *pending,
+ struct work_struct *work,
+ struct sas_ha_struct *ha)
+{
+ if (!test_and_set_bit(event, pending)) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&ha->state_lock, flags);
+ sas_queue_work(ha, work);
+ spin_unlock_irqrestore(&ha->state_lock, flags);
+ }
+}
+
+int sas_drain_work(struct sas_ha_struct *ha)
+{
+ struct workqueue_struct *wq = ha->core.shost->work_q;
+ struct work_struct *w, *_w;
+ int err;
+
+ err = mutex_lock_interruptible(&ha->drain_mutex);
+ if (err)
+ return err;
+
+ set_bit(SAS_HA_DRAINING, &ha->state);
+ /* flush submitters */
+ spin_lock_irq(&ha->state_lock);
+ spin_unlock_irq(&ha->state_lock);
+
+ drain_workqueue(wq);
+
+ spin_lock_irq(&ha->state_lock);
+ clear_bit(SAS_HA_DRAINING, &ha->state);
+ list_for_each_entry_safe(w, _w, &ha->defer_q, entry) {
+ list_del_init(&w->entry);
+ sas_queue_work(ha, w);
+ }
+ spin_unlock_irq(&ha->state_lock);
+ mutex_unlock(&ha->drain_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(sas_drain_work);
+
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_init.c b/drivers/scsi/libsas/sas_init.c
index da244e6..572b943 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -114,6 +114,8 @@ int sas_register_ha(struct sas_ha_struct *sas_ha)
set_bit(SAS_HA_REGISTERED, &sas_ha->state);
spin_lock_init(&sas_ha->state_lock);
+ mutex_init(&sas_ha->drain_mutex);
+ INIT_LIST_HEAD(&sas_ha->defer_q);
error = sas_register_phys(sas_ha);
if (error) {
@@ -157,12 +159,13 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
{
unsigned long flags;
- /* Set the state to unregistered to avoid further
- * events to be queued */
+ /* Set the state to unregistered to avoid further unchained
+ * events to be queued
+ */
spin_lock_irqsave(&sas_ha->state_lock, flags);
clear_bit(SAS_HA_REGISTERED, &sas_ha->state);
spin_unlock_irqrestore(&sas_ha->state_lock, flags);
- scsi_flush_work(sas_ha->core.shost);
+ sas_drain_work(sas_ha);
sas_unregister_ports(sas_ha);
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 1fd84b3..948ea64 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -92,20 +92,6 @@ static inline int sas_smp_host_handler(struct Scsi_Host *shost,
}
#endif
-static inline void sas_queue_event(int event, unsigned long *pending,
- struct work_struct *work,
- struct sas_ha_struct *sas_ha)
-{
- if (!test_and_set_bit(event, pending)) {
- unsigned long flags;
-
- spin_lock_irqsave(&sas_ha->state_lock, flags);
- if (test_bit(SAS_HA_REGISTERED, &sas_ha->state))
- scsi_queue_work(sas_ha->core.shost, work);
- spin_unlock_irqrestore(&sas_ha->state_lock, flags);
- }
-}
-
static inline void sas_fill_in_rphy(struct domain_device *dev,
struct sas_rphy *rphy)
{
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a4884a5..b118e63 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -308,7 +308,7 @@ int mvs_scan_finished(struct Scsi_Host *shost, unsigned long time)
if (mvs_prv->scan_finished == 0)
return 0;
- scsi_flush_work(shost);
+ sas_drain_work(sha);
return 1;
}
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index fb3dc99..13811c7 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -234,12 +234,14 @@ void pm8001_scan_start(struct Scsi_Host *shost)
int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time)
{
+ struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
+
/* give the phy enabling interrupt event time to come in (1s
* is empirically about all it takes) */
if (time < HZ)
return 0;
/* Wait for discovery to finish */
- scsi_flush_work(shost);
+ sas_drain_work(ha);
return 1;
}
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 8e402d5..42900fa 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -330,6 +330,7 @@ struct sas_ha_event {
enum sas_ha_state {
SAS_HA_REGISTERED,
+ SAS_HA_DRAINING,
};
struct sas_ha_struct {
@@ -337,6 +338,8 @@ struct sas_ha_struct {
struct sas_ha_event ha_events[HA_NUM_EVENTS];
unsigned long pending;
+ struct list_head defer_q; /* work queued while draining */
+ struct mutex drain_mutex;
unsigned long state;
spinlock_t state_lock;
@@ -657,6 +660,7 @@ int sas_eh_bus_reset_handler(struct scsi_cmnd *cmd);
extern void sas_target_destroy(struct scsi_target *);
extern int sas_slave_alloc(struct scsi_device *);
extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+extern int sas_drain_work(struct sas_ha_struct *ha);
extern int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
struct request *req);
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 02/14] libsas: remove ata_port.lock management duties from lldds
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
2012-01-06 0:59 ` [PATCH v3 01/14] libsas: introduce sas_drain_work() Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 03/14] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
` (12 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: Xiangliang Yu, linux-ide, Christoph Hellwig, Jack Wang
Each libsas driver (mvsas, pm8001, and isci) has invented a different
method for managing the ap->lock. The lock is held by the ata
->queuecommand() path. mvsas drops it prior to acquiring any internal
locks which allows it to hold its internal lock across calls to
task->task_done(). This capability is important as it is the only way
the driver can flush task->task_done() instances to guarantee that it no
longer has any in-flight references to a domain_device at
->lldd_dev_gone() time.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Jack Wang <jack_wang@usish.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/request.c | 3 +--
drivers/scsi/isci/task.c | 6 ++----
drivers/scsi/isci/task.h | 36 -----------------------------------
drivers/scsi/libsas/sas_ata.c | 36 ++++++++++++++++++++++-------------
drivers/scsi/libsas/sas_scsi_host.c | 6 ++----
drivers/scsi/mvsas/mv_sas.c | 6 ------
drivers/scsi/pm8001/pm8001_sas.c | 6 +-----
7 files changed, 29 insertions(+), 70 deletions(-)
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 192cb48..83383ef 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -3649,8 +3649,7 @@ int isci_request_execute(struct isci_host *ihost, struct isci_remote_device *ide
/* Cause this task to be scheduled in the SCSI error
* handler thread.
*/
- isci_execpath_callback(ihost, task,
- sas_task_abort);
+ sas_task_abort(task);
/* Change the status, since we are holding
* the I/O until it is managed by the SCSI
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 66ad3dc..5901a0e 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -96,8 +96,7 @@ static void isci_task_refuse(struct isci_host *ihost, struct sas_task *task,
__func__, task, response, status);
task->lldd_task = NULL;
-
- isci_execpath_callback(ihost, task, task->task_done);
+ task->task_done(task);
break;
case isci_perform_aborted_io_completion:
@@ -117,8 +116,7 @@ static void isci_task_refuse(struct isci_host *ihost, struct sas_task *task,
"%s: Error - task = %p, response=%d, "
"status=%d\n",
__func__, task, response, status);
-
- isci_execpath_callback(ihost, task, sas_task_abort);
+ sas_task_abort(task);
break;
default:
diff --git a/drivers/scsi/isci/task.h b/drivers/scsi/isci/task.h
index bc78c0a..df8d440 100644
--- a/drivers/scsi/isci/task.h
+++ b/drivers/scsi/isci/task.h
@@ -322,40 +322,4 @@ isci_task_set_completion_status(
return task_notification_selection;
}
-/**
-* isci_execpath_callback() - This function is called from the task
-* execute path when the task needs to callback libsas about the submit-time
-* task failure. The callback occurs either through the task's done function
-* or through sas_task_abort. In the case of regular non-discovery SATA/STP I/O
-* requests, libsas takes the host lock before calling execute task. Therefore
-* in this situation the host lock must be managed before calling the func.
-*
-* @ihost: This parameter is the controller to which the I/O request was sent.
-* @task: This parameter is the I/O request.
-* @func: This parameter is the function to call in the correct context.
-* @status: This parameter is the status code for the completed task.
-*
-*/
-static inline void isci_execpath_callback(struct isci_host *ihost,
- struct sas_task *task,
- void (*func)(struct sas_task *))
-{
- struct domain_device *dev = task->dev;
-
- if (dev_is_sata(dev) && task->uldd_task) {
- unsigned long flags;
-
- /* Since we are still in the submit path, and since
- * libsas takes the host lock on behalf of SATA
- * devices before I/O starts (in the non-discovery case),
- * we need to unlock before we can call the callback function.
- */
- raw_local_irq_save(flags);
- spin_unlock(dev->sata_dev.ap->lock);
- func(task);
- spin_lock(dev->sata_dev.ap->lock);
- raw_local_irq_restore(flags);
- } else
- func(task);
-}
#endif /* !defined(_SCI_TASK_H_) */
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 83118d0..81ce39d 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -166,23 +166,30 @@ qc_already_gone:
static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
{
- int res;
+ unsigned long flags;
struct sas_task *task;
- struct domain_device *dev = qc->ap->private_data;
+ struct scatterlist *sg;
+ int ret = AC_ERR_SYSTEM;
+ unsigned int si, xfer = 0;
+ struct ata_port *ap = qc->ap;
+ struct domain_device *dev = ap->private_data;
struct sas_ha_struct *sas_ha = dev->port->ha;
struct Scsi_Host *host = sas_ha->core.shost;
struct sas_internal *i = to_sas_internal(host->transportt);
- struct scatterlist *sg;
- unsigned int xfer = 0;
- unsigned int si;
+
+ /* TODO: audit callers to ensure they are ready for qc_issue to
+ * unconditionally re-enable interrupts
+ */
+ local_irq_save(flags);
+ spin_unlock(ap->lock);
/* If the device fell off, no sense in issuing commands */
if (dev->gone)
- return AC_ERR_SYSTEM;
+ goto out;
task = sas_alloc_task(GFP_ATOMIC);
if (!task)
- return AC_ERR_SYSTEM;
+ goto out;
task->dev = dev;
task->task_proto = SAS_PROTOCOL_STP;
task->task_done = sas_ata_task_done;
@@ -227,21 +234,24 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
ASSIGN_SAS_TASK(qc->scsicmd, task);
if (sas_ha->lldd_max_execute_num < 2)
- res = i->dft->lldd_execute_task(task, 1, GFP_ATOMIC);
+ ret = i->dft->lldd_execute_task(task, 1, GFP_ATOMIC);
else
- res = sas_queue_up(task);
+ ret = sas_queue_up(task);
/* Examine */
- if (res) {
- SAS_DPRINTK("lldd_execute_task returned: %d\n", res);
+ if (ret) {
+ SAS_DPRINTK("lldd_execute_task returned: %d\n", ret);
if (qc->scsicmd)
ASSIGN_SAS_TASK(qc->scsicmd, NULL);
sas_free_task(task);
- return AC_ERR_SYSTEM;
+ ret = AC_ERR_SYSTEM;
}
- return 0;
+ out:
+ spin_lock(ap->lock);
+ local_irq_restore(flags);
+ return ret;
}
static bool sas_ata_qc_fill_rtf(struct ata_queued_cmd *qc)
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 2a163c7..fd60465 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -198,11 +198,9 @@ int sas_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
}
if (dev_is_sata(dev)) {
- unsigned long flags;
-
- spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
+ spin_lock_irq(dev->sata_dev.ap->lock);
res = ata_sas_queuecmd(cmd, dev->sata_dev.ap);
- spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
+ spin_unlock_irq(dev->sata_dev.ap->lock);
return res;
}
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index b118e63..cd88223 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -893,9 +893,6 @@ static int mvs_task_exec(struct sas_task *task, const int num, gfp_t gfp_flags,
mvi = ((struct mvs_device *)task->dev->lldd_dev)->mvi_info;
- if ((dev->dev_type == SATA_DEV) && (dev->sata_dev.ap != NULL))
- spin_unlock_irq(dev->sata_dev.ap->lock);
-
spin_lock_irqsave(&mvi->lock, flags);
rc = mvs_task_prep(task, mvi, is_tmf, tmf, &pass);
if (rc)
@@ -906,9 +903,6 @@ static int mvs_task_exec(struct sas_task *task, const int num, gfp_t gfp_flags,
(MVS_CHIP_SLOT_SZ - 1));
spin_unlock_irqrestore(&mvi->lock, flags);
- if ((dev->dev_type == SATA_DEV) && (dev->sata_dev.ap != NULL))
- spin_lock_irq(dev->sata_dev.ap->lock);
-
return rc;
}
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 13811c7..5add18c 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -342,7 +342,7 @@ static int pm8001_task_exec(struct sas_task *task, const int num,
struct pm8001_ccb_info *ccb;
u32 tag = 0xdeadbeef, rc, n_elem = 0;
u32 n = num;
- unsigned long flags = 0, flags_libsas = 0;
+ unsigned long flags = 0;
if (!dev->port) {
struct task_status_struct *tsm = &t->task_status;
@@ -366,11 +366,7 @@ static int pm8001_task_exec(struct sas_task *task, const int num,
ts->stat = SAS_PHY_DOWN;
spin_unlock_irqrestore(&pm8001_ha->lock, flags);
- spin_unlock_irqrestore(dev->sata_dev.ap->lock,
- flags_libsas);
t->task_done(t);
- spin_lock_irqsave(dev->sata_dev.ap->lock,
- flags_libsas);
spin_lock_irqsave(&pm8001_ha->lock, flags);
if (n > 1)
t = list_entry(t->list.next,
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 03/14] libsas: prevent domain rediscovery competing with ata error handling
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
2012-01-06 0:59 ` [PATCH v3 01/14] libsas: introduce sas_drain_work() Dan Williams
2012-01-06 0:59 ` [PATCH v3 02/14] libsas: remove ata_port.lock management duties from lldds Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-09 19:14 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 04/14] libsas: fix timeout vs completion race Dan Williams
` (11 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 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 a new cleanup state for domain devices to libsas
'allocated-not-probed'. In this state dev->rphy points to a rphy that
is known to have been through a sas_rphy_add() event. At
sas_unregister_dev() time check if this device is still pending probe
and cleanup accordingly.
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 | 62 +++++++++++++++++++++++++++++++++---
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 | 11 +++++-
include/scsi/sas_ata.h | 5 +++
include/scsi/scsi_transport_sas.h | 1 +
11 files changed, 172 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 81ce39d..5418778 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..8926274 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,42 @@ 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, dev_list_node) {
sas_remove_children(&dev->rphy->dev);
sas_rphy_delete(dev->rphy);
dev->rphy = NULL;
+ sas_unregister_common_dev(port, dev);
+ }
+}
+
+void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
+{
+ if (!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) {
+ sas_rphy_unlink(dev->rphy);
+
+ spin_lock_irq(&port->dev_list_lock);
+ list_move_tail(&dev->dev_list_node, &port->destroy_list);
+ spin_unlock_irq(&port->dev_list_lock);
+
+ 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 +304,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 +370,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 +389,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 +462,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 15d2239..c3846cf 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 a47c7a7..e8e68d0 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 42900fa..c66c034 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 ---------- */
@@ -188,6 +190,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;
enum sas_protocol iproto;
enum sas_protocol tproto;
@@ -223,7 +226,6 @@ struct sas_discovery {
int max_level;
};
-
/* The port struct is Class:RW, driver:RO */
struct asd_sas_port {
/* private: */
@@ -233,6 +235,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;
@@ -331,6 +335,7 @@ struct sas_ha_event {
enum sas_ha_state {
SAS_HA_REGISTERED,
SAS_HA_DRAINING,
+ SAS_HA_ATA_EH_ACTIVE,
};
struct sas_ha_struct {
@@ -343,6 +348,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] 21+ messages in thread* Re: [PATCH v3 03/14] libsas: prevent domain rediscovery competing with ata error handling
2012-01-06 0:59 ` [PATCH v3 03/14] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
@ 2012-01-09 19:14 ` Dan Williams
2012-01-09 20:03 ` Dan Williams
0 siblings, 1 reply; 21+ messages in thread
From: Dan Williams @ 2012-01-09 19:14 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide, Christoph Hellwig
On Thu, Jan 5, 2012 at 4:59 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> +void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
> +{
> + if (!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) {
> + sas_rphy_unlink(dev->rphy);
> +
> + spin_lock_irq(&port->dev_list_lock);
> + list_move_tail(&dev->dev_list_node, &port->destroy_list);
> + spin_unlock_irq(&port->dev_list_lock);
This is too early to make the device appear removed from the domain.
Will revise the patch to re-use disco_list_node to for
port->destroy_list.
> +
> + sas_discover_event(dev->port, DISCE_DESTRUCT);
> }
> - sas_unregister_common_dev(port, dev);
> }
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 03/14] libsas: prevent domain rediscovery competing with ata error handling
2012-01-09 19:14 ` Dan Williams
@ 2012-01-09 20:03 ` Dan Williams
0 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-09 20:03 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide, Christoph Hellwig
On Mon, Jan 9, 2012 at 11:14 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Thu, Jan 5, 2012 at 4:59 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> +void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
>> +{
>> + if (!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) {
>> + sas_rphy_unlink(dev->rphy);
>> +
>> + spin_lock_irq(&port->dev_list_lock);
>> + list_move_tail(&dev->dev_list_node, &port->destroy_list);
>> + spin_unlock_irq(&port->dev_list_lock);
>
> This is too early to make the device appear removed from the domain.
> Will revise the patch to re-use disco_list_node to for
> port->destroy_list.
...we also need to clear the parent sas_port's rphy reference to get
sas_port_delete to skip its call to sas_rphy_delete and leave it to
the DISCE_DESTRUCT event.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 04/14] libsas: fix timeout vs completion race
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (2 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 03/14] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 05/14] libsas: perform sas-transport resets in shost->workq context Dan Williams
` (10 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 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 4fabd51..27d80d6 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 6ee9826..f15e33a 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 1e6e078..37f8e2b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -174,6 +174,7 @@ struct sata_device {
/* ---------- Domain device ---------- */
struct domain_device {
+ spinlock_t done_lock;
enum sas_dev_type dev_type;
enum sas_linkrate linkrate;
@@ -317,6 +318,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;
@@ -333,6 +335,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] 21+ messages in thread* [PATCH v3 05/14] libsas: perform sas-transport resets in shost->workq context
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (3 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 04/14] libsas: fix timeout vs completion race Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 06/14] libsas: sas_phy_enable via transport_sas_phy_reset Dan Williams
` (9 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide
Extend the sas transport class to allow transport users to attach extra
data to a sas_phy (->hostdata). Use this area in libsas to move resets
to workq context in preparation for scheduling ata device resets through
libata-eh.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/libsas/sas_event.c | 2 +
drivers/scsi/libsas/sas_init.c | 59 +++++++++++++++++++++++++++++++++++-
drivers/scsi/libsas/sas_internal.h | 10 ++++++
drivers/scsi/scsi_transport_sas.c | 18 ++++++++++-
include/scsi/scsi_transport_sas.h | 5 ++-
5 files changed, 90 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index 933d757..dbfacee 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,7 +27,7 @@
#include "sas_internal.h"
#include "sas_dump.h"
-static void sas_queue_work(struct sas_ha_struct *ha, struct work_struct *work)
+void sas_queue_work(struct sas_ha_struct *ha, struct work_struct *work)
{
if (!test_bit(SAS_HA_REGISTERED, &ha->state))
return;
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index e17fe35..cb65adf 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -290,9 +290,66 @@ int sas_set_phy_speed(struct sas_phy *phy,
return ret;
}
+static void sas_phy_release(struct sas_phy *phy)
+{
+ kfree(phy->hostdata);
+ phy->hostdata = NULL;
+}
+
+static void phy_reset_work(struct work_struct *work)
+{
+ struct sas_phy_data *d = container_of(work, typeof(*d), reset_work);
+
+ d->reset_result = sas_phy_reset(d->phy, d->hard_reset);
+}
+
+static int sas_phy_setup(struct sas_phy *phy)
+{
+ struct sas_phy_data *d = kzalloc(sizeof(*d), GFP_KERNEL);
+
+ if (!d)
+ return -ENOMEM;
+
+ mutex_init(&d->event_lock);
+ INIT_WORK(&d->reset_work, phy_reset_work);
+ d->phy = phy;
+ phy->hostdata = d;
+
+ return 0;
+}
+
+static int queue_phy_reset(struct sas_phy *phy, int hard_reset)
+{
+ struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);
+ struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
+ struct sas_phy_data *d = phy->hostdata;
+ int rc;
+
+ if (!d)
+ return -ENOMEM;
+
+ /* libsas workqueue coordinates ata-eh reset with discovery */
+ mutex_lock(&d->event_lock);
+ d->reset_result = 0;
+ d->hard_reset = hard_reset;
+
+ spin_lock_irq(&ha->state_lock);
+ sas_queue_work(ha, &d->reset_work);
+ spin_unlock_irq(&ha->state_lock);
+
+ rc = sas_drain_work(ha);
+ if (rc == 0)
+ rc = d->reset_result;
+ mutex_unlock(&d->event_lock);
+
+ return rc;
+}
+
static struct sas_function_template sft = {
.phy_enable = sas_phy_enable,
- .phy_reset = sas_phy_reset,
+ .phy_reset = queue_phy_reset,
+ .phy_setup = sas_phy_setup,
+ .phy_release = sas_phy_release,
.set_phy_speed = sas_set_phy_speed,
.get_linkerrors = sas_get_linkerrors,
.smp_handler = sas_smp_handler,
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 662ffcb..9ba65e0 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -38,6 +38,15 @@
#define TO_SAS_TASK(_scsi_cmd) ((void *)(_scsi_cmd)->host_scribble)
#define ASSIGN_SAS_TASK(_sc, _t) do { (_sc)->host_scribble = (void *) _t; } while (0)
+struct sas_phy_data {
+ /* let reset be performed in sas_queue_work() context */
+ struct sas_phy *phy;
+ struct mutex event_lock;
+ int hard_reset;
+ int reset_result;
+ struct work_struct reset_work;
+};
+
void sas_scsi_recover_host(struct Scsi_Host *shost);
int sas_show_class(enum sas_class class, char *buf);
@@ -66,6 +75,7 @@ void sas_porte_broadcast_rcvd(struct work_struct *work);
void sas_porte_link_reset_err(struct work_struct *work);
void sas_porte_timer_event(struct work_struct *work);
void sas_porte_hard_reset(struct work_struct *work);
+void sas_queue_work(struct sas_ha_struct *ha, struct work_struct *work);
int sas_notify_lldd_dev_found(struct domain_device *);
void sas_notify_lldd_dev_gone(struct domain_device *);
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index 9421bae..ab3bd0b 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -652,9 +652,21 @@ sas_phy_linkerror_attr(running_disparity_error_count);
sas_phy_linkerror_attr(loss_of_dword_sync_count);
sas_phy_linkerror_attr(phy_reset_problem_count);
+static int sas_phy_setup(struct transport_container *tc, struct device *dev,
+ struct device *cdev)
+{
+ struct sas_phy *phy = dev_to_phy(dev);
+ struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);
+ struct sas_internal *i = to_sas_internal(shost->transportt);
+
+ if (i->f->phy_setup)
+ i->f->phy_setup(phy);
+
+ return 0;
+}
static DECLARE_TRANSPORT_CLASS(sas_phy_class,
- "sas_phy", NULL, NULL, NULL);
+ "sas_phy", sas_phy_setup, NULL, NULL);
static int sas_phy_match(struct attribute_container *cont, struct device *dev)
{
@@ -678,7 +690,11 @@ static int sas_phy_match(struct attribute_container *cont, struct device *dev)
static void sas_phy_release(struct device *dev)
{
struct sas_phy *phy = dev_to_phy(dev);
+ struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);
+ struct sas_internal *i = to_sas_internal(shost->transportt);
+ if (i->f->phy_release)
+ i->f->phy_release(phy);
put_device(dev->parent);
kfree(phy);
}
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 6d14daa..42817fa 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -75,7 +75,8 @@ struct sas_phy {
/* for the list of phys belonging to a port */
struct list_head port_siblings;
- struct work_struct reset_work;
+ /* available to the lldd */
+ void *hostdata;
};
#define dev_to_phy(d) \
@@ -169,6 +170,8 @@ struct sas_function_template {
int (*get_bay_identifier)(struct sas_rphy *);
int (*phy_reset)(struct sas_phy *, int);
int (*phy_enable)(struct sas_phy *, int);
+ int (*phy_setup)(struct sas_phy *);
+ void (*phy_release)(struct sas_phy *);
int (*set_phy_speed)(struct sas_phy *, struct sas_phy_linkrates *);
int (*smp_handler)(struct Scsi_Host *, struct sas_rphy *, struct request *);
};
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 06/14] libsas: sas_phy_enable via transport_sas_phy_reset
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (4 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 05/14] libsas: perform sas-transport resets in shost->workq context Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 07/14] libsas: async ata-eh Dan Williams
` (8 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide
Execute the link-reset triggered by sas_phy_enable via
transport_sas_phy_reset so that it can be managed by libata.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/libsas/sas_init.c | 57 ++++++++++++++++++++++++++++++-----
drivers/scsi/libsas/sas_internal.h | 3 ++
drivers/scsi/libsas/sas_scsi_host.c | 1 -
include/scsi/libsas.h | 1 -
4 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index a15fb86..53ae893 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -249,15 +249,15 @@ static int transport_sas_phy_reset(struct sas_phy *phy, int hard_reset)
return ret;
}
-int sas_phy_enable(struct sas_phy *phy, int enable)
+static int sas_phy_enable(struct sas_phy *phy, int enable)
{
int ret;
- enum phy_func command;
+ enum phy_func cmd;
if (enable)
- command = PHY_FUNC_LINK_RESET;
+ cmd = PHY_FUNC_LINK_RESET;
else
- command = PHY_FUNC_DISABLE;
+ cmd = PHY_FUNC_DISABLE;
if (scsi_is_sas_phy_local(phy)) {
struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);
@@ -266,15 +266,21 @@ int sas_phy_enable(struct sas_phy *phy, int enable)
struct sas_internal *i =
to_sas_internal(sas_ha->core.shost->transportt);
- if (!enable) {
+ if (enable)
+ ret = transport_sas_phy_reset(phy, 0);
+ else {
sas_phy_disconnected(asd_phy);
sas_ha->notify_phy_event(asd_phy, PHYE_LOSS_OF_SIGNAL);
+ ret = i->dft->lldd_control_phy(asd_phy, cmd, NULL);
}
- ret = i->dft->lldd_control_phy(asd_phy, command, NULL);
} else {
struct sas_rphy *rphy = dev_to_rphy(phy->dev.parent);
struct domain_device *ddev = sas_find_dev_by_rphy(rphy);
- ret = sas_smp_phy_control(ddev, phy->number, command, NULL);
+
+ if (enable)
+ ret = transport_sas_phy_reset(phy, 0);
+ else
+ ret = sas_smp_phy_control(ddev, phy->number, cmd, NULL);
}
return ret;
}
@@ -357,6 +363,13 @@ static void phy_reset_work(struct work_struct *work)
d->reset_result = transport_sas_phy_reset(d->phy, d->hard_reset);
}
+static void phy_enable_work(struct work_struct *work)
+{
+ struct sas_phy_data *d = container_of(work, typeof(*d), enable_work);
+
+ d->enable_result = sas_phy_enable(d->phy, d->enable);
+}
+
static int sas_phy_setup(struct sas_phy *phy)
{
struct sas_phy_data *d = kzalloc(sizeof(*d), GFP_KERNEL);
@@ -366,6 +379,7 @@ static int sas_phy_setup(struct sas_phy *phy)
mutex_init(&d->event_lock);
INIT_WORK(&d->reset_work, phy_reset_work);
+ INIT_WORK(&d->enable_work, phy_enable_work);
d->phy = phy;
phy->hostdata = d;
@@ -399,8 +413,35 @@ static int queue_phy_reset(struct sas_phy *phy, int hard_reset)
return rc;
}
+static int queue_phy_enable(struct sas_phy *phy, int enable)
+{
+ struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);
+ struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
+ struct sas_phy_data *d = phy->hostdata;
+ int rc;
+
+ if (!d)
+ return -ENOMEM;
+
+ /* libsas workqueue coordinates ata-eh reset with discovery */
+ mutex_lock(&d->event_lock);
+ d->enable_result = 0;
+ d->enable = enable;
+
+ spin_lock_irq(&ha->state_lock);
+ sas_queue_work(ha, &d->enable_work);
+ spin_unlock_irq(&ha->state_lock);
+
+ rc = sas_drain_work(ha);
+ if (rc == 0)
+ rc = d->enable_result;
+ mutex_unlock(&d->event_lock);
+
+ return rc;
+}
+
static struct sas_function_template sft = {
- .phy_enable = sas_phy_enable,
+ .phy_enable = queue_phy_enable,
.phy_reset = queue_phy_reset,
.phy_setup = sas_phy_setup,
.phy_release = sas_phy_release,
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index ae9698d..9e960b2 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -45,6 +45,9 @@ struct sas_phy_data {
int hard_reset;
int reset_result;
struct work_struct reset_work;
+ int enable;
+ int enable_result;
+ struct work_struct enable_work;
};
void sas_scsi_recover_host(struct Scsi_Host *shost);
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 57a3484..b849dcd 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -1077,7 +1077,6 @@ EXPORT_SYMBOL_GPL(sas_change_queue_type);
EXPORT_SYMBOL_GPL(sas_bios_param);
EXPORT_SYMBOL_GPL(sas_task_abort);
EXPORT_SYMBOL_GPL(sas_phy_reset);
-EXPORT_SYMBOL_GPL(sas_phy_enable);
EXPORT_SYMBOL_GPL(sas_eh_device_reset_handler);
EXPORT_SYMBOL_GPL(sas_eh_bus_reset_handler);
EXPORT_SYMBOL_GPL(sas_slave_alloc);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index a32407f..04b74bf 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -630,7 +630,6 @@ extern int sas_unregister_ha(struct sas_ha_struct *);
int sas_set_phy_speed(struct sas_phy *phy,
struct sas_phy_linkrates *rates);
-int sas_phy_enable(struct sas_phy *phy, int enabled);
int sas_phy_reset(struct sas_phy *phy, int hard_reset);
int sas_queue_up(struct sas_task *task);
extern int sas_queuecommand(struct Scsi_Host * ,struct scsi_cmnd *);
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 07/14] libsas: async ata-eh
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (5 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 06/14] libsas: sas_phy_enable via transport_sas_phy_reset Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 08/14] isci: kill iphy->isci_port lookups Dan Williams
` (7 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide
Once sas_ata_hard_reset() starts honoring the 'deadline' parameter a
pathological configuration could take 25 seconds per ata device
(serialized) to recover. Run per-port recoveries in parallel.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/libsas/sas_ata.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 39ce224..537e8da 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -23,6 +23,7 @@
#include <linux/scatterlist.h>
#include <linux/slab.h>
+#include <linux/async.h>
#include <scsi/sas_ata.h>
#include "sas_internal.h"
@@ -605,10 +606,21 @@ int sas_discover_sata(struct domain_device *dev)
return 0;
}
+static void async_sas_ata_eh(void *data, async_cookie_t cookie)
+{
+ struct domain_device *dev = data;
+ struct ata_port *ap = dev->sata_dev.ap;
+ struct sas_ha_struct *ha = dev->port->ha;
+
+ ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error handler");
+ ata_scsi_port_error_handler(ha->core.shost, ap);
+}
+
void sas_ata_strategy_handler(struct Scsi_Host *shost)
{
struct scsi_device *sdev;
struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(shost);
+ LIST_HEAD(async);
/* it's ok to defer revalidation events during ata eh, these
* disks are in one of three states:
@@ -622,14 +634,13 @@ void sas_ata_strategy_handler(struct Scsi_Host *shost)
shost_for_each_device(sdev, shost) {
struct domain_device *ddev = sdev_to_domain_dev(sdev);
- struct ata_port *ap = ddev->sata_dev.ap;
if (!dev_is_sata(ddev))
continue;
- ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error handler");
- ata_scsi_port_error_handler(shost, ap);
+ async_schedule_domain(async_sas_ata_eh, ddev, &async);
}
+ async_synchronize_full_domain(&async);
sas_enable_revalidation(sas_ha);
}
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 08/14] isci: kill iphy->isci_port lookups
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (6 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 07/14] libsas: async ata-eh Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-23 23:45 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 09/14] isci: kill isci_port->status Dan Williams
` (6 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: Maciej Trela, linux-ide
This field is a holdover from the OS abstraction conversion. The stable
phy to port lookups are done via iphy->ownining_port under scic_lock.
After this conversion to use port->lldd_port the only volatile lookup is
the initial lookup in isci_port_formed(). After that point any lookup
via a successfully notified domain_device is guaranteed to be valid
until the domain_device is destroyed.
Delete ->start_complete as it is only set once and is set as a
consequence of the port going link up, by definition of getting a port
formed event the port is "ready".
While we are correcting port lookups also move the asd_sas_port table
out from under the isci_port. This is to preclude any temptation to use
container_of() to convert an asd_sas_port to an isci_port, the
association is dynamic and under libsas control.
Tested-by: Maciej Trela <maciej.trela@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/host.h | 19 --------
drivers/scsi/isci/init.c | 7 ---
drivers/scsi/isci/phy.c | 18 +++++---
drivers/scsi/isci/phy.h | 1
drivers/scsi/isci/port.c | 84 +++++++++++++++++++++++++++----------
drivers/scsi/isci/port.h | 2 -
drivers/scsi/isci/remote_device.c | 29 ++++---------
7 files changed, 83 insertions(+), 77 deletions(-)
diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h
index 646051a..26d4cba 100644
--- a/drivers/scsi/isci/host.h
+++ b/drivers/scsi/isci/host.h
@@ -187,6 +187,7 @@ struct isci_host {
int id; /* unique within a given pci device */
struct isci_phy phys[SCI_MAX_PHYS];
struct isci_port ports[SCI_MAX_PORTS + 1]; /* includes dummy port */
+ struct asd_sas_port sas_ports[SCI_MAX_PORTS];
struct sas_ha_struct sas_ha;
spinlock_t state_lock;
@@ -393,24 +394,6 @@ static inline int sci_remote_device_node_count(struct isci_remote_device *idev)
#define sci_controller_clear_invalid_phy(controller, phy) \
((controller)->invalid_phy_mask &= ~(1 << (phy)->phy_index))
-static inline struct device *sciphy_to_dev(struct isci_phy *iphy)
-{
-
- if (!iphy || !iphy->isci_port || !iphy->isci_port->isci_host)
- return NULL;
-
- return &iphy->isci_port->isci_host->pdev->dev;
-}
-
-static inline struct device *sciport_to_dev(struct isci_port *iport)
-{
-
- if (!iport || !iport->isci_host)
- return NULL;
-
- return &iport->isci_host->pdev->dev;
-}
-
static inline struct device *scirdev_to_dev(struct isci_remote_device *idev)
{
if (!idev || !idev->isci_port || !idev->isci_port->isci_host)
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index f988c16..59f2ae7 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -233,18 +233,13 @@ static int isci_register_sas_ha(struct isci_host *isci_host)
if (!sas_ports)
return -ENOMEM;
- /*----------------- Libsas Initialization Stuff----------------------
- * Set various fields in the sas_ha struct:
- */
-
sas_ha->sas_ha_name = DRV_NAME;
sas_ha->lldd_module = THIS_MODULE;
sas_ha->sas_addr = &isci_host->phys[0].sas_addr[0];
- /* set the array of phy and port structs. */
for (i = 0; i < SCI_MAX_PHYS; i++) {
sas_phys[i] = &isci_host->phys[i].sas_phy;
- sas_ports[i] = &isci_host->ports[i].sas_port;
+ sas_ports[i] = &isci_host->sas_ports[i];
}
sas_ha->sas_phy = sas_phys;
diff --git a/drivers/scsi/isci/phy.c b/drivers/scsi/isci/phy.c
index 35f50c2..59f3d2e 100644
--- a/drivers/scsi/isci/phy.c
+++ b/drivers/scsi/isci/phy.c
@@ -67,6 +67,14 @@ enum sas_linkrate sci_phy_linkrate(struct isci_phy *iphy)
return iphy->max_negotiated_speed;
}
+static struct device *sciphy_to_dev(struct isci_phy *iphy)
+{
+ struct isci_phy *table = iphy - iphy->phy_index;
+ struct isci_host *ihost = container_of(table, typeof(*ihost), phys[0]);
+
+ return &ihost->pdev->dev;
+}
+
static enum sci_status
sci_phy_transport_layer_initialization(struct isci_phy *iphy,
struct scu_transport_layer_registers __iomem *reg)
@@ -1249,7 +1257,6 @@ void isci_phy_init(struct isci_phy *iphy, struct isci_host *ihost, int index)
sas_addr = cpu_to_be64(sci_sas_addr);
memcpy(iphy->sas_addr, &sas_addr, sizeof(sas_addr));
- iphy->isci_port = NULL;
iphy->sas_phy.enabled = 0;
iphy->sas_phy.id = index;
iphy->sas_phy.sas_addr = &iphy->sas_addr[0];
@@ -1283,13 +1290,13 @@ int isci_phy_control(struct asd_sas_phy *sas_phy,
{
int ret = 0;
struct isci_phy *iphy = sas_phy->lldd_phy;
- struct isci_port *iport = iphy->isci_port;
+ struct asd_sas_port *port = sas_phy->port;
struct isci_host *ihost = sas_phy->ha->lldd_ha;
unsigned long flags;
dev_dbg(&ihost->pdev->dev,
"%s: phy %p; func %d; buf %p; isci phy %p, port %p\n",
- __func__, sas_phy, func, buf, iphy, iport);
+ __func__, sas_phy, func, buf, iphy, port);
switch (func) {
case PHY_FUNC_DISABLE:
@@ -1306,11 +1313,10 @@ int isci_phy_control(struct asd_sas_phy *sas_phy,
break;
case PHY_FUNC_HARD_RESET:
- if (!iport)
+ if (!port)
return -ENODEV;
- /* Perform the port reset. */
- ret = isci_port_perform_hard_reset(ihost, iport, iphy);
+ ret = isci_port_perform_hard_reset(ihost, port->lldd_port, iphy);
break;
case PHY_FUNC_GET_EVENTS: {
diff --git a/drivers/scsi/isci/phy.h b/drivers/scsi/isci/phy.h
index 67699c8..a5e1a9e 100644
--- a/drivers/scsi/isci/phy.h
+++ b/drivers/scsi/isci/phy.h
@@ -103,7 +103,6 @@ struct isci_phy {
struct scu_transport_layer_registers __iomem *transport_layer_registers;
struct scu_link_layer_registers __iomem *link_layer_registers;
struct asd_sas_phy sas_phy;
- struct isci_port *isci_port;
u8 sas_addr[SAS_ADDR_SIZE];
union {
struct sas_identify_frame iaf;
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index ac7f277..773ff34 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -60,6 +60,21 @@
#define SCIC_SDS_PORT_HARD_RESET_TIMEOUT (1000)
#define SCU_DUMMY_INDEX (0xFFFF)
+static struct device *sciport_to_dev(struct isci_port *iport)
+{
+ int i = iport->physical_port_index;
+ struct isci_port *table;
+ struct isci_host *ihost;
+
+ if (i == SCIC_SDS_DUMMY_PORT)
+ i = SCI_MAX_PORTS+1;
+
+ table = iport - i;
+ ihost = container_of(table, typeof(*ihost), ports[0]);
+
+ return &ihost->pdev->dev;
+}
+
static void isci_port_change_state(struct isci_port *iport, enum isci_status status)
{
unsigned long flags;
@@ -165,17 +180,13 @@ static void isci_port_link_up(struct isci_host *isci_host,
struct sci_port_properties properties;
unsigned long success = true;
- BUG_ON(iphy->isci_port != NULL);
-
- iphy->isci_port = iport;
-
dev_dbg(&isci_host->pdev->dev,
"%s: isci_port = %p\n",
__func__, iport);
spin_lock_irqsave(&iphy->sas_phy.frame_rcvd_lock, flags);
- isci_port_change_state(iphy->isci_port, isci_starting);
+ isci_port_change_state(iport, isci_starting);
sci_port_get_properties(iport, &properties);
@@ -269,8 +280,6 @@ static void isci_port_link_down(struct isci_host *isci_host,
isci_host->sas_ha.notify_phy_event(&isci_phy->sas_phy,
PHYE_LOSS_OF_SIGNAL);
- isci_phy->isci_port = NULL;
-
dev_dbg(&isci_host->pdev->dev,
"%s: isci_port = %p - Done\n", __func__, isci_port);
}
@@ -288,7 +297,6 @@ static void isci_port_ready(struct isci_host *isci_host, struct isci_port *isci_
dev_dbg(&isci_host->pdev->dev,
"%s: isci_port = %p\n", __func__, isci_port);
- complete_all(&isci_port->start_complete);
isci_port_change_state(isci_port, isci_ready);
return;
}
@@ -1633,7 +1641,6 @@ void isci_port_init(struct isci_port *iport, struct isci_host *ihost, int index)
INIT_LIST_HEAD(&iport->remote_dev_list);
INIT_LIST_HEAD(&iport->domain_dev_list);
spin_lock_init(&iport->state_lock);
- init_completion(&iport->start_complete);
iport->isci_host = ihost;
isci_port_change_state(iport, isci_freed);
}
@@ -1714,24 +1721,55 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
return ret;
}
-/**
- * isci_port_deformed() - This function is called by libsas when a port becomes
- * inactive.
- * @phy: This parameter specifies the libsas phy with the inactive port.
- *
- */
void isci_port_deformed(struct asd_sas_phy *phy)
{
- pr_debug("%s: sas_phy = %p\n", __func__, phy);
+ struct isci_host *ihost = phy->ha->lldd_ha;
+ struct isci_port *iport = phy->port->lldd_port;
+ unsigned long flags;
+ int i;
+
+ /* we got a port notification on a port that was subsequently
+ * torn down and libsas is just now catching up
+ */
+ if (!iport)
+ return;
+
+ spin_lock_irqsave(&ihost->scic_lock, flags);
+ for (i = 0; i < SCI_MAX_PHYS; i++) {
+ if (iport->active_phy_mask & 1 << i)
+ break;
+ }
+ spin_unlock_irqrestore(&ihost->scic_lock, flags);
+
+ if (i >= SCI_MAX_PHYS)
+ dev_dbg(&ihost->pdev->dev, "%s: port: %ld\n",
+ __func__, iport - &ihost->ports[0]);
}
-/**
- * isci_port_formed() - This function is called by libsas when a port becomes
- * active.
- * @phy: This parameter specifies the libsas phy with the active port.
- *
- */
void isci_port_formed(struct asd_sas_phy *phy)
{
- pr_debug("%s: sas_phy = %p, sas_port = %p\n", __func__, phy, phy->port);
+ struct isci_host *ihost = phy->ha->lldd_ha;
+ struct isci_phy *iphy = to_iphy(phy);
+ struct asd_sas_port *port = phy->port;
+ struct isci_port *iport;
+ unsigned long flags;
+ int i;
+
+ /* initial ports are formed as the driver is still initializing,
+ * wait for that process to complete
+ */
+ wait_for_start(ihost);
+
+ spin_lock_irqsave(&ihost->scic_lock, flags);
+ for (i = 0; i < SCI_MAX_PORTS; i++) {
+ iport = &ihost->ports[i];
+ if (iport->active_phy_mask & 1 << iphy->phy_index)
+ break;
+ }
+ spin_unlock_irqrestore(&ihost->scic_lock, flags);
+
+ if (i >= SCI_MAX_PORTS)
+ iport = NULL;
+
+ port->lldd_port = iport;
}
diff --git a/drivers/scsi/isci/port.h b/drivers/scsi/isci/port.h
index cb5ffbc..83de4b4 100644
--- a/drivers/scsi/isci/port.h
+++ b/drivers/scsi/isci/port.h
@@ -92,11 +92,9 @@ enum isci_status {
struct isci_port {
enum isci_status status;
struct isci_host *isci_host;
- struct asd_sas_port sas_port;
struct list_head remote_dev_list;
spinlock_t state_lock;
struct list_head domain_dev_list;
- struct completion start_complete;
struct completion hard_reset_complete;
enum sci_status hard_reset_status;
struct sci_base_state_machine sm;
diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index b207cd3..49259e0 100644
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -1377,31 +1377,18 @@ void isci_remote_device_gone(struct domain_device *dev)
*
* status, zero indicates success.
*/
-int isci_remote_device_found(struct domain_device *domain_dev)
+int isci_remote_device_found(struct domain_device *dev)
{
- struct isci_host *isci_host = dev_to_ihost(domain_dev);
- struct isci_port *isci_port;
- struct isci_phy *isci_phy;
- struct asd_sas_port *sas_port;
- struct asd_sas_phy *sas_phy;
+ struct isci_host *isci_host = dev_to_ihost(dev);
+ struct isci_port *isci_port = dev->port->lldd_port;
struct isci_remote_device *isci_device;
enum sci_status status;
dev_dbg(&isci_host->pdev->dev,
- "%s: domain_device = %p\n", __func__, domain_dev);
+ "%s: domain_device = %p\n", __func__, dev);
- wait_for_start(isci_host);
-
- sas_port = domain_dev->port;
- sas_phy = list_first_entry(&sas_port->phy_list, struct asd_sas_phy,
- port_phy_el);
- isci_phy = to_iphy(sas_phy);
- isci_port = isci_phy->isci_port;
-
- /* we are being called for a device on this port,
- * so it has to come up eventually
- */
- wait_for_completion(&isci_port->start_complete);
+ if (!isci_port)
+ return -ENODEV;
if ((isci_stopping == isci_port_get_state(isci_port)) ||
(isci_stopped == isci_port_get_state(isci_port)))
@@ -1415,7 +1402,7 @@ int isci_remote_device_found(struct domain_device *domain_dev)
INIT_LIST_HEAD(&isci_device->node);
spin_lock_irq(&isci_host->scic_lock);
- isci_device->domain_dev = domain_dev;
+ isci_device->domain_dev = dev;
isci_device->isci_port = isci_port;
list_add_tail(&isci_device->node, &isci_port->remote_dev_list);
@@ -1428,7 +1415,7 @@ int isci_remote_device_found(struct domain_device *domain_dev)
if (status == SCI_SUCCESS) {
/* device came up, advertise it to the world */
- domain_dev->lldd_dev = isci_device;
+ dev->lldd_dev = isci_device;
} else
isci_put_device(isci_device);
spin_unlock_irq(&isci_host->scic_lock);
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v3 08/14] isci: kill iphy->isci_port lookups
2012-01-06 0:59 ` [PATCH v3 08/14] isci: kill iphy->isci_port lookups Dan Williams
@ 2012-01-23 23:45 ` Dan Williams
0 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-23 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Maciej Trela, linux-ide, David Milburn
On Thu, Jan 5, 2012 at 4:59 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> This field is a holdover from the OS abstraction conversion. The stable
> phy to port lookups are done via iphy->ownining_port under scic_lock.
> After this conversion to use port->lldd_port the only volatile lookup is
> the initial lookup in isci_port_formed(). After that point any lookup
> via a successfully notified domain_device is guaranteed to be valid
> until the domain_device is destroyed.
>
> Delete ->start_complete as it is only set once and is set as a
> consequence of the port going link up, by definition of getting a port
> formed event the port is "ready".
>
> While we are correcting port lookups also move the asd_sas_port table
> out from under the isci_port. This is to preclude any temptation to use
> container_of() to convert an asd_sas_port to an isci_port, the
> association is dynamic and under libsas control.
>
> Tested-by: Maciej Trela <maciej.trela@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
[..]
> @@ -1714,24 +1721,55 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
> return ret;
> }
>
> -/**
> - * isci_port_deformed() - This function is called by libsas when a port becomes
> - * inactive.
> - * @phy: This parameter specifies the libsas phy with the inactive port.
> - *
> - */
> void isci_port_deformed(struct asd_sas_phy *phy)
> {
> - pr_debug("%s: sas_phy = %p\n", __func__, phy);
> + struct isci_host *ihost = phy->ha->lldd_ha;
> + struct isci_port *iport = phy->port->lldd_port;
> + unsigned long flags;
> + int i;
> +
> + /* we got a port notification on a port that was subsequently
> + * torn down and libsas is just now catching up
> + */
> + if (!iport)
> + return;
> +
> + spin_lock_irqsave(&ihost->scic_lock, flags);
> + for (i = 0; i < SCI_MAX_PHYS; i++) {
> + if (iport->active_phy_mask & 1 << i)
> + break;
> + }
> + spin_unlock_irqrestore(&ihost->scic_lock, flags);
> +
> + if (i >= SCI_MAX_PHYS)
> + dev_dbg(&ihost->pdev->dev, "%s: port: %ld\n",
> + __func__, iport - &ihost->ports[0]);
> }
As spotted by David this causes 32-bit build breakage. I'll fold in
the fix, rebase, and re-post for the next round of libsas fixups.
--
Dan
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 09/14] isci: kill isci_port->status
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (7 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 08/14] isci: kill iphy->isci_port lookups Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 10/14] isci: fix interpretation of "hard" reset Dan Williams
` (5 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide
It only tracks whether the port is stopping in order to gate new devices
being discovered while the port is stopping. However, since the check
and subsequent handling is unlocked there is nothing to stop the port
from going down immediately after the check.
Driver is already prepared to handle devices arriving on stale ports,
and those will be cleaned up by an eventual ->lldd_dev_gone()
notification.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/port.c | 89 +++++--------------------------------
drivers/scsi/isci/port.h | 5 --
drivers/scsi/isci/remote_device.c | 4 --
3 files changed, 11 insertions(+), 87 deletions(-)
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index 773ff34..255c52f 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -75,20 +75,6 @@ static struct device *sciport_to_dev(struct isci_port *iport)
return &ihost->pdev->dev;
}
-static void isci_port_change_state(struct isci_port *iport, enum isci_status status)
-{
- unsigned long flags;
-
- dev_dbg(&iport->isci_host->pdev->dev,
- "%s: iport = %p, state = 0x%x\n",
- __func__, iport, status);
-
- /* XXX pointless lock */
- spin_lock_irqsave(&iport->state_lock, flags);
- iport->status = status;
- spin_unlock_irqrestore(&iport->state_lock, flags);
-}
-
static void sci_port_get_protocols(struct isci_port *iport, struct sci_phy_proto *proto)
{
u8 index;
@@ -186,8 +172,6 @@ static void isci_port_link_up(struct isci_host *isci_host,
spin_lock_irqsave(&iphy->sas_phy.frame_rcvd_lock, flags);
- isci_port_change_state(iport, isci_starting);
-
sci_port_get_properties(iport, &properties);
if (iphy->protocol == SCIC_SDS_PHY_PROTOCOL_SATA) {
@@ -269,7 +253,6 @@ static void isci_port_link_down(struct isci_host *isci_host,
__func__, isci_device);
set_bit(IDEV_GONE, &isci_device->flags);
}
- isci_port_change_state(isci_port, isci_stopping);
}
}
@@ -284,45 +267,6 @@ static void isci_port_link_down(struct isci_host *isci_host,
"%s: isci_port = %p - Done\n", __func__, isci_port);
}
-
-/**
- * isci_port_ready() - This function is called by the sci core when a link
- * becomes ready.
- * @isci_host: This parameter specifies the isci host object.
- * @port: This parameter specifies the sci port with the active link.
- *
- */
-static void isci_port_ready(struct isci_host *isci_host, struct isci_port *isci_port)
-{
- dev_dbg(&isci_host->pdev->dev,
- "%s: isci_port = %p\n", __func__, isci_port);
-
- isci_port_change_state(isci_port, isci_ready);
- return;
-}
-
-/**
- * isci_port_not_ready() - This function is called by the sci core when a link
- * is not ready. All remote devices on this link will be removed if they are
- * in the stopping state.
- * @isci_host: This parameter specifies the isci host object.
- * @port: This parameter specifies the sci port with the active link.
- *
- */
-static void isci_port_not_ready(struct isci_host *isci_host, struct isci_port *isci_port)
-{
- dev_dbg(&isci_host->pdev->dev,
- "%s: isci_port = %p\n", __func__, isci_port);
-}
-
-static void isci_port_stop_complete(struct isci_host *ihost,
- struct isci_port *iport,
- enum sci_status completion_status)
-{
- dev_dbg(&ihost->pdev->dev, "Port stop complete\n");
-}
-
-
static bool is_port_ready_state(enum sci_port_states state)
{
switch (state) {
@@ -828,10 +772,9 @@ static void port_timeout(unsigned long data)
__func__,
iport);
} else if (current_state == SCI_PORT_STOPPING) {
- /* if the port is still stopping then the stop has not completed */
- isci_port_stop_complete(iport->owning_controller,
- iport,
- SCI_FAILURE_TIMEOUT);
+ dev_dbg(sciport_to_dev(iport),
+ "%s: port%d: stop complete timeout\n",
+ __func__, iport->physical_port_index);
} else {
/* The port is in the ready state and we have a timer
* reporting a timeout this should not happen.
@@ -989,7 +932,8 @@ static void sci_port_ready_substate_operational_enter(struct sci_base_state_mach
struct isci_port *iport = container_of(sm, typeof(*iport), sm);
struct isci_host *ihost = iport->owning_controller;
- isci_port_ready(ihost, iport);
+ dev_dbg(&ihost->pdev->dev, "%s: port%d ready\n",
+ __func__, iport->physical_port_index);
for (index = 0; index < SCI_MAX_PHYS; index++) {
if (iport->phy_table[index]) {
@@ -1055,7 +999,8 @@ static void sci_port_ready_substate_operational_exit(struct sci_base_state_machi
*/
sci_port_abort_dummy_request(iport);
- isci_port_not_ready(ihost, iport);
+ dev_dbg(&ihost->pdev->dev, "%s: port%d !ready\n",
+ __func__, iport->physical_port_index);
if (iport->ready_exit)
sci_port_invalidate_dummy_remote_node(iport);
@@ -1067,7 +1012,8 @@ static void sci_port_ready_substate_configuring_enter(struct sci_base_state_mach
struct isci_host *ihost = iport->owning_controller;
if (iport->active_phy_mask == 0) {
- isci_port_not_ready(ihost, iport);
+ dev_dbg(&ihost->pdev->dev, "%s: port%d !ready\n",
+ __func__, iport->physical_port_index);
port_state_machine_change(iport,
SCI_PORT_SUB_WAITING);
@@ -1544,7 +1490,8 @@ static void sci_port_ready_state_enter(struct sci_base_state_machine *sm)
if (prev_state == SCI_PORT_RESETTING)
isci_port_hard_reset_complete(iport, SCI_SUCCESS);
else
- isci_port_not_ready(ihost, iport);
+ dev_dbg(&ihost->pdev->dev, "%s: port%d !ready\n",
+ __func__, iport->physical_port_index);
/* Post and suspend the dummy remote node context for this port. */
sci_port_post_dummy_remote_node(iport);
@@ -1640,21 +1587,7 @@ void isci_port_init(struct isci_port *iport, struct isci_host *ihost, int index)
{
INIT_LIST_HEAD(&iport->remote_dev_list);
INIT_LIST_HEAD(&iport->domain_dev_list);
- spin_lock_init(&iport->state_lock);
iport->isci_host = ihost;
- isci_port_change_state(iport, isci_freed);
-}
-
-/**
- * isci_port_get_state() - This function gets the status of the port object.
- * @isci_port: This parameter points to the isci_port object
- *
- * status of the object as a isci_status enum.
- */
-enum isci_status isci_port_get_state(
- struct isci_port *isci_port)
-{
- return isci_port->status;
}
void sci_port_broadcast_change_received(struct isci_port *iport, struct isci_phy *iphy)
diff --git a/drivers/scsi/isci/port.h b/drivers/scsi/isci/port.h
index 83de4b4..b0b7cc1 100644
--- a/drivers/scsi/isci/port.h
+++ b/drivers/scsi/isci/port.h
@@ -90,10 +90,8 @@ enum isci_status {
* @timer: timeout start/stop operations
*/
struct isci_port {
- enum isci_status status;
struct isci_host *isci_host;
struct list_head remote_dev_list;
- spinlock_t state_lock;
struct list_head domain_dev_list;
struct completion hard_reset_complete;
enum sci_status hard_reset_status;
@@ -284,9 +282,6 @@ void sci_port_get_attached_sas_address(
struct isci_port *iport,
struct sci_sas_address *sas_address);
-enum isci_status isci_port_get_state(
- struct isci_port *isci_port);
-
void isci_port_formed(struct asd_sas_phy *);
void isci_port_deformed(struct asd_sas_phy *);
diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index 49259e0..967394d 100644
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -1390,10 +1390,6 @@ int isci_remote_device_found(struct domain_device *dev)
if (!isci_port)
return -ENODEV;
- if ((isci_stopping == isci_port_get_state(isci_port)) ||
- (isci_stopped == isci_port_get_state(isci_port)))
- return -ENODEV;
-
isci_device = isci_remote_device_alloc(isci_host, isci_port);
if (!isci_device)
return -ENODEV;
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 10/14] isci: fix interpretation of "hard" reset
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (8 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 09/14] isci: kill isci_port->status Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 0:59 ` [PATCH v3 11/14] isci: stop interpreting ->lldd_lu_reset() as an ata soft-reset Dan Williams
` (4 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide
A hard reset to isci in the direct-attached case is one where the driver
internally manages debouncing the link. In the sas-expander-attached
case a hard reset is one that clears affiliations. The driver should
not be prematurely dropping affiliations at run time, that decision (to
force expander hard resets to ata devices) is left to userspace to
manage. So, arrange for I_T_nexus resets to be sas-link-resets in the
expander-attached case and isci-hard-resets in the direct-attached case.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/port.c | 31 ++++++++++++++++++++-----------
drivers/scsi/isci/port.h | 3 ++-
drivers/scsi/isci/task.c | 24 +++++++++++++++---------
3 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index 255c52f..7e4a9ee 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -305,7 +305,9 @@ static void port_state_machine_change(struct isci_port *iport,
static void isci_port_hard_reset_complete(struct isci_port *isci_port,
enum sci_status completion_status)
{
- dev_dbg(&isci_port->isci_host->pdev->dev,
+ struct isci_host *ihost = isci_port->owning_controller;
+
+ dev_dbg(&ihost->pdev->dev,
"%s: isci_port = %p, completion_status=%x\n",
__func__, isci_port, completion_status);
@@ -316,23 +318,24 @@ static void isci_port_hard_reset_complete(struct isci_port *isci_port,
/* The reset failed. The port state is now SCI_PORT_FAILED. */
if (isci_port->active_phy_mask == 0) {
+ int phy_idx = isci_port->last_active_phy;
+ struct isci_phy *iphy = &ihost->phys[phy_idx];
/* Generate the link down now to the host, since it
* was intercepted by the hard reset state machine when
* it really happened.
*/
- isci_port_link_down(isci_port->isci_host,
- &isci_port->isci_host->phys[
- isci_port->last_active_phy],
- isci_port);
+ isci_port_link_down(ihost, iphy, isci_port);
}
/* Advance the port state so that link state changes will be
- * noticed.
- */
+ * noticed.
+ */
port_state_machine_change(isci_port, SCI_PORT_SUB_WAITING);
}
- complete_all(&isci_port->hard_reset_complete);
+ clear_bit(IPORT_RESET_PENDING, &isci_port->state);
+ wake_up(&ihost->eventq);
+
}
/* This method will return a true value if the specified phy can be assigned to
@@ -1598,6 +1601,11 @@ void sci_port_broadcast_change_received(struct isci_port *iport, struct isci_phy
isci_port_bc_change_received(ihost, iport, iphy);
}
+static void wait_port_reset(struct isci_host *ihost, struct isci_port *iport)
+{
+ wait_event(ihost->eventq, !test_bit(IPORT_RESET_PENDING, &iport->state));
+}
+
int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *iport,
struct isci_phy *iphy)
{
@@ -1608,9 +1616,8 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
dev_dbg(&ihost->pdev->dev, "%s: iport = %p\n",
__func__, iport);
- init_completion(&iport->hard_reset_complete);
-
spin_lock_irqsave(&ihost->scic_lock, flags);
+ set_bit(IPORT_RESET_PENDING, &iport->state);
#define ISCI_PORT_RESET_TIMEOUT SCIC_SDS_SIGNATURE_FIS_TIMEOUT
status = sci_port_hard_reset(iport, ISCI_PORT_RESET_TIMEOUT);
@@ -1618,7 +1625,7 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
spin_unlock_irqrestore(&ihost->scic_lock, flags);
if (status == SCI_SUCCESS) {
- wait_for_completion(&iport->hard_reset_complete);
+ wait_port_reset(ihost, iport);
dev_dbg(&ihost->pdev->dev,
"%s: iport = %p; hard reset completion\n",
@@ -1632,6 +1639,8 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
__func__, iport, iport->hard_reset_status);
}
} else {
+ clear_bit(IPORT_RESET_PENDING, &iport->state);
+ wake_up(&ihost->eventq);
ret = TMF_RESP_FUNC_FAILED;
dev_err(&ihost->pdev->dev,
diff --git a/drivers/scsi/isci/port.h b/drivers/scsi/isci/port.h
index b0b7cc1..78e1e82 100644
--- a/drivers/scsi/isci/port.h
+++ b/drivers/scsi/isci/port.h
@@ -93,7 +93,8 @@ struct isci_port {
struct isci_host *isci_host;
struct list_head remote_dev_list;
struct list_head domain_dev_list;
- struct completion hard_reset_complete;
+ #define IPORT_RESET_PENDING 0
+ unsigned long state;
enum sci_status hard_reset_status;
struct sci_base_state_machine sm;
bool ready_exit;
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index a6ab49a..0e7429b 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -1330,29 +1330,35 @@ isci_task_request_complete(struct isci_host *ihost,
}
static int isci_reset_device(struct isci_host *ihost,
+ struct domain_device *dev,
struct isci_remote_device *idev)
{
- struct sas_phy *phy = sas_get_local_phy(idev->domain_dev);
- enum sci_status status;
- unsigned long flags;
int rc;
+ unsigned long flags;
+ enum sci_status status;
+ struct sas_phy *phy = sas_get_local_phy(dev);
+ struct isci_port *iport = dev->port->lldd_port;
dev_dbg(&ihost->pdev->dev, "%s: idev %p\n", __func__, idev);
spin_lock_irqsave(&ihost->scic_lock, flags);
status = sci_remote_device_reset(idev);
- if (status != SCI_SUCCESS) {
- spin_unlock_irqrestore(&ihost->scic_lock, flags);
+ spin_unlock_irqrestore(&ihost->scic_lock, flags);
+ if (status != SCI_SUCCESS) {
dev_dbg(&ihost->pdev->dev,
"%s: sci_remote_device_reset(%p) returned %d!\n",
__func__, idev, status);
rc = TMF_RESP_FUNC_FAILED;
goto out;
}
- spin_unlock_irqrestore(&ihost->scic_lock, flags);
- rc = sas_phy_reset(phy, true);
+ if (scsi_is_sas_phy_local(phy)) {
+ struct isci_phy *iphy = &ihost->phys[phy->number];
+
+ rc = isci_port_perform_hard_reset(ihost, iport, iphy);
+ } else
+ rc = sas_phy_reset(phy, !dev_is_sata(dev));
/* Terminate in-progress I/O now. */
isci_remote_device_nuke_requests(ihost, idev);
@@ -1390,7 +1396,7 @@ int isci_task_I_T_nexus_reset(struct domain_device *dev)
goto out;
}
- ret = isci_reset_device(ihost, idev);
+ ret = isci_reset_device(ihost, dev, idev);
out:
isci_put_device(idev);
return ret;
@@ -1413,7 +1419,7 @@ int isci_bus_reset_handler(struct scsi_cmnd *cmd)
goto out;
}
- ret = isci_reset_device(ihost, idev);
+ ret = isci_reset_device(ihost, dev, idev);
out:
isci_put_device(idev);
return ret;
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 11/14] isci: stop interpreting ->lldd_lu_reset() as an ata soft-reset
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (9 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 10/14] isci: fix interpretation of "hard" reset Dan Williams
@ 2012-01-06 0:59 ` Dan Williams
2012-01-06 1:00 ` [PATCH v3 12/14] isci: ->lldd_ata_check_ready handler Dan Williams
` (3 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 0:59 UTC (permalink / raw)
To: linux-scsi; +Cc: Xiangliang Yu, linux-ide, Jack Wang
Driving resets from libsas-eh is pre-mature as libata will make a
decision about performing a softreset. Currently libata determines
whether to perform a softreset based on ata_eh_followup_srst_needed(),
and none of those conditions apply to isci.
Remove the srst implementation and translate ->lldd_lu_reset() for ata
devices as a request to drive a reset via libata-eh.
Cc: Jack Wang <jack_wang@usish.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/request.c | 195 +----------------------------------------
drivers/scsi/isci/request.h | 9 --
drivers/scsi/isci/task.c | 93 +-------------------
drivers/scsi/isci/task.h | 2
drivers/scsi/libsas/sas_ata.c | 2
5 files changed, 15 insertions(+), 286 deletions(-)
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 83383ef..0e43efd 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -519,18 +519,12 @@ sci_io_request_construct_sata(struct isci_request *ireq,
if (test_bit(IREQ_TMF, &ireq->flags)) {
struct isci_tmf *tmf = isci_request_access_tmf(ireq);
- if (tmf->tmf_code == isci_tmf_sata_srst_high ||
- tmf->tmf_code == isci_tmf_sata_srst_low) {
- scu_stp_raw_request_construct_task_context(ireq);
- return SCI_SUCCESS;
- } else {
- dev_err(&ireq->owning_controller->pdev->dev,
- "%s: Request 0x%p received un-handled SAT "
- "management protocol 0x%x.\n",
- __func__, ireq, tmf->tmf_code);
+ dev_err(&ireq->owning_controller->pdev->dev,
+ "%s: Request 0x%p received un-handled SAT "
+ "management protocol 0x%x.\n",
+ __func__, ireq, tmf->tmf_code);
- return SCI_FAILURE;
- }
+ return SCI_FAILURE;
}
if (!sas_protocol_ata(task->task_proto)) {
@@ -627,34 +621,6 @@ static enum sci_status sci_io_request_construct_basic_sata(struct isci_request *
return status;
}
-enum sci_status sci_task_request_construct_sata(struct isci_request *ireq)
-{
- enum sci_status status = SCI_SUCCESS;
-
- /* check for management protocols */
- if (test_bit(IREQ_TMF, &ireq->flags)) {
- struct isci_tmf *tmf = isci_request_access_tmf(ireq);
-
- if (tmf->tmf_code == isci_tmf_sata_srst_high ||
- tmf->tmf_code == isci_tmf_sata_srst_low) {
- scu_stp_raw_request_construct_task_context(ireq);
- } else {
- dev_err(&ireq->owning_controller->pdev->dev,
- "%s: Request 0x%p received un-handled SAT "
- "Protocol 0x%x.\n",
- __func__, ireq, tmf->tmf_code);
-
- return SCI_FAILURE;
- }
- }
-
- if (status != SCI_SUCCESS)
- return status;
- sci_change_state(&ireq->sm, SCI_REQ_CONSTRUCTED);
-
- return status;
-}
-
/**
* sci_req_tx_bytes - bytes transferred when reply underruns request
* @ireq: request that was terminated early
@@ -756,9 +722,6 @@ sci_io_request_terminate(struct isci_request *ireq)
case SCI_REQ_STP_PIO_WAIT_FRAME:
case SCI_REQ_STP_PIO_DATA_IN:
case SCI_REQ_STP_PIO_DATA_OUT:
- case SCI_REQ_STP_SOFT_RESET_WAIT_H2D_ASSERTED:
- case SCI_REQ_STP_SOFT_RESET_WAIT_H2D_DIAG:
- case SCI_REQ_STP_SOFT_RESET_WAIT_D2H:
case SCI_REQ_ATAPI_WAIT_H2D:
case SCI_REQ_ATAPI_WAIT_PIO_SETUP:
case SCI_REQ_ATAPI_WAIT_D2H:
@@ -1938,59 +1901,6 @@ sci_io_request_frame_handler(struct isci_request *ireq,
return status;
}
- case SCI_REQ_STP_SOFT_RESET_WAIT_D2H: {
- struct dev_to_host_fis *frame_header;
- u32 *frame_buffer;
-
- status = sci_unsolicited_frame_control_get_header(&ihost->uf_control,
- frame_index,
- (void **)&frame_header);
- if (status != SCI_SUCCESS) {
- dev_err(&ihost->pdev->dev,
- "%s: SCIC IO Request 0x%p could not get frame "
- "header for frame index %d, status %x\n",
- __func__,
- stp_req,
- frame_index,
- status);
- return status;
- }
-
- switch (frame_header->fis_type) {
- case FIS_REGD2H:
- sci_unsolicited_frame_control_get_buffer(&ihost->uf_control,
- frame_index,
- (void **)&frame_buffer);
-
- sci_controller_copy_sata_response(&ireq->stp.rsp,
- frame_header,
- frame_buffer);
-
- /* The command has completed with error */
- ireq->scu_status = SCU_TASK_DONE_CHECK_RESPONSE;
- ireq->sci_status = SCI_FAILURE_IO_RESPONSE_VALID;
- break;
-
- default:
- dev_warn(&ihost->pdev->dev,
- "%s: IO Request:0x%p Frame Id:%d protocol "
- "violation occurred\n",
- __func__,
- stp_req,
- frame_index);
-
- ireq->scu_status = SCU_TASK_DONE_UNEXP_FIS;
- ireq->sci_status = SCI_FAILURE_PROTOCOL_VIOLATION;
- break;
- }
-
- sci_change_state(&ireq->sm, SCI_REQ_COMPLETED);
-
- /* Frame has been decoded return it to the controller */
- sci_controller_release_frame(ihost, frame_index);
-
- return status;
- }
case SCI_REQ_ATAPI_WAIT_PIO_SETUP: {
struct sas_task *task = isci_request_access_task(ireq);
@@ -2088,57 +1998,6 @@ static enum sci_status stp_request_udma_await_tc_event(struct isci_request *ireq
return status;
}
-static enum sci_status
-stp_request_soft_reset_await_h2d_asserted_tc_event(struct isci_request *ireq,
- u32 completion_code)
-{
- switch (SCU_GET_COMPLETION_TL_STATUS(completion_code)) {
- case SCU_MAKE_COMPLETION_STATUS(SCU_TASK_DONE_GOOD):
- ireq->scu_status = SCU_TASK_DONE_GOOD;
- ireq->sci_status = SCI_SUCCESS;
- sci_change_state(&ireq->sm, SCI_REQ_STP_SOFT_RESET_WAIT_H2D_DIAG);
- break;
-
- default:
- /*
- * All other completion status cause the IO to be complete.
- * If a NAK was received, then it is up to the user to retry
- * the request.
- */
- ireq->scu_status = SCU_NORMALIZE_COMPLETION_STATUS(completion_code);
- ireq->sci_status = SCI_FAILURE_CONTROLLER_SPECIFIC_IO_ERR;
- sci_change_state(&ireq->sm, SCI_REQ_COMPLETED);
- break;
- }
-
- return SCI_SUCCESS;
-}
-
-static enum sci_status
-stp_request_soft_reset_await_h2d_diagnostic_tc_event(struct isci_request *ireq,
- u32 completion_code)
-{
- switch (SCU_GET_COMPLETION_TL_STATUS(completion_code)) {
- case SCU_MAKE_COMPLETION_STATUS(SCU_TASK_DONE_GOOD):
- ireq->scu_status = SCU_TASK_DONE_GOOD;
- ireq->sci_status = SCI_SUCCESS;
- sci_change_state(&ireq->sm, SCI_REQ_STP_SOFT_RESET_WAIT_D2H);
- break;
-
- default:
- /* All other completion status cause the IO to be complete. If
- * a NAK was received, then it is up to the user to retry the
- * request.
- */
- ireq->scu_status = SCU_NORMALIZE_COMPLETION_STATUS(completion_code);
- ireq->sci_status = SCI_FAILURE_CONTROLLER_SPECIFIC_IO_ERR;
- sci_change_state(&ireq->sm, SCI_REQ_COMPLETED);
- break;
- }
-
- return SCI_SUCCESS;
-}
-
static enum sci_status atapi_raw_completion(struct isci_request *ireq, u32 completion_code,
enum sci_base_request_states next)
{
@@ -2284,14 +2143,6 @@ sci_io_request_tc_completion(struct isci_request *ireq,
case SCI_REQ_STP_PIO_DATA_OUT:
return pio_data_out_tx_done_tc_event(ireq, completion_code);
- case SCI_REQ_STP_SOFT_RESET_WAIT_H2D_ASSERTED:
- return stp_request_soft_reset_await_h2d_asserted_tc_event(ireq,
- completion_code);
-
- case SCI_REQ_STP_SOFT_RESET_WAIT_H2D_DIAG:
- return stp_request_soft_reset_await_h2d_diagnostic_tc_event(ireq,
- completion_code);
-
case SCI_REQ_ABORTING:
return request_aborting_state_tc_event(ireq,
completion_code);
@@ -3065,10 +2916,6 @@ static void sci_request_started_state_enter(struct sci_base_state_machine *sm)
*/
if (!task && dev->dev_type == SAS_END_DEV) {
state = SCI_REQ_TASK_WAIT_TC_COMP;
- } else if (!task &&
- (isci_request_access_tmf(ireq)->tmf_code == isci_tmf_sata_srst_high ||
- isci_request_access_tmf(ireq)->tmf_code == isci_tmf_sata_srst_low)) {
- state = SCI_REQ_STP_SOFT_RESET_WAIT_H2D_ASSERTED;
} else if (task && task->task_proto == SAS_PROTOCOL_SMP) {
state = SCI_REQ_SMP_WAIT_RESP;
} else if (task && sas_protocol_ata(task->task_proto) &&
@@ -3125,31 +2972,6 @@ static void sci_stp_request_started_pio_await_h2d_completion_enter(struct sci_ba
ireq->target_device->working_request = ireq;
}
-static void sci_stp_request_started_soft_reset_await_h2d_asserted_completion_enter(struct sci_base_state_machine *sm)
-{
- struct isci_request *ireq = container_of(sm, typeof(*ireq), sm);
-
- ireq->target_device->working_request = ireq;
-}
-
-static void sci_stp_request_started_soft_reset_await_h2d_diagnostic_completion_enter(struct sci_base_state_machine *sm)
-{
- struct isci_request *ireq = container_of(sm, typeof(*ireq), sm);
- struct scu_task_context *tc = ireq->tc;
- struct host_to_dev_fis *h2d_fis;
- enum sci_status status;
-
- /* Clear the SRST bit */
- h2d_fis = &ireq->stp.cmd;
- h2d_fis->control = 0;
-
- /* Clear the TC control bit */
- tc->control_frame = 0;
-
- status = sci_controller_continue_io(ireq);
- WARN_ONCE(status != SCI_SUCCESS, "isci: continue io failure\n");
-}
-
static const struct sci_base_state sci_request_state_table[] = {
[SCI_REQ_INIT] = { },
[SCI_REQ_CONSTRUCTED] = { },
@@ -3168,13 +2990,6 @@ static const struct sci_base_state sci_request_state_table[] = {
[SCI_REQ_STP_PIO_DATA_OUT] = { },
[SCI_REQ_STP_UDMA_WAIT_TC_COMP] = { },
[SCI_REQ_STP_UDMA_WAIT_D2H] = { },
- [SCI_REQ_STP_SOFT_RESET_WAIT_H2D_ASSERTED] = {
- .enter_state = sci_stp_request_started_soft_reset_await_h2d_asserted_completion_enter,
- },
- [SCI_REQ_STP_SOFT_RESET_WAIT_H2D_DIAG] = {
- .enter_state = sci_stp_request_started_soft_reset_await_h2d_diagnostic_completion_enter,
- },
- [SCI_REQ_STP_SOFT_RESET_WAIT_D2H] = { },
[SCI_REQ_TASK_WAIT_TC_COMP] = { },
[SCI_REQ_TASK_WAIT_TC_RESP] = { },
[SCI_REQ_SMP_WAIT_RESP] = { },
diff --git a/drivers/scsi/isci/request.h b/drivers/scsi/isci/request.h
index be38933..bcf2f37 100644
--- a/drivers/scsi/isci/request.h
+++ b/drivers/scsi/isci/request.h
@@ -211,10 +211,6 @@ enum sci_base_request_states {
SCI_REQ_STP_NON_DATA_WAIT_H2D,
SCI_REQ_STP_NON_DATA_WAIT_D2H,
- SCI_REQ_STP_SOFT_RESET_WAIT_H2D_ASSERTED,
- SCI_REQ_STP_SOFT_RESET_WAIT_H2D_DIAG,
- SCI_REQ_STP_SOFT_RESET_WAIT_D2H,
-
/*
* While in this state the IO request object is waiting for the TC
* completion notification for the H2D Register FIS
@@ -446,10 +442,7 @@ sci_task_request_construct(struct isci_host *ihost,
struct isci_remote_device *idev,
u16 io_tag,
struct isci_request *ireq);
-enum sci_status
-sci_task_request_construct_ssp(struct isci_request *ireq);
-enum sci_status
-sci_task_request_construct_sata(struct isci_request *ireq);
+enum sci_status sci_task_request_construct_ssp(struct isci_request *ireq);
void sci_smp_request_copy_response(struct isci_request *ireq);
static inline int isci_task_is_ncq_recovery(struct sas_task *task)
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 0e7429b..48df8e5 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -247,46 +247,6 @@ int isci_task_execute_task(struct sas_task *task, int num, gfp_t gfp_flags)
return 0;
}
-static enum sci_status isci_sata_management_task_request_build(struct isci_request *ireq)
-{
- struct isci_tmf *isci_tmf;
- enum sci_status status;
-
- if (!test_bit(IREQ_TMF, &ireq->flags))
- return SCI_FAILURE;
-
- isci_tmf = isci_request_access_tmf(ireq);
-
- switch (isci_tmf->tmf_code) {
-
- case isci_tmf_sata_srst_high:
- case isci_tmf_sata_srst_low: {
- struct host_to_dev_fis *fis = &ireq->stp.cmd;
-
- memset(fis, 0, sizeof(*fis));
-
- fis->fis_type = 0x27;
- fis->flags &= ~0x80;
- fis->flags &= 0xF0;
- if (isci_tmf->tmf_code == isci_tmf_sata_srst_high)
- fis->control |= ATA_SRST;
- else
- fis->control &= ~ATA_SRST;
- break;
- }
- /* other management commnd go here... */
- default:
- return SCI_FAILURE;
- }
-
- /* core builds the protocol specific request
- * based on the h2d fis.
- */
- status = sci_task_request_construct_sata(ireq);
-
- return status;
-}
-
static struct isci_request *isci_task_request_build(struct isci_host *ihost,
struct isci_remote_device *idev,
u16 tag, struct isci_tmf *isci_tmf)
@@ -326,13 +286,6 @@ static struct isci_request *isci_task_request_build(struct isci_host *ihost,
return NULL;
}
- if (dev->dev_type == SATA_DEV || (dev->tproto & SAS_PROTOCOL_STP)) {
- isci_tmf->proto = SAS_PROTOCOL_SATA;
- status = isci_sata_management_task_request_build(ireq);
-
- if (status != SCI_SUCCESS)
- return NULL;
- }
return ireq;
}
@@ -871,53 +824,20 @@ static int isci_task_send_lu_reset_sas(
return ret;
}
-static int isci_task_send_lu_reset_sata(struct isci_host *ihost,
- struct isci_remote_device *idev, u8 *lun)
+int isci_task_lu_reset(struct domain_device *dev, u8 *lun)
{
- int ret = TMF_RESP_FUNC_FAILED;
- struct isci_tmf tmf;
-
- /* Send the soft reset to the target */
- #define ISCI_SRST_TIMEOUT_MS 25000 /* 25 second timeout. */
- isci_task_build_tmf(&tmf, isci_tmf_sata_srst_high, NULL, NULL);
-
- ret = isci_task_execute_tmf(ihost, idev, &tmf, ISCI_SRST_TIMEOUT_MS);
-
- if (ret != TMF_RESP_FUNC_COMPLETE) {
- dev_dbg(&ihost->pdev->dev,
- "%s: Assert SRST failed (%p) = %x",
- __func__, idev, ret);
-
- /* Return the failure so that the LUN reset is escalated
- * to a target reset.
- */
- }
- return ret;
-}
-
-/**
- * isci_task_lu_reset() - This function is one of the SAS Domain Template
- * functions. This is one of the Task Management functoins called by libsas,
- * to reset the given lun. Note the assumption that while this call is
- * executing, no I/O will be sent by the host to the device.
- * @lun: This parameter specifies the lun to be reset.
- *
- * status, zero indicates success.
- */
-int isci_task_lu_reset(struct domain_device *domain_device, u8 *lun)
-{
- struct isci_host *isci_host = dev_to_ihost(domain_device);
+ struct isci_host *isci_host = dev_to_ihost(dev);
struct isci_remote_device *isci_device;
unsigned long flags;
int ret;
spin_lock_irqsave(&isci_host->scic_lock, flags);
- isci_device = isci_lookup_device(domain_device);
+ isci_device = isci_lookup_device(dev);
spin_unlock_irqrestore(&isci_host->scic_lock, flags);
dev_dbg(&isci_host->pdev->dev,
"%s: domain_device=%p, isci_host=%p; isci_device=%p\n",
- __func__, domain_device, isci_host, isci_device);
+ __func__, dev, isci_host, isci_device);
if (!isci_device) {
/* If the device is gone, stop the escalations. */
@@ -929,8 +849,9 @@ int isci_task_lu_reset(struct domain_device *domain_device, u8 *lun)
set_bit(IDEV_EH, &isci_device->flags);
/* Send the task management part of the reset. */
- if (sas_protocol_ata(domain_device->tproto)) {
- ret = isci_task_send_lu_reset_sata(isci_host, isci_device, lun);
+ if (dev_is_sata(dev)) {
+ sas_ata_schedule_reset(dev);
+ ret = TMF_RESP_FUNC_COMPLETE;
} else
ret = isci_task_send_lu_reset_sas(isci_host, isci_device, lun);
diff --git a/drivers/scsi/isci/task.h b/drivers/scsi/isci/task.h
index df8d440..8ffcdc9 100644
--- a/drivers/scsi/isci/task.h
+++ b/drivers/scsi/isci/task.h
@@ -86,8 +86,6 @@ enum isci_tmf_function_codes {
isci_tmf_func_none = 0,
isci_tmf_ssp_task_abort = TMF_ABORT_TASK,
isci_tmf_ssp_lun_reset = TMF_LU_RESET,
- isci_tmf_sata_srst_high = TMF_LU_RESET + 0x100, /* Non SCSI */
- isci_tmf_sata_srst_low = TMF_LU_RESET + 0x101 /* Non SCSI */
};
/**
* struct isci_tmf - This class represents the task management object which
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 84eae8b..f90fdcf 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -24,6 +24,7 @@
#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/async.h>
+#include <linux/export.h>
#include <scsi/sas_ata.h>
#include "sas_internal.h"
@@ -757,6 +758,7 @@ void sas_ata_schedule_reset(struct domain_device *dev)
ata_port_schedule_eh(ap);
spin_unlock_irqrestore(ap->lock, flags);
}
+EXPORT_SYMBOL_GPL(sas_ata_schedule_reset);
void sas_ata_wait_eh(struct domain_device *dev)
{
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 12/14] isci: ->lldd_ata_check_ready handler
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (10 preceding siblings ...)
2012-01-06 0:59 ` [PATCH v3 11/14] isci: stop interpreting ->lldd_lu_reset() as an ata soft-reset Dan Williams
@ 2012-01-06 1:00 ` Dan Williams
2012-01-09 1:57 ` Jack Wang
2012-01-06 1:00 ` [PATCH v3 13/14] isci: remove bus and reset handlers Dan Williams
` (2 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Dan Williams @ 2012-01-06 1:00 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide
Report to libata whether the link to the given domain_device is up and the
signature fis has been received.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/init.c | 3 +++
drivers/scsi/isci/port.c | 28 ++++++++++++++++++++++++++++
drivers/scsi/isci/port.h | 1 +
3 files changed, 32 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..e795645 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -1663,6 +1663,34 @@ 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;
+
+ /* snapshot active phy mask */
+ spin_lock_irqsave(&ihost->scic_lock, flags);
+ rc = !!iport->active_phy_mask;
+ spin_unlock_irqrestore(&ihost->scic_lock, flags);
+ 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] 21+ messages in thread* RE: [PATCH v3 12/14] isci: ->lldd_ata_check_ready handler
2012-01-06 1:00 ` [PATCH v3 12/14] isci: ->lldd_ata_check_ready handler Dan Williams
@ 2012-01-09 1:57 ` Jack Wang
2012-01-09 19:26 ` Dan Williams
0 siblings, 1 reply; 21+ messages in thread
From: Jack Wang @ 2012-01-09 1:57 UTC (permalink / raw)
To: 'Dan Williams', linux-scsi; +Cc: linux-ide
> +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;
> +
> + /* snapshot active phy mask */
> + spin_lock_irqsave(&ihost->scic_lock, flags);
> + rc = !!iport->active_phy_mask;
[Jack Wang]
Hi Dan,
Could you explain why here you not directly use "
rc = !!iport->active_phy_mask;
"
> + spin_unlock_irqrestore(&ihost->scic_lock, flags);
> + 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_) */
>
> --
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 12/14] isci: ->lldd_ata_check_ready handler
2012-01-09 1:57 ` Jack Wang
@ 2012-01-09 19:26 ` Dan Williams
0 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-09 19:26 UTC (permalink / raw)
To: Jack Wang; +Cc: linux-scsi, linux-ide
On Sun, Jan 8, 2012 at 5:57 PM, Jack Wang <jack_wang@usish.com> wrote:
>> +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;
>> +
>> + /* snapshot active phy mask */
>> + spin_lock_irqsave(&ihost->scic_lock, flags);
>> + rc = !!iport->active_phy_mask;
> [Jack Wang]
> Hi Dan,
>
> Could you explain why here you not directly use "
> rc = !!iport->active_phy_mask;
You are right, it can look at the field outside the lock... and since
we have already taken the lock to get a device reference we know we
have flushed any recent port events.
--
Dan
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 13/14] isci: remove bus and reset handlers
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (11 preceding siblings ...)
2012-01-06 1:00 ` [PATCH v3 12/14] isci: ->lldd_ata_check_ready handler Dan Williams
@ 2012-01-06 1:00 ` Dan Williams
2012-01-06 1:00 ` [PATCH v3 14/14] isci: remove IDEV_EH hack to disable "discovery-time" ata resets Dan Williams
2012-01-06 1:46 ` [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 1:00 UTC (permalink / raw)
To: linux-scsi; +Cc: Xiangliang Yu, linux-ide, Luben Tuikov, Jack Wang
Remove ->eh_device_reset_handler() and ->eh_bus_reset_handler() for the
same reason they are not implemented for libata hosts, they cannot be
implemented reliably with ata-eh. ATA error recovery wants to divert
all resets to the eh thread and wait for completion, these handlers may
be invoked from a non-blocking ioctl.
The other path they are called from is libsas-eh, and if we escalate
past I_T_nexus reset we have larger problems i.e. tear down all
in-flight commands in the domain potentially without notification to the
lldd if it has chosen not to implement ->lldd_clear_nexus_port() /
->lldd_clear_nexus_ha().
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/isci/init.c | 2 --
drivers/scsi/isci/task.c | 23 -----------------------
drivers/scsi/isci/task.h | 2 --
3 files changed, 0 insertions(+), 27 deletions(-)
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 40293b3..437f76b 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -157,8 +157,6 @@ static struct scsi_host_template isci_sht = {
.sg_tablesize = SG_ALL,
.max_sectors = SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_bus_reset_handler = isci_bus_reset_handler,
.slave_alloc = sas_slave_alloc,
.target_destroy = sas_target_destroy,
.ioctl = sas_ioctl,
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 48df8e5..edaba78 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -1322,26 +1322,3 @@ int isci_task_I_T_nexus_reset(struct domain_device *dev)
isci_put_device(idev);
return ret;
}
-
-int isci_bus_reset_handler(struct scsi_cmnd *cmd)
-{
- struct domain_device *dev = sdev_to_domain_dev(cmd->device);
- struct isci_host *ihost = dev_to_ihost(dev);
- struct isci_remote_device *idev;
- unsigned long flags;
- int ret;
-
- spin_lock_irqsave(&ihost->scic_lock, flags);
- idev = isci_lookup_device(dev);
- spin_unlock_irqrestore(&ihost->scic_lock, flags);
-
- if (!idev) {
- ret = TMF_RESP_FUNC_COMPLETE;
- goto out;
- }
-
- ret = isci_reset_device(ihost, dev, idev);
- out:
- isci_put_device(idev);
- return ret;
-}
diff --git a/drivers/scsi/isci/task.h b/drivers/scsi/isci/task.h
index 8ffcdc9..7bc6fd6 100644
--- a/drivers/scsi/isci/task.h
+++ b/drivers/scsi/isci/task.h
@@ -209,8 +209,6 @@ int isci_queuecommand(
struct scsi_cmnd *scsi_cmd,
void (*donefunc)(struct scsi_cmnd *));
-int isci_bus_reset_handler(struct scsi_cmnd *cmd);
-
/**
* enum isci_completion_selection - This enum defines the possible actions to
* take with respect to a given request's notification back to libsas.
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 14/14] isci: remove IDEV_EH hack to disable "discovery-time" ata resets
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (12 preceding siblings ...)
2012-01-06 1:00 ` [PATCH v3 13/14] isci: remove bus and reset handlers Dan Williams
@ 2012-01-06 1:00 ` Dan Williams
2012-01-06 1:46 ` [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 1:00 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide, Xiangliang Yu
Prior to commit 61aaff49 "isci: filter broadcast change notifications
during SMP phy resets" we borrowed the MVS_DEV_EH approach from the
mvsas driver for preventing ->lldd_I_T_nexus_reset() events during ata
discovery. This hack was protecting against the old ->phy_reset() in
ata_bus_probe(), but since the conversion to the new error handling this
hack is preventing resets from reaching ata devices.
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/remote_device.c | 1 -
drivers/scsi/isci/remote_device.h | 7 +++----
drivers/scsi/isci/task.c | 9 ++++-----
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index 967394d..f83be32 100644
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -1304,7 +1304,6 @@ void isci_remote_device_release(struct kref *kref)
clear_bit(IDEV_STOP_PENDING, &idev->flags);
clear_bit(IDEV_IO_READY, &idev->flags);
clear_bit(IDEV_GONE, &idev->flags);
- clear_bit(IDEV_EH, &idev->flags);
smp_mb__before_clear_bit();
clear_bit(IDEV_ALLOCATED, &idev->flags);
wake_up(&ihost->eventq);
diff --git a/drivers/scsi/isci/remote_device.h b/drivers/scsi/isci/remote_device.h
index 483ee50..98c2801 100644
--- a/drivers/scsi/isci/remote_device.h
+++ b/drivers/scsi/isci/remote_device.h
@@ -82,10 +82,9 @@ struct isci_remote_device {
#define IDEV_START_PENDING 0
#define IDEV_STOP_PENDING 1
#define IDEV_ALLOCATED 2
- #define IDEV_EH 3
- #define IDEV_GONE 4
- #define IDEV_IO_READY 5
- #define IDEV_IO_NCQERROR 6
+ #define IDEV_GONE 3
+ #define IDEV_IO_READY 4
+ #define IDEV_IO_NCQERROR 5
unsigned long flags;
struct kref kref;
struct isci_port *isci_port;
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index edaba78..fd9f67c 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -846,7 +846,6 @@ int isci_task_lu_reset(struct domain_device *dev, u8 *lun)
ret = TMF_RESP_FUNC_COMPLETE;
goto out;
}
- set_bit(IDEV_EH, &isci_device->flags);
/* Send the task management part of the reset. */
if (dev_is_sata(dev)) {
@@ -981,9 +980,6 @@ int isci_task_abort_task(struct sas_task *task)
"%s: dev = %p, task = %p, old_request == %p\n",
__func__, isci_device, task, old_request);
- if (isci_device)
- set_bit(IDEV_EH, &isci_device->flags);
-
/* Device reset conditions signalled in task_state_flags are the
* responsbility of libsas to observe at the start of the error
* handler thread.
@@ -1312,7 +1308,10 @@ int isci_task_I_T_nexus_reset(struct domain_device *dev)
idev = isci_lookup_device(dev);
spin_unlock_irqrestore(&ihost->scic_lock, flags);
- if (!idev || !test_bit(IDEV_EH, &idev->flags)) {
+ if (!idev) {
+ /* XXX: need to cleanup any ireqs targeting this
+ * domain_device
+ */
ret = TMF_RESP_FUNC_COMPLETE;
goto out;
}
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...)
2012-01-06 0:59 [GIT PATCH v3 00/14] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
` (13 preceding siblings ...)
2012-01-06 1:00 ` [PATCH v3 14/14] isci: remove IDEV_EH hack to disable "discovery-time" ata resets Dan Williams
@ 2012-01-06 1:46 ` Dan Williams
14 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2012-01-06 1:46 UTC (permalink / raw)
To: linux-scsi; +Cc: linux-ide
On Thu, 2012-01-05 at 16:59 -0800, Dan Williams wrote:
> Note, the patches mailed with this update only include the libsas patches
> that have been revised since v2, and the isci updates that were
> dependent on these changes.
>
> For the full set in proper order see the current state of the 'libsas'
> branch in isci.git (commit c3766a3):
Just noticed that scsi-misc was rebased since last checked. Patch kit
re-flowed (none of the contents changed). 'libsas' is now at commit
5c41dc3:
The following changes since commit 5c41dc3a79150e93e5d050871a10b761be8281a1:
[SCSI] lpfc 8.3.28: Update driver version to 8.3.28 (2011-12-15 10:57:45 +0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/djbw/isci.git libsas
Dan Williams (33):
libsas: remove unused ata_task_resp fields
libsas: kill sas_slave_destroy
libsas: fix domain_device leak
libsas: fix leak of dev->sata_dev.identify_[packet_]device
libsas: replace event locks with atomic bitops
libsas: convert ha->state to flags
libsas: introduce sas_drain_work()
libsas: remove ata_port.lock management duties from lldds
libsas: prevent domain rediscovery competing with ata error handling
libsas: use ->set_dmamode to notify lldds of NCQ parameters
libsas: kill invocation of scsi_eh_finish_cmd from sas_ata_task_done
libsas: close error handling vs sas_ata_task_done() race
libsas: prevent double completion of scmds from eh
libsas: fix timeout vs completion race
libsas: let libata handle command timeouts
libsas: defer SAS_TASK_NEED_DEV_RESET commands to libata
libsas: use libata-eh-reset for sata rediscovery fis transmit failures
libsas: perform sas-transport resets in shost->workq context
libsas: execute transport link resets with libata-eh via host workqueue
libsas: sas_phy_enable via transport_sas_phy_reset
libsas: async ata-eh
libsas: poll for ata device readiness after reset
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: kill iphy->isci_port lookups
isci: kill isci_port->status
isci: fix interpretation of "hard" reset
isci: stop interpreting ->lldd_lu_reset() as an ata soft-reset
isci: ->lldd_ata_check_ready handler
isci: remove bus and reset handlers
isci: remove IDEV_EH hack to disable "discovery-time" ata resets
Jeff Skirvin (2):
libsas: Remove redundant phy state notification calls.
libsas: add mutex for SMP task execution
Documentation/scsi/libsas.txt | 15 -
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 | 5 +-
drivers/scsi/aic94xx/aic94xx_tmf.c | 9 +-
drivers/scsi/isci/host.c | 8 +-
drivers/scsi/isci/host.h | 19 +-
drivers/scsi/isci/init.c | 13 +-
drivers/scsi/isci/phy.c | 18 +-
drivers/scsi/isci/phy.h | 1 -
drivers/scsi/isci/port.c | 220 ++++++------
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 | 198 +----------
drivers/scsi/isci/request.h | 9 +-
drivers/scsi/isci/task.c | 158 ++-------
drivers/scsi/isci/task.h | 40 --
drivers/scsi/libsas/sas_ata.c | 685 +++++++++++++++--------------------
drivers/scsi/libsas/sas_discover.c | 151 +++++++--
drivers/scsi/libsas/sas_event.c | 89 +++++-
drivers/scsi/libsas/sas_expander.c | 107 ++++--
drivers/scsi/libsas/sas_init.c | 192 +++++++++-
drivers/scsi/libsas/sas_internal.h | 73 ++--
drivers/scsi/libsas/sas_phy.c | 12 +-
drivers/scsi/libsas/sas_port.c | 24 +-
drivers/scsi/libsas/sas_scsi_host.c | 299 +++++++---------
drivers/scsi/mvsas/mv_init.c | 1 -
drivers/scsi/mvsas/mv_sas.c | 11 +-
drivers/scsi/pm8001/pm8001_init.c | 1 -
drivers/scsi/pm8001/pm8001_sas.c | 29 +-
drivers/scsi/scsi_transport_sas.c | 59 +++-
include/linux/libata.h | 1 +
include/scsi/libsas.h | 59 ++--
include/scsi/sas_ata.h | 26 +-
include/scsi/scsi_transport_sas.h | 12 +-
38 files changed, 1292 insertions(+), 1344 deletions(-)
^ permalink raw reply [flat|nested] 21+ messages in thread