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. Assumes ->queuecommand() is always called with irqs enabled which was the assumption mvsas was making prior to the conversion. Cc: Jack Wang <jack_wang@xxxxxxxxx> Cc: Xiangliang Yu <yuxiangl@xxxxxxxxxxx> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- drivers/scsi/isci/request.c | 3 +-- drivers/scsi/isci/task.c | 6 ++---- drivers/scsi/isci/task.h | 36 ----------------------------------- drivers/scsi/libsas/sas_ata.c | 31 ++++++++++++++++++------------ 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, 24 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..0489001 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -166,23 +166,26 @@ qc_already_gone: static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc) { - int res; + unsigned int si; + unsigned int xfer = 0; struct sas_task *task; - struct domain_device *dev = qc->ap->private_data; + struct scatterlist *sg; + int ret = AC_ERR_SYSTEM; + 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; + + spin_unlock_irq(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 +230,23 @@ 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_irq(ap->lock); + 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 4636453..8c91afa 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 a4884a5..911fc42 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 fb3dc99..0ba1f723 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -340,7 +340,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; @@ -364,11 +364,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, -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html