2006-11-17 21:01:11

by djwong

[permalink] [raw]
Subject: [PATCH 00/15] Roll-up of my libsas, aic94xx and sas_ata patches


Hi all,

This is a roll-up of all of my patches against libsas, aic94xx and
sas_ata to date. The only new patches are #4-5 and #12-15; everything
else has already been seen in some form on this mailing list.
Hopefully this will make things a bit saner since most of these have
been floating out over the past 3 weeks. :)

(Apologies for any stgit mail misconfiguration on my part.)

--D


2006-11-17 21:01:16

by djwong

[permalink] [raw]
Subject: [PATCH 01/15] aic94xx: Handle REQ_DEVICE_RESET


This patch implements a REQ_DEVICE_RESET handler for the aic94xx
driver. Like the earlier REQ_TASK_ABORT patch, this patch defers
the device reset to the Scsi_Host's workqueue, which has the added
benefit of ensuring that the device reset does not happen at the
same time that the ABORT TASK TMFs that are issued in anticipation
of it are being processed. After the phy reset, the busted drive
should go away and be re-detected later, which is indeed what
I've seen on both a x260 and a x206m.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/aic94xx/aic94xx_scb.c | 52 ++++++++++++++++++++++++++++++-----
drivers/scsi/libsas/sas_init.c | 2 +
drivers/scsi/libsas/sas_scsi_host.c | 1 +
include/scsi/libsas.h | 1 +
include/scsi/scsi_transport_sas.h | 2 +
5 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_scb.c b/drivers/scsi/aic94xx/aic94xx_scb.c
index 1911c5d..7145531 100644
--- a/drivers/scsi/aic94xx/aic94xx_scb.c
+++ b/drivers/scsi/aic94xx/aic94xx_scb.c
@@ -342,6 +342,28 @@ void asd_invalidate_edb(struct asd_ascb
asd_printk("couldn't post escb, err:%d\n", i);
}
}
+
+/* hard reset a phy later */
+static void do_phy_reset_later(void *data)
+{
+ struct sas_phy *sas_phy = data;
+ int error;
+
+ ASD_DPRINTK("%s: About to hard reset phy %d\n", __FUNCTION__,
+ sas_phy->identify.phy_identifier);
+ /* Reset device port */
+ error = sas_phy_reset(sas_phy, 1);
+ if (error)
+ ASD_DPRINTK("%s: Hard reset of phy %d failed (%d).\n",
+ __FUNCTION__, sas_phy->identify.phy_identifier,
+ error);
+}
+
+static void phy_reset_later(struct sas_phy *sas_phy, struct Scsi_Host *shost)
+{
+ INIT_WORK(&sas_phy->reset_work, do_phy_reset_later, sas_phy);
+ queue_work(shost->work_q, &sas_phy->reset_work);
+}

/* start up the ABORT TASK tmf... */
static void task_kill_later(struct asd_ascb *ascb)
@@ -402,7 +424,9 @@ static void escb_tasklet_complete(struct
goto out;
}
case REQ_DEVICE_RESET: {
- struct asd_ascb *a, *b;
+ struct Scsi_Host *shost = sas_ha->core.shost;
+ struct sas_phy *dev_phy;
+ struct asd_ascb *a;
u16 conn_handle;

conn_handle = *((u16*)(&dl->status_block[1]));
@@ -412,17 +436,31 @@ static void escb_tasklet_complete(struct
dl->status_block[3]);

/* Kill all pending tasks and reset the device */
- list_for_each_entry_safe(a, b, &asd_ha->seq.pend_q, list) {
- struct sas_task *task = a->uldd_task;
- struct domain_device *dev = task->dev;
+ dev_phy = NULL;
+ list_for_each_entry(a, &asd_ha->seq.pend_q, list) {
+ struct sas_task *task;
+ struct domain_device *dev;
u16 x;

- x = *((u16*)(&dev->lldd_dev));
- if (x == conn_handle)
+ task = a->uldd_task;
+ if (!task)
+ continue;
+ dev = task->dev;
+
+ x = (u16)dev->lldd_dev;
+ if (x == conn_handle) {
+ dev_phy = dev->port->phy;
task_kill_later(a);
+ }
}

- /* FIXME: Reset device port (huh?) */
+ /* Reset device port */
+ if (!dev_phy) {
+ ASD_DPRINTK("%s: No pending commands; can't reset.\n",
+ __FUNCTION__);
+ goto out;
+ }
+ phy_reset_later(dev_phy, shost);
goto out;
}
case SIGNAL_NCQ_ERROR:
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index a2b479a..0fb347b 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -144,7 +144,7 @@ static int sas_get_linkerrors(struct sas
return sas_smp_get_phy_events(phy);
}

-static int sas_phy_reset(struct sas_phy *phy, int hard_reset)
+int sas_phy_reset(struct sas_phy *phy, int hard_reset)
{
int ret;
enum phy_func reset_type;
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 6ccbb62..5c6e6f2 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -925,3 +925,4 @@ EXPORT_SYMBOL_GPL(sas_slave_alloc);
EXPORT_SYMBOL_GPL(sas_target_destroy);
EXPORT_SYMBOL_GPL(sas_ioctl);
EXPORT_SYMBOL_GPL(sas_task_abort);
+EXPORT_SYMBOL_GPL(sas_phy_reset);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 7da678d..921db78 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -605,6 +605,7 @@ struct sas_domain_function_template {
extern int sas_register_ha(struct sas_ha_struct *);
extern int sas_unregister_ha(struct sas_ha_struct *);

+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_cmnd *,
void (*scsi_done)(struct scsi_cmnd *));
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 5302437..59633a8 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -73,6 +73,8 @@ struct sas_phy {

/* for the list of phys belonging to a port */
struct list_head port_siblings;
+
+ struct work_struct reset_work;
};

#define dev_to_phy(d) \

2006-11-17 21:01:47

by djwong

[permalink] [raw]
Subject: [PATCH 04/15] libsas: Don't give scsi_cmnds to the EH if they never made it to the SAS LLDD or have already returned


On a system with many SAS targets, it appears possible that a scsi_cmnd
can time out without ever making it to the SAS LLDD or at the same time
that a completion is occurring. In both of these cases, telling the
LLDD to abort the sas_task makes no sense because the LLDD won't know
about the sas_task; what we really want to do is to increase the timer.
Note that this involves creating another sas_task bit to indicate
whether or not the task has been sent to the LLDD; I could have
implemented this by slightly redefining SAS_TASK_STATE_PENDING, but
this way seems cleaner.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/aic94xx/aic94xx_task.c | 9 +++++++++
drivers/scsi/aic94xx/aic94xx_tmf.c | 4 +++-
drivers/scsi/libsas/sas_ata.c | 1 -
drivers/scsi/libsas/sas_scsi_host.c | 7 +++++++
include/scsi/libsas.h | 1 +
5 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
index 9840708..466b492 100644
--- a/drivers/scsi/aic94xx/aic94xx_task.c
+++ b/drivers/scsi/aic94xx/aic94xx_task.c
@@ -349,6 +349,7 @@ Again:

spin_lock_irqsave(&task->task_state_lock, flags);
task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
+ task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
task->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((task->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&task->task_state_lock, flags);
@@ -556,6 +557,7 @@ int asd_execute_task(struct sas_task *ta
struct sas_task *t = task;
struct asd_ascb *ascb = NULL, *a;
struct asd_ha_struct *asd_ha = task->dev->port->ha->lldd_ha;
+ unsigned long flags;

res = asd_can_queue(asd_ha, num);
if (res)
@@ -601,9 +603,16 @@ int asd_execute_task(struct sas_task *ta
}
list_del_init(&alist);

+ spin_lock_irqsave(&task->task_state_lock, flags);
+ task->task_state_flags |= SAS_TASK_AT_INITIATOR;
+ spin_unlock_irqrestore(&task->task_state_lock, flags);
+
res = asd_post_ascb_list(asd_ha, ascb, num);
if (unlikely(res)) {
a = NULL;
+ spin_lock_irqsave(&task->task_state_lock, flags);
+ task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
+ spin_unlock_irqrestore(&task->task_state_lock, flags);
__list_add(&alist, ascb->list.prev, &ascb->list);
goto out_err_unmap;
}
diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c b/drivers/scsi/aic94xx/aic94xx_tmf.c
index 6123438..686cea1 100644
--- a/drivers/scsi/aic94xx/aic94xx_tmf.c
+++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
@@ -345,7 +345,7 @@ static inline int asd_clear_nexus(struct
int asd_abort_task(struct sas_task *task)
{
struct asd_ascb *tascb = task->lldd_task;
- struct asd_ha_struct *asd_ha = tascb->ha;
+ struct asd_ha_struct *asd_ha;
int res = 1;
unsigned long flags;
struct asd_ascb *ascb = NULL;
@@ -360,6 +360,8 @@ int asd_abort_task(struct sas_task *task
}
spin_unlock_irqrestore(&task->task_state_lock, flags);

+ asd_ha = tascb->ha;
+
ascb = asd_ascb_alloc_list(asd_ha, &res, GFP_KERNEL);
if (!ascb)
return -ENOMEM;
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index de42b5b..f92f035 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -161,7 +161,6 @@ static unsigned int sas_ata_qc_issue(str
task->data_dir = qc->dma_dir;
task->scatter = qc->__sg;
task->ata_task.retry_count = 1;
- task->task_state_flags = SAS_TASK_STATE_PENDING;

switch (qc->tf.protocol) {
case ATA_PROT_NCQ:
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 5c6e6f2..7cc7a1e 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -550,6 +550,13 @@ enum scsi_eh_timer_return sas_scsi_timed
cmd, task);
return 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: "
+ "EH_RESET_TIMER\n",
+ cmd, task);
+ return EH_RESET_TIMER;
+ }
task->task_state_flags |= SAS_TASK_STATE_ABORTED;
spin_unlock_irqrestore(&task->task_state_lock, flags);

diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 921db78..d2ec1be 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -546,6 +546,7 @@ #define SAS_TASK_STATE_PENDING 1
#define SAS_TASK_STATE_DONE 2
#define SAS_TASK_STATE_ABORTED 4
#define SAS_TASK_INITIATOR_ABORTED 8
+#define SAS_TASK_AT_INITIATOR 16

static inline struct sas_task *sas_alloc_task(gfp_t flags)
{

2006-11-17 21:01:36

by djwong

[permalink] [raw]
Subject: [PATCH 02/15] libsas: Clean up rphys/port dev list after a discovery error.


sas_get_port_device assigns a rphy to a domain device in anticipation
of finding a disk. When a discovery error occurs in
sas_discover_{sata,sas,expander}*, however, we need to clean up that
rphy and the port device list so that we don't GPF. In addition, we
need to check the result of the second sas_notify_lldd_dev_found.
This patch seems ok on a x260, x366 and x206m.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_discover.c | 40 ++++++++++++++++++++++++++++++------
drivers/scsi/libsas/sas_expander.c | 20 ++++++++++++++----
2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 9e4fd2a..758b153 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -552,7 +552,7 @@ int sas_discover_sata(struct domain_devi

res = sas_notify_lldd_dev_found(dev);
if (res)
- return res;
+ goto out_err2;

switch (dev->dev_type) {
case SATA_DEV:
@@ -564,12 +564,25 @@ int sas_discover_sata(struct domain_devi
default:
break;
}
+ if (res)
+ goto out_err;

sas_notify_lldd_dev_gone(dev);
- if (!res) {
- sas_notify_lldd_dev_found(dev);
- res = sas_rphy_add(dev->rphy);
- }
+ res = sas_notify_lldd_dev_found(dev);
+ if (res)
+ goto out_err2;
+
+ res = sas_rphy_add(dev->rphy);
+ if (res)
+ goto out_err;
+
+ return res;
+
+out_err:
+ sas_notify_lldd_dev_gone(dev);
+out_err2:
+ sas_rphy_free(dev->rphy);
+ dev->rphy = NULL;
return res;
}

@@ -585,7 +598,7 @@ int sas_discover_end_dev(struct domain_d

res = sas_notify_lldd_dev_found(dev);
if (res)
- return res;
+ goto out_err2;

res = sas_rphy_add(dev->rphy);
if (res)
@@ -594,12 +607,21 @@ int sas_discover_end_dev(struct domain_d
/* do this to get the end device port attributes which will have
* been scanned in sas_rphy_add */
sas_notify_lldd_dev_gone(dev);
- sas_notify_lldd_dev_found(dev);
+ res = sas_notify_lldd_dev_found(dev);
+ if (res)
+ goto out_err3;

return 0;

out_err:
sas_notify_lldd_dev_gone(dev);
+out_err2:
+ sas_rphy_free(dev->rphy);
+ dev->rphy = NULL;
+ return res;
+out_err3:
+ sas_rphy_delete(dev->rphy);
+ dev->rphy = NULL;
return res;
}

@@ -689,6 +711,10 @@ static void sas_discover_domain(void *da
}

if (error) {
+ spin_lock(&port->dev_list_lock);
+ list_del_init(&port->port_dev->dev_list_node);
+ spin_unlock(&port->dev_list_lock);
+
kfree(port->port_dev); /* not kobject_register-ed yet */
port->port_dev = NULL;
}
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 4cc7457..a38d05b 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -711,7 +711,6 @@ static struct domain_device *sas_ex_disc

out_list_del:
list_del(&child->dev_list_node);
- sas_rphy_free(rphy);
out_free:
sas_port_delete(phy->port);
out_err:
@@ -1474,14 +1473,27 @@ int sas_discover_root_expander(struct do
int res;
struct sas_expander_device *ex = rphy_to_expander_device(dev->rphy);

- sas_rphy_add(dev->rphy);
+ res = sas_rphy_add(dev->rphy);
+ if (res)
+ goto out_err;

ex->level = dev->port->disc.max_level; /* 0 */
res = sas_discover_expander(dev);
- if (!res)
- sas_ex_bfs_disc(dev->port);
+ if (res)
+ goto out_err2;
+
+ sas_ex_bfs_disc(dev->port);

return res;
+
+out_err2:
+ sas_rphy_delete(dev->rphy);
+ dev->rphy = NULL;
+ return res;
+out_err:
+ sas_rphy_free(dev->rphy);
+ dev->rphy = NULL;
+ return res;
}

/* ---------- Domain revalidation ---------- */

2006-11-17 21:02:15

by djwong

[permalink] [raw]
Subject: [PATCH 14/15] libsas: Provide a generic SATL registration function


Decouple libsas and sas_ata so that the latter can be provided as a
plug-in module for the former. Any module wishing to provide SATL
services registers itself with libsas; when SATA devices are
discovered, libsas will module_get/put as necessary to ensure that
the module cannot go away accidentally. At this time, the ability
to start a SAS HBA without a SATL, load a SATL later, and then
rerun device discovery; that may be addressed in a later patch.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/Kconfig | 11 +++
drivers/scsi/libsas/sas_discover.c | 6 --
drivers/scsi/libsas/sas_scsi_host.c | 134 ++++++++++++++++++++++++++++++++---
include/scsi/libsas.h | 30 +-------
include/scsi/sas_ata.h | 37 +++++++++-
5 files changed, 174 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index b64e391..9c06eec 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -24,12 +24,21 @@ #

config SCSI_SAS_LIBSAS
tristate "SAS Domain Transport Attributes"
- depends on SCSI && ATA
+ depends on SCSI
select SCSI_SAS_ATTRS
help
This provides transport specific helpers for SAS drivers which
use the domain device construct (like the aic94xxx).

+config SCSI_SAS_SATL
+ tristate "Serial ATA Translation Layer (SATL) on SAS controllers"
+ depends on SCSI_SAS_LIBSAS && ATA
+ default y
+ help
+ This provides an ATA translation layer between libsas and
+ libata to load SATA devices that are connected to SAS
+ controllers.
+
config SCSI_SAS_LIBSAS_DEBUG
bool "Compile the SAS Domain Transport Attributes in debug mode"
default y
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 758b153..01ff15c 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -476,12 +476,6 @@ cont1:
if (!dev->parent)
sas_sata_propagate_sas_addr(dev);

- /* XXX Hint: register this SATA device with SATL.
- When this returns, dev->sata_dev->lu is alive and
- present.
- sas_satl_register_dev(dev);
- */
-
sas_fill_in_rphy(dev, dev->rphy);

return 0;
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 59409be..c1a1e06 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -46,6 +46,9 @@ #include <linux/libata.h>
#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)

+static DEFINE_SPINLOCK(satl_ops_lock);
+static struct satl_operations *satl_ops;
+
static void sas_scsi_task_done(struct sas_task *task)
{
struct task_status_struct *ts = &task->task_status;
@@ -215,8 +218,8 @@ int sas_queuecommand(struct scsi_cmnd *c
unsigned long flags;

spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
- res = ata_sas_queuecmd(cmd, scsi_done,
- dev->sata_dev.ap);
+ res = satl_ops->queuecommand(cmd, scsi_done,
+ dev->sata_dev.ap);
spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
goto out;
}
@@ -574,8 +577,9 @@ int sas_ioctl(struct scsi_device *sdev,
{
struct domain_device *dev = sdev_to_domain_dev(sdev);

- if (dev_is_sata(dev))
- return ata_scsi_ioctl(sdev, cmd, arg);
+ if (dev_is_sata(dev)) {
+ return satl_ops->ioctl(sdev, cmd, arg);
+ }

return -EINVAL;
}
@@ -615,6 +619,29 @@ static inline struct domain_device *sas_
return sas_find_dev_by_rphy(rphy);
}

+static int sas_target_alloc_sata(struct domain_device *dev,
+ struct scsi_target *starget)
+{
+ int res = -ENODEV;
+
+ /* Do we have a SATL available? */
+ if (!get_satl())
+ goto satl_found;
+
+ request_module("sas_ata");
+ if (!get_satl())
+ goto satl_found;
+
+ SAS_DPRINTK("sas_ata not loaded, ignoring SATA devices\n");
+ goto no_satl;
+
+satl_found:
+ res = satl_ops->init_target(dev, starget);
+
+no_satl:
+ return res;
+}
+
int sas_target_alloc(struct scsi_target *starget)
{
struct domain_device *found_dev = sas_find_target(starget);
@@ -624,7 +651,7 @@ int sas_target_alloc(struct scsi_target
return -ENODEV;

if (dev_is_sata(found_dev)) {
- res = sas_ata_init_host_and_port(found_dev, starget);
+ res = sas_target_alloc_sata(found_dev, starget);
if (res)
return res;
}
@@ -644,7 +671,7 @@ int sas_slave_configure(struct scsi_devi
BUG_ON(dev->rphy->identify.device_type != SAS_END_DEVICE);

if (dev_is_sata(dev)) {
- ata_sas_slave_configure(scsi_dev, dev->sata_dev.ap);
+ satl_ops->configure_port(scsi_dev, dev->sata_dev.ap);
return 0;
}

@@ -672,7 +699,7 @@ void sas_slave_destroy(struct scsi_devic
struct domain_device *dev = sdev_to_domain_dev(scsi_dev);

if (dev_is_sata(dev))
- ata_port_disable(dev->sata_dev.ap);
+ satl_ops->deactivate_port(dev->sata_dev.ap);
}

int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth)
@@ -849,7 +876,7 @@ int sas_slave_alloc(struct scsi_device *
struct domain_device *dev = sdev_to_domain_dev(scsi_dev);

if (dev_is_sata(dev))
- return ata_sas_port_init(dev->sata_dev.ap);
+ return satl_ops->init_port(dev->sata_dev.ap);

return 0;
}
@@ -861,8 +888,11 @@ void sas_target_destroy(struct scsi_targ
if (!found_dev)
return;

- if (dev_is_sata(found_dev))
- ata_sas_port_destroy(found_dev->sata_dev.ap);
+ if (dev_is_sata(found_dev)) {
+ if (found_dev->sata_dev.ap)
+ satl_ops->destroy_port(found_dev->sata_dev.ap);
+ put_satl();
+ }

return;
}
@@ -925,6 +955,85 @@ void sas_task_abort(struct sas_task *tas
SAS_DPRINTK("%s: Could not kill task!\n", __FUNCTION__);
}

+struct sas_task *sas_alloc_task(gfp_t flags)
+{
+ extern kmem_cache_t *sas_task_cache;
+ struct sas_task *task = kmem_cache_alloc(sas_task_cache, flags);
+
+ if (task) {
+ memset(task, 0, sizeof(*task));
+ INIT_LIST_HEAD(&task->list);
+ spin_lock_init(&task->task_state_lock);
+ task->task_state_flags = SAS_TASK_STATE_PENDING;
+ init_timer(&task->timer);
+ init_completion(&task->completion);
+ }
+
+ return task;
+}
+
+void sas_free_task(struct sas_task *task)
+{
+ if (task) {
+ extern kmem_cache_t *sas_task_cache;
+ BUG_ON(!list_empty(&task->list));
+ kmem_cache_free(sas_task_cache, task);
+ }
+}
+
+/* Jump table to ATA translation layer functions */
+int sas_register_satl(struct satl_operations *ops)
+{
+ int res = -ENODEV;
+
+ spin_lock(&satl_ops_lock);
+ if (satl_ops)
+ goto out;
+ satl_ops = ops;
+ res = 0;
+out:
+ spin_unlock(&satl_ops_lock);
+ return res;
+}
+
+int sas_unregister_satl(struct satl_operations *ops)
+{
+ int res = -ENODEV;
+
+ spin_lock(&satl_ops_lock);
+ if (satl_ops != ops)
+ goto out;
+ satl_ops = NULL;
+ res = 0;
+out:
+ spin_unlock(&satl_ops_lock);
+ return res;
+}
+
+int get_satl(void)
+{
+ int res = -ENODEV;
+
+ spin_lock(&satl_ops_lock);
+
+ if (!satl_ops)
+ goto out;
+ if (!try_module_get(satl_ops->owner))
+ goto out;
+ res = 0;
+
+out:
+ spin_unlock(&satl_ops_lock);
+ return res;
+}
+
+void put_satl(void)
+{
+ spin_lock(&satl_ops_lock);
+ module_put(satl_ops->owner);
+ spin_unlock(&satl_ops_lock);
+}
+
EXPORT_SYMBOL_GPL(sas_queuecommand);
EXPORT_SYMBOL_GPL(sas_target_alloc);
EXPORT_SYMBOL_GPL(sas_slave_configure);
@@ -939,3 +1048,8 @@ EXPORT_SYMBOL_GPL(sas_task_abort);
EXPORT_SYMBOL_GPL(sas_phy_reset);
EXPORT_SYMBOL_GPL(sas_phy_enable);
EXPORT_SYMBOL_GPL(sas_set_phy_speed);
+EXPORT_SYMBOL_GPL(sas_register_satl);
+EXPORT_SYMBOL_GPL(sas_unregister_satl);
+EXPORT_SYMBOL_GPL(sas_queue_up);
+EXPORT_SYMBOL_GPL(sas_alloc_task);
+EXPORT_SYMBOL_GPL(sas_free_task);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 7dcf593..90748ce 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -549,31 +549,8 @@ #define SAS_TASK_STATE_ABORTED 4
#define SAS_TASK_INITIATOR_ABORTED 8
#define SAS_TASK_AT_INITIATOR 16

-static inline struct sas_task *sas_alloc_task(gfp_t flags)
-{
- extern kmem_cache_t *sas_task_cache;
- struct sas_task *task = kmem_cache_alloc(sas_task_cache, flags);
-
- if (task) {
- memset(task, 0, sizeof(*task));
- INIT_LIST_HEAD(&task->list);
- spin_lock_init(&task->task_state_lock);
- task->task_state_flags = SAS_TASK_STATE_PENDING;
- init_timer(&task->timer);
- init_completion(&task->completion);
- }
-
- return task;
-}
-
-static inline void sas_free_task(struct sas_task *task)
-{
- if (task) {
- extern kmem_cache_t *sas_task_cache;
- BUG_ON(!list_empty(&task->list));
- kmem_cache_free(sas_task_cache, task);
- }
-}
+struct sas_task *sas_alloc_task(gfp_t flags);
+void sas_free_task(struct sas_task *task);

struct sas_domain_function_template {
/* The class calls these to notify the LLDD of an event. */
@@ -649,4 +626,7 @@ extern int sas_slave_alloc(struct scsi_d
extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
void sas_task_abort(struct sas_task *task);

+int get_satl(void);
+void put_satl(void);
+
#endif /* _SASLIB_H_ */
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index 72a1904..bb4a1cb 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -28,12 +28,45 @@ #define _SAS_ATA_H_
#include <linux/libata.h>
#include <scsi/libsas.h>

+struct satl_operations {
+ struct module *owner;
+
+ int (*init_target) (struct domain_device *found_dev,
+ struct scsi_target *starget);
+ int (*queuecommand) (struct scsi_cmnd *cmd,
+ void (*done)(struct scsi_cmnd *),
+ struct ata_port *ap);
+ int (*ioctl) (struct scsi_device *dev, int cmd,
+ void __user *arg);
+ int (*configure_port) (struct scsi_device *,
+ struct ata_port *);
+ void (*deactivate_port) (struct ata_port *);
+ void (*destroy_port) (struct ata_port *);
+ int (*init_port) (struct ata_port *);
+};
+
+int sas_register_satl(struct satl_operations *satl_ops);
+int sas_unregister_satl(struct satl_operations *satl_ops);
+
+#ifdef CONFIG_SCSI_SAS_SATL_MODULE
+# define SAS_ATA_AVAILABLE
+#endif
+
+#ifdef CONFIG_SCSI_SAS_SATL
+# define SAS_ATA_AVAILABLE
+#endif
+
+#ifdef SAS_ATA_AVAILABLE
+
static inline int dev_is_sata(struct domain_device *dev)
{
return (dev->rphy->identify.target_port_protocols & SAS_PROTOCOL_SATA);
}

-int sas_ata_init_host_and_port(struct domain_device *found_dev,
- struct scsi_target *starget);
+#else
+
+#define dev_is_sata(x) (0)
+
+#endif /* SAS_ATA_AVAILABLE */

#endif /* _SAS_ATA_H_ */

2006-11-17 21:02:18

by djwong

[permalink] [raw]
Subject: [PATCH 12/15] sas_ata: Implement sata phy control


This patch requires "libsas: Add a sysfs knob to enable/disable a phy"
to be applied. It hooks the SControl write function to provide basic
SATA phy control for phy enable/disable and speed limits. Power
management is still broken, though it is unclear that libata actually
uses those SControl bits anyway.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_ata.c | 42 ++++++++++++++++++++++++++++++++++-
drivers/scsi/libsas/sas_scsi_host.c | 1 +
2 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index e2650fa..e897140 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -270,6 +270,46 @@ static void sas_ata_tf_read(struct ata_p
memcpy(tf, &dev->sata_dev.tf, sizeof (*tf));
}

+static void sas_ata_scontrol_write(struct domain_device *dev, u32 val)
+{
+ u32 tmp = dev->sata_dev.scontrol;
+ struct sas_phy *phy = dev->port->phy;
+
+ val &= 0x0FF; /* only set max spd and dev ctrl */
+ val |= 0x300; /* disallow host pm */
+ val |= tmp & 0xFFFFF000; /* preserve upper bits */
+
+ /* disable phy */
+ if ((val & 0x4) && !(tmp & 0x4))
+ sas_phy_enable(phy, 0);
+
+ /* enable phy */
+ if (!(val & 0x4) && (tmp & 0x4))
+ sas_phy_enable(phy, 1);
+
+ /* reset phy */
+ if ((val & 0x1) && !(tmp & 0x1))
+ sas_phy_reset(phy, 0);
+
+ /* speed limit */
+ if ((val & 0xF0) != (tmp & 0xF0)) {
+ struct sas_phy_linkrates rates = {0};
+
+ switch ((val & 0xF0) >> 4) {
+ case 0:
+ case 2:
+ rates.maximum_linkrate = SAS_LINK_RATE_3_0_GBPS;
+ break;
+ case 1:
+ rates.maximum_linkrate = SAS_LINK_RATE_1_5_GBPS;
+ break;
+ }
+ sas_set_phy_speed(phy, &rates);
+ }
+
+ dev->sata_dev.scontrol = val;
+}
+
static void sas_ata_scr_write(struct ata_port *ap, unsigned int sc_reg_in,
u32 val)
{
@@ -281,7 +321,7 @@ static void sas_ata_scr_write(struct ata
dev->sata_dev.sstatus = val;
break;
case SCR_CONTROL:
- dev->sata_dev.scontrol = val;
+ sas_ata_scontrol_write(dev, val);
break;
case SCR_ERROR:
dev->sata_dev.serror = val;
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 25639b5..59409be 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -938,3 +938,4 @@ EXPORT_SYMBOL_GPL(sas_ioctl);
EXPORT_SYMBOL_GPL(sas_task_abort);
EXPORT_SYMBOL_GPL(sas_phy_reset);
EXPORT_SYMBOL_GPL(sas_phy_enable);
+EXPORT_SYMBOL_GPL(sas_set_phy_speed);

2006-11-17 21:05:20

by djwong

[permalink] [raw]
Subject: [PATCH 08/15] sas_ata: sas_ata_qc_issue should return AC_ERR_*


The sas_ata_qc_issue function was incorrectly written to return error
codes such as -ENOMEM. Since libata OR's qc->err_mask with the
return value, It is necessary to make my code return one of the
AC_ERR_ codes instead. For now, use AC_ERR_SYSTEM because an error
here means that the OS couldn't send the command to the controller.

If anybody has a suggestion for a better AC_ERR_ code to use, please
suggest it.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_ata.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 32f2b66..a33ef6d 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -123,7 +123,7 @@ static void sas_ata_task_done(struct sas

static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
{
- int res = -ENOMEM;
+ int res;
struct sas_task *task;
struct domain_device *dev = qc->ap->private_data;
struct sas_ha_struct *sas_ha = dev->port->ha;
@@ -135,7 +135,7 @@ static unsigned int sas_ata_qc_issue(str

task = sas_alloc_task(GFP_ATOMIC);
if (!task)
- goto out;
+ return AC_ERR_SYSTEM;
task->dev = dev;
task->task_proto = SAS_PROTOCOL_STP;
task->task_done = sas_ata_task_done;
@@ -186,12 +186,10 @@ static unsigned int sas_ata_qc_issue(str
SAS_DPRINTK("lldd_execute_task returned: %d\n", res);

sas_free_task(task);
- if (res == -SAS_QUEUE_FULL)
- return -ENOMEM;
+ return AC_ERR_SYSTEM;
}

-out:
- return res;
+ return 0;
}

static u8 sas_ata_check_status(struct ata_port *ap)

2006-11-17 21:06:20

by djwong

[permalink] [raw]
Subject: [PATCH 03/15] aic94xx: Delete ascb timers when freeing queues


When the aic94xx driver creates ascbs, each ascb is initialized with a
timeout timer. If there are any ascbs left over when the driver is being
torn down, these timers need to be deleted. In particular, we seem to
hit this case when ascbs are issued yet never end up on the done list.
Right now there's a sequencer bug that results in this happening every
so often. CONTROL PHY commands also use these per-ascb timers.

We also want to print a warning if there are leftover ascbs that are
not for CONTROL PHY commands.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/aic94xx/aic94xx_init.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index f7f009e..d9cf607 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -727,6 +727,15 @@ static void asd_free_queues(struct asd_h

list_for_each_safe(pos, n, &pending) {
struct asd_ascb *ascb = list_entry(pos, struct asd_ascb, list);
+ /*
+ * Delete unexpired ascb timers. This may happen if we issue
+ * a CONTROL PHY scb to an adapter and rmmod before the scb
+ * times out. Apparently we don't wait for the CONTROL PHY
+ * to complete, so it doesn't matter if we kill the timer.
+ */
+ del_timer_sync(&ascb->timer);
+ WARN_ON(ascb->scb->header.opcode != CONTROL_PHY);
+
list_del_init(pos);
ASD_DPRINTK("freeing from pending\n");
asd_ascb_free(ascb);

2006-11-17 21:02:20

by djwong

[permalink] [raw]
Subject: [PATCH 13/15] sas_ata: Implement a libata error handler




Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_ata.c | 24 +++++++++++++++++++++++-
1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index e897140..7338775 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -351,6 +351,27 @@ static u32 sas_ata_scr_read(struct ata_p
}
}

+static int sas_ata_hardreset(struct ata_port *ap, unsigned int *classes)
+{
+ struct domain_device *dev = ap->private_data;
+ struct sas_phy *phy = dev->port->phy;
+
+ return sas_phy_reset(phy, 1);
+}
+
+static int sas_ata_softreset(struct ata_port *ap, unsigned int *classes)
+{
+ struct domain_device *dev = ap->private_data;
+ struct sas_phy *phy = dev->port->phy;
+
+ return sas_phy_reset(phy, 0);
+}
+
+static void sas_ata_eh(struct ata_port *ap)
+{
+ ata_do_eh(ap, NULL, sas_ata_softreset, sas_ata_hardreset, NULL);
+}
+
static struct ata_port_operations sas_sata_ops = {
.port_disable = ata_port_disable,
.check_status = sas_ata_check_status,
@@ -364,7 +385,8 @@ static struct ata_port_operations sas_sa
.port_start = ata_sas_port_start,
.port_stop = ata_sas_port_stop,
.scr_read = sas_ata_scr_read,
- .scr_write = sas_ata_scr_write
+ .scr_write = sas_ata_scr_write,
+ .error_handler = sas_ata_eh
};

static struct ata_port_info sata_port_info = {

2006-11-17 21:06:13

by djwong

[permalink] [raw]
Subject: [PATCH 05/15] libsas: Add a sysfs knob to enable/disable a phy


This patch lets a user arbitrarily enable or disable a phy via sysfs.
Potential applications include shutting down a phy to replace one
lane of wide port, and (more importantly) providing a method for the
libata SATL to control the phy.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_init.c | 35 ++++++++++++++++++++++++++++++++--
drivers/scsi/libsas/sas_scsi_host.c | 1 +
drivers/scsi/scsi_transport_sas.c | 36 +++++++++++++++++++++++++++++++++++
include/scsi/libsas.h | 3 +++
include/scsi/scsi_transport_sas.h | 1 +
5 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 0fb347b..89814f9 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -144,6 +144,36 @@ static int sas_get_linkerrors(struct sas
return sas_smp_get_phy_events(phy);
}

+int sas_phy_enable(struct sas_phy *phy, int enable)
+{
+ int ret;
+ enum phy_func command;
+
+ if (enable)
+ command = PHY_FUNC_LINK_RESET;
+ else
+ command = PHY_FUNC_DISABLE;
+
+ if (scsi_is_sas_phy_local(phy)) {
+ struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);
+ struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(shost);
+ struct asd_sas_phy *asd_phy = sas_ha->sas_phy[phy->number];
+ struct sas_internal *i =
+ to_sas_internal(sas_ha->core.shost->transportt);
+
+ if (!enable) {
+ sas_phy_disconnected(asd_phy);
+ sas_ha->notify_phy_event(asd_phy, PHYE_LOSS_OF_SIGNAL);
+ }
+ 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);
+ }
+ return ret;
+}
+
int sas_phy_reset(struct sas_phy *phy, int hard_reset)
{
int ret;
@@ -170,8 +200,8 @@ int sas_phy_reset(struct sas_phy *phy, i
return ret;
}

-static int sas_set_phy_speed(struct sas_phy *phy,
- struct sas_phy_linkrates *rates)
+int sas_set_phy_speed(struct sas_phy *phy,
+ struct sas_phy_linkrates *rates)
{
int ret;

@@ -210,6 +240,7 @@ static int sas_set_phy_speed(struct sas_
}

static struct sas_function_template sft = {
+ .phy_enable = sas_phy_enable,
.phy_reset = sas_phy_reset,
.set_phy_speed = sas_set_phy_speed,
.get_linkerrors = sas_get_linkerrors,
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 7cc7a1e..a88d3a4 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -933,3 +933,4 @@ EXPORT_SYMBOL_GPL(sas_target_destroy);
EXPORT_SYMBOL_GPL(sas_ioctl);
EXPORT_SYMBOL_GPL(sas_task_abort);
EXPORT_SYMBOL_GPL(sas_phy_reset);
+EXPORT_SYMBOL_GPL(sas_phy_enable);
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index b5b0c2c..04df212 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -335,6 +335,41 @@ show_sas_device_type(struct class_device
}
static CLASS_DEVICE_ATTR(device_type, S_IRUGO, show_sas_device_type, NULL);

+static ssize_t do_sas_phy_enable(struct class_device *cdev,
+ size_t count, int enable)
+{
+ struct sas_phy *phy = transport_class_to_phy(cdev);
+ struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);
+ struct sas_internal *i = to_sas_internal(shost->transportt);
+ int error;
+
+ error = i->f->phy_enable(phy, enable);
+ if (error)
+ return error;
+ return count;
+};
+
+static ssize_t store_sas_phy_enable(struct class_device *cdev,
+ const char *buf, size_t count)
+{
+ if (count < 1)
+ return -EINVAL;
+
+ switch (buf[0]) {
+ case '0':
+ do_sas_phy_enable(cdev, count, 0);
+ break;
+ case '1':
+ do_sas_phy_enable(cdev, count, 1);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return count;
+}
+static CLASS_DEVICE_ATTR(enable, S_IWUSR, NULL, store_sas_phy_enable);
+
static ssize_t do_sas_phy_reset(struct class_device *cdev,
size_t count, int hard_reset)
{
@@ -1478,6 +1513,7 @@ sas_attach_transport(struct sas_function
SETUP_PHY_ATTRIBUTE(phy_reset_problem_count);
SETUP_OPTIONAL_PHY_ATTRIBUTE_WRONLY(link_reset, phy_reset);
SETUP_OPTIONAL_PHY_ATTRIBUTE_WRONLY(hard_reset, phy_reset);
+ SETUP_PHY_ATTRIBUTE_WRONLY(enable);
i->phy_attrs[count] = NULL;

count = 0;
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index d2ec1be..a06cbde 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -606,6 +606,9 @@ struct sas_domain_function_template {
extern int sas_register_ha(struct sas_ha_struct *);
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_cmnd *,
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 59633a8..dea9127 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -163,6 +163,7 @@ struct sas_function_template {
int (*get_enclosure_identifier)(struct sas_rphy *, u64 *);
int (*get_bay_identifier)(struct sas_rphy *);
int (*phy_reset)(struct sas_phy *, int);
+ int (*phy_enable)(struct sas_phy *, int);
int (*set_phy_speed)(struct sas_phy *, struct sas_phy_linkrates *);
};

2006-11-17 21:04:14

by djwong

[permalink] [raw]
Subject: [PATCH 09/15] sas_ata: ata_post_internal should abort the sas_task


This patch adds a new field, lldd_task, to ata_queued_cmd so that libata
users such as libsas can associate some data with a qc. The particular
ambition with this patch is to associate a sas_task with a qc; that way,
if libata decides to timeout a command, we can come back (in
sas_ata_post_internal) and abort the sas task.

One question remains: Is it necessary to reset the phy on error, or will
the libata error handler take care of it? (Assuming that one is written,
of course.) This patch, as it is today, works well enough to clean
things up when an ATA device probe attempt fails halfway through the probe,
though I'm not sure this is always the right thing to do.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_ata.c | 30 +++++++++++++++++++++++++++---
include/linux/libata.h | 1 +
2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index a33ef6d..209f402 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -88,12 +88,17 @@ static enum ata_completion_errors sas_to
static void sas_ata_task_done(struct sas_task *task)
{
struct ata_queued_cmd *qc = task->uldd_task;
- struct domain_device *dev = qc->ap->private_data;
+ struct domain_device *dev;
struct task_status_struct *stat = &task->task_status;
struct ata_task_resp *resp = (struct ata_task_resp *)stat->buf;
enum ata_completion_errors ac;
unsigned long flags;

+ if (!qc)
+ goto qc_already_gone;
+
+ dev = qc->ap->private_data;
+
if (stat->stat == SAS_PROTO_RESPONSE) {
ata_tf_from_fis(resp->ending_fis, &dev->sata_dev.tf);
qc->err_mask |= ac_err_mask(dev->sata_dev.tf.command);
@@ -113,10 +118,12 @@ static void sas_ata_task_done(struct sas
}
}

+ qc->lldd_task = NULL;
spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
ata_qc_complete(qc);
spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);

+qc_already_gone:
list_del_init(&task->list);
sas_free_task(task);
}
@@ -165,6 +172,7 @@ static unsigned int sas_ata_qc_issue(str
task->data_dir = qc->dma_dir;
task->scatter = qc->__sg;
task->ata_task.retry_count = 1;
+ qc->lldd_task = task;

switch (qc->tf.protocol) {
case ATA_PROT_NCQ:
@@ -236,8 +244,24 @@ static void sas_ata_post_internal(struct
if (qc->flags & ATA_QCFLAG_FAILED)
qc->err_mask |= AC_ERR_OTHER;

- if (qc->err_mask)
- SAS_DPRINTK("%s: Failure; reset phy!\n", __FUNCTION__);
+ if (qc->err_mask) {
+ /*
+ * Find the sas_task and kill it. By this point,
+ * libata has decided to kill the qc, so we needn't
+ * bother with sas_ata_task_done. But we still
+ * ought to abort the task.
+ */
+ struct sas_task *task = qc->lldd_task;
+ struct domain_device *dev = qc->ap->private_data;
+
+ qc->lldd_task = NULL;
+ if (task) {
+ task->uldd_task = NULL;
+ sas_task_abort(task);
+ }
+
+ sas_phy_reset(dev->port->phy, 1);
+ }
}

static void sas_ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index abd2deb..e2df9d0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -444,6 +444,7 @@ struct ata_queued_cmd {
ata_qc_cb_t complete_fn;

void *private_data;
+ void *lldd_task;
};

struct ata_port_stats {

2006-11-17 21:03:29

by djwong

[permalink] [raw]
Subject: [PATCH 11/15] sas_ata: Don't copy aic94xx's sactive to ata_port


Since the aic94xx sequencer assigns its own NCQ tags to ATA commands, it
no longer makes any sense to copy the sactive field in the STP response
to ata_port->sactive, as that will confuse libata. Also, libata seems
to be capable of managing sactive on its own.

The attached patch gets rid of one of the causes of the BUG messages in
ata_qc_new, and seems to work without problems on an IBM x206m.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_ata.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 77860ab..e2650fa 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -105,7 +105,6 @@ static void sas_ata_task_done(struct sas
dev->sata_dev.sstatus = resp->sstatus;
dev->sata_dev.serror = resp->serror;
dev->sata_dev.scontrol = resp->scontrol;
- dev->sata_dev.ap->sactive = resp->sactive;
} else if (stat->stat != SAM_STAT_GOOD) {
ac = sas_to_ata_err(stat);
if (ac) {

2006-11-17 21:02:11

by djwong

[permalink] [raw]
Subject: [PATCH 15/15] sas_ata: Make this a module separate from libsas


Break out sas_ata as a free-standing module that provides a SATA
Translation Layer (SATL) for libsas. This patch requires the libsas
SATL registration patch; the changes to sas_ata itself are rather
minor.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/Makefile | 5 +++--
drivers/scsi/libsas/sas_ata.c | 34 ++++++++++++++++++++++++++++++++--
2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/libsas/Makefile b/drivers/scsi/libsas/Makefile
index 6383eb5..5e95902 100644
--- a/drivers/scsi/libsas/Makefile
+++ b/drivers/scsi/libsas/Makefile
@@ -33,5 +33,6 @@ libsas-y += sas_init.o \
sas_dump.o \
sas_discover.o \
sas_expander.o \
- sas_scsi_host.o \
- sas_ata.o
+ sas_scsi_host.o
+
+obj-$(CONFIG_SCSI_SAS_SATL) += sas_ata.o
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 7338775..bfaee88 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -398,8 +398,8 @@ static struct ata_port_info sata_port_in
.port_ops = &sas_sata_ops
};

-int sas_ata_init_host_and_port(struct domain_device *found_dev,
- struct scsi_target *starget)
+static int sas_ata_init_host_and_port(struct domain_device *found_dev,
+ struct scsi_target *starget)
{
struct Scsi_Host *shost = dev_to_shost(&starget->dev);
struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
@@ -424,3 +424,33 @@ int sas_ata_init_host_and_port(struct do

return 0;
}
+
+/* Module initialization */
+static struct satl_operations sas_ata_ops = {
+ .owner = THIS_MODULE,
+ .init_target = sas_ata_init_host_and_port,
+ .queuecommand = ata_sas_queuecmd,
+ .ioctl = ata_scsi_ioctl,
+ .configure_port = ata_sas_slave_configure,
+ .deactivate_port = ata_port_disable,
+ .destroy_port = ata_sas_port_destroy,
+ .init_port = ata_sas_port_init
+};
+
+static int __init sas_ata_init(void)
+{
+ return sas_register_satl(&sas_ata_ops);
+}
+
+static void __exit sas_ata_exit(void)
+{
+ sas_unregister_satl(&sas_ata_ops);
+}
+
+module_init(sas_ata_init);
+module_exit(sas_ata_exit);
+
+MODULE_AUTHOR("Darrick Wong <[email protected]>");
+MODULE_DESCRIPTION("libata SATL for SAS");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");

2006-11-17 21:03:30

by djwong

[permalink] [raw]
Subject: [PATCH 10/15] aic94xx: Don't call pci_map_sg for already-mapped scatterlists


It turns out that libata has already dma_map_sg'd the scatterlist
entries that go with an ata_queued_cmd by the time it calls
sas_ata_qc_issue. sas_ata_qc_issue passes this scatterlist to aic94xx.
Unfortunately, aic94xx assumes that any scatterlist passed to it needs
to be pci_map_sg'd... which blows away the mapping that libata created!
This causes (on a x260) Calgary IOMMU table leaks and duplicate frees
when aic94xx and libata try to {pci,dma}_unmap_sg the scatterlist.

Since dma_map_sg and pci_map_sg are fed the same struct device, I think
it's safe to add a flag to sas_task that tells aic94xx that it need
not map the scatterlist. It didn't break anything on the x260, though
I don't have any SATAPI devices handy for testing.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/aic94xx/aic94xx_task.c | 17 +++++++++++------
drivers/scsi/libsas/sas_ata.c | 1 +
include/scsi/libsas.h | 1 +
3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
index 466b492..f801c64 100644
--- a/drivers/scsi/aic94xx/aic94xx_task.c
+++ b/drivers/scsi/aic94xx/aic94xx_task.c
@@ -74,8 +74,11 @@ static inline int asd_map_scatterlist(st
return 0;
}

- num_sg = pci_map_sg(asd_ha->pcidev, task->scatter, task->num_scatter,
- task->data_dir);
+ if (task->external_sg)
+ num_sg = task->num_scatter;
+ else
+ num_sg = pci_map_sg(asd_ha->pcidev, task->scatter,
+ task->num_scatter, task->data_dir);
if (num_sg == 0)
return -ENOMEM;

@@ -120,8 +123,9 @@ static inline int asd_map_scatterlist(st

return 0;
err_unmap:
- pci_unmap_sg(asd_ha->pcidev, task->scatter, task->num_scatter,
- task->data_dir);
+ if (!task->external_sg)
+ pci_unmap_sg(asd_ha->pcidev, task->scatter, task->num_scatter,
+ task->data_dir);
return res;
}

@@ -142,8 +146,9 @@ static inline void asd_unmap_scatterlist
}

asd_free_coherent(asd_ha, ascb->sg_arr);
- pci_unmap_sg(asd_ha->pcidev, task->scatter, task->num_scatter,
- task->data_dir);
+ if (!task->external_sg)
+ pci_unmap_sg(asd_ha->pcidev, task->scatter, task->num_scatter,
+ task->data_dir);
}

/* ---------- Task complete tasklet ---------- */
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 209f402..77860ab 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -169,6 +169,7 @@ static unsigned int sas_ata_qc_issue(str
task->num_scatter = num;
}

+ task->external_sg = 1;
task->data_dir = qc->dma_dir;
task->scatter = qc->__sg;
task->ata_task.retry_count = 1;
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index a06cbde..7dcf593 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -538,6 +538,7 @@ struct sas_task {
void *uldd_task;

struct work_struct abort_work;
+ int external_sg;
};


2006-11-17 21:03:25

by djwong

[permalink] [raw]
Subject: [PATCH 07/15] sas_ata: Satisfy libata qc function locking requirements


ata_qc_complete and ata_sas_queuecmd require that the port lock be held
when they are called. sas_ata doesn't do this, leading to BUG messages
about qc tags newly allocated qc tags already being in use. This patch
fixes the locking, which should clean up the rest of those messages.

So far I've tested this against an IBM x206m with two SATA disks with no
BUG messages and no other signs of things going wrong, and the machine
finally passed the pounder stress test.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/sas_ata.c | 4 ++++
drivers/scsi/libsas/sas_scsi_host.c | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index f92f035..32f2b66 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -92,6 +92,7 @@ static void sas_ata_task_done(struct sas
struct task_status_struct *stat = &task->task_status;
struct ata_task_resp *resp = (struct ata_task_resp *)stat->buf;
enum ata_completion_errors ac;
+ unsigned long flags;

if (stat->stat == SAS_PROTO_RESPONSE) {
ata_tf_from_fis(resp->ending_fis, &dev->sata_dev.tf);
@@ -112,7 +113,10 @@ static void sas_ata_task_done(struct sas
}
}

+ spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
ata_qc_complete(qc);
+ spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
+
list_del_init(&task->list);
sas_free_task(task);
}
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index a88d3a4..25639b5 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -212,8 +212,12 @@ int sas_queuecommand(struct scsi_cmnd *c
struct sas_task *task;

if (dev_is_sata(dev)) {
+ unsigned long flags;
+
+ spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
res = ata_sas_queuecmd(cmd, scsi_done,
dev->sata_dev.ap);
+ spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
goto out;
}

2006-11-17 21:04:09

by djwong

[permalink] [raw]
Subject: [PATCH 06/15] sas_ata: Require CONFIG_ATA in Kconfig




Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/libsas/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index aafdc92..b64e391 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -24,7 +24,7 @@ #

config SCSI_SAS_LIBSAS
tristate "SAS Domain Transport Attributes"
- depends on SCSI
+ depends on SCSI && ATA
select SCSI_SAS_ATTRS
help
This provides transport specific helpers for SAS drivers which

2006-11-21 01:19:32

by djwong

[permalink] [raw]
Subject: Re: [PATCH 00/15] Roll-up of my libsas, aic94xx and sas_ata patches

Darrick J. Wong wrote:
> Hi all,
>
> This is a roll-up of all of my patches against libsas, aic94xx and
> sas_ata to date. The only new patches are #4-5 and #12-15; everything
> else has already been seen in some form on this mailing list.
> Hopefully this will make things a bit saner since most of these have
> been floating out over the past 3 weeks. :)
>
> (Apologies for any stgit mail misconfiguration on my part.)

I've respun the non sas_ata patches from last week against scsi-misc:
http://sweaglesw.net/~djwong/docs/libsas-patches/ . The *jejb*patch
files are pulled from aic94xx-sas, so I suppose it's not quite a pure
rebasing; however, some of those patches don't really make any sense
without those three included.

--D

2006-11-25 06:58:53

by djwong

[permalink] [raw]
Subject: [PATCH v2] libsas: Don't give scsi_cmnds to the EH if they never made it to the SAS LLDD or have already returned

On a system with many SAS targets, it appears possible that a scsi_cmnd
can time out without ever making it to the SAS LLDD or at the same time
that a completion is occurring. In both of these cases, telling the
LLDD to abort the sas_task makes no sense because the LLDD won't know
about the sas_task; what we really want to do is to increase the timer.
Note that this involves creating another sas_task bit to indicate
whether or not the task has been sent to the LLDD; I could have
implemented this by slightly redefining SAS_TASK_STATE_PENDING, but
this way seems cleaner.

This second version amends the aic94xx portion to set the
TASK_AT_INITIATOR flag for all sas_tasks that were passed to
lldd_execute_task.

Signed-off-by: Darrick J. Wong <[email protected]>
---

drivers/scsi/aic94xx/aic94xx_task.c | 9 +++++++++
drivers/scsi/aic94xx/aic94xx_tmf.c | 4 +++-
drivers/scsi/libsas/sas_scsi_host.c | 7 +++++++
include/scsi/libsas.h | 1 +
4 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
index d202ed5..e2ad5be 100644
--- a/drivers/scsi/aic94xx/aic94xx_task.c
+++ b/drivers/scsi/aic94xx/aic94xx_task.c
@@ -349,6 +349,7 @@ Again:

spin_lock_irqsave(&task->task_state_lock, flags);
task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
+ task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
task->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((task->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&task->task_state_lock, flags);
@@ -557,6 +558,7 @@ int asd_execute_task(struct sas_task *ta
struct sas_task *t = task;
struct asd_ascb *ascb = NULL, *a;
struct asd_ha_struct *asd_ha = task->dev->port->ha->lldd_ha;
+ unsigned long flags;

res = asd_can_queue(asd_ha, num);
if (res)
@@ -599,6 +601,10 @@ int asd_execute_task(struct sas_task *ta
}
if (res)
goto out_err_unmap;
+
+ spin_lock_irqsave(&t->task_state_lock, flags);
+ t->task_state_flags |= SAS_TASK_AT_INITIATOR;
+ spin_unlock_irqrestore(&t->task_state_lock, flags);
}
list_del_init(&alist);

@@ -617,6 +623,9 @@ out_err_unmap:
if (a == b)
break;
t = a->uldd_task;
+ spin_lock_irqsave(&t->task_state_lock, flags);
+ t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
+ spin_unlock_irqrestore(&t->task_state_lock, flags);
switch (t->task_proto) {
case SATA_PROTO:
case SAS_PROTO_STP:
diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c b/drivers/scsi/aic94xx/aic94xx_tmf.c
index 6123438..686cea1 100644
--- a/drivers/scsi/aic94xx/aic94xx_tmf.c
+++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
@@ -345,7 +345,7 @@ static inline int asd_clear_nexus(struct
int asd_abort_task(struct sas_task *task)
{
struct asd_ascb *tascb = task->lldd_task;
- struct asd_ha_struct *asd_ha = tascb->ha;
+ struct asd_ha_struct *asd_ha;
int res = 1;
unsigned long flags;
struct asd_ascb *ascb = NULL;
@@ -360,6 +360,8 @@ int asd_abort_task(struct sas_task *task
}
spin_unlock_irqrestore(&task->task_state_lock, flags);

+ asd_ha = tascb->ha;
+
ascb = asd_ascb_alloc_list(asd_ha, &res, GFP_KERNEL);
if (!ascb)
return -ENOMEM;
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index e064aac..c3fc8d6 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -542,6 +542,13 @@ enum scsi_eh_timer_return sas_scsi_timed
cmd, task);
return 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: "
+ "EH_RESET_TIMER\n",
+ cmd, task);
+ return EH_RESET_TIMER;
+ }
task->task_state_flags |= SAS_TASK_STATE_ABORTED;
spin_unlock_irqrestore(&task->task_state_lock, flags);

diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index c4a8af1..33b72ae 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -538,6 +538,7 @@ #define SAS_TASK_STATE_PENDING 1
#define SAS_TASK_STATE_DONE 2
#define SAS_TASK_STATE_ABORTED 4
#define SAS_TASK_INITIATOR_ABORTED 8
+#define SAS_TASK_AT_INITIATOR 16

static inline struct sas_task *sas_alloc_task(gfp_t flags)
{

2006-11-25 19:03:25

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v2] libsas: Don't give scsi_cmnds to the EH if they never made it to the SAS LLDD or have already returned

On Fri, 2006-11-24 at 22:58 -0800, Darrick J. Wong wrote:
> On a system with many SAS targets, it appears possible that a scsi_cmnd
> can time out without ever making it to the SAS LLDD or at the same time
> that a completion is occurring. In both of these cases, telling the
> LLDD to abort the sas_task makes no sense because the LLDD won't know
> about the sas_task; what we really want to do is to increase the timer.
> Note that this involves creating another sas_task bit to indicate
> whether or not the task has been sent to the LLDD; I could have
> implemented this by slightly redefining SAS_TASK_STATE_PENDING, but
> this way seems cleaner.
>
> This second version amends the aic94xx portion to set the
> TASK_AT_INITIATOR flag for all sas_tasks that were passed to
> lldd_execute_task.

Actually, this patch causes my ATAPI device not to appear initially. It
looks like the libata IDENTIFY is failing. It seems to be a problem
with the initial reset the device needs to get the D2H FIS. This is
what I see:

sas: sas_ata_phy_reset: Found ATAPI device.
ata1.00: ATAPI, max UDMA/66
aic94xx: escb_tasklet_complete: phy5: BYTES_DMAED
aic94xx: STP proto device-to-host FIS:
aic94xx: 00: 34 00 50 01
aic94xx: 04: 01 00 00 00
aic94xx: 08: 01 00 00 00
aic94xx: 0c: 01 00 00 00
aic94xx: 10: 00 00 00 00
aic94xx: asd_form_port: updating phy_mask 0x20 for phy5
ata1.00: qc timeout (cmd 0xa1)
sas: sas_ata_post_internal: Failure; reset phy!
ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
ata1.00: revalidation failed (errno=-5)
sas: STUB sas_ata_scr_read
ata1.00: limiting speed to UDMA/44
sas: sas_ata_phy_reset: Found ATAPI device.
ata1.00: qc timeout (cmd 0xa1)
sas: sas_ata_post_internal: Failure; reset phy!
ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
sas: STUB sas_ata_scr_read
sas: sas_ata_phy_reset: Found ATAPI device.


James