[SCSI] libsas: remove ata_port.lock management duties from lldds

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.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 751368b46b440fa1de671a03f943060fe767e4b2..788daeedc89f64954f95639b423c81b284ea6813 100644 (file)
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -3796,8 +3796,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 f5a3f7d2bdab29059af48d4b333d39b91c57d3d0..4bd88ef83cdf19ed9aba3b492a880bc14889206a 100644 (file)
--- 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 1b27b3797c6c9cce8c1160c11d43240ccc4a52fe..bb472c339523c124f83ca7f5747d9bca788fdd61 100644 (file)
--- a/drivers/scsi/isci/task.h
+++ b/drivers/scsi/isci/task.h
@@ -321,40 +321,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 83118d0b6d0c55faaf0be170ce14e64e42d65814..81ce39d166d1bdb56d84abba2b8c6df525ef4842 100644 (file)
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -166,23 +166,30 @@ static void sas_ata_task_done(struct sas_task *task)
 
 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 2a163c73fd8b6f1930e9853ba72c0e71b322be34..fd60465d4b2d4abbe60e2b2a042e460a54c9b578 100644 (file)
--- 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 b118e632bc7d931782f6528172a14e5e22e69e38..cd882230591f1bb8075aaf0b6d8988225e5bcb38 100644 (file)
--- 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 50837933a1e5822dc7416cbec25328afdd185a7e..310860e37d98f7d6cf4862ac9fdde78e1441229b 100644 (file)
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -364,7 +364,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;
@@ -388,11 +388,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,