From: Dan Williams <dan.j.williams@intel.com>
To: linux-scsi@vger.kernel.org
Cc: Xiangliang Yu <yuxiangl@marvell.com>,
linux-ide@vger.kernel.org, Jack Wang <jack_wang@usish.com>
Subject: [PATCH 09/24] libsas: remove ata_port.lock management duties from lldds
Date: Fri, 16 Dec 2011 18:33:54 -0800 [thread overview]
Message-ID: <20111217023354.15036.93552.stgit@localhost6.localdomain6> (raw)
In-Reply-To: <20111217022912.15036.85808.stgit@localhost6.localdomain6>
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@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 | 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,
next prev parent reply other threads:[~2011-12-17 2:33 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-17 2:33 [PATCH 00/24] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
2011-12-17 2:33 ` [PATCH 01/24] workqueue: promote workqueue_lock to hard-irq safe Dan Williams
2011-12-17 2:33 ` [PATCH 02/24] workqueue: defer work to a draining queue Dan Williams
2011-12-19 21:49 ` Tejun Heo
2011-12-19 22:01 ` Dan Williams
2011-12-19 22:08 ` Tejun Heo
2011-12-19 22:25 ` Williams, Dan J
2011-12-17 2:33 ` [PATCH 03/24] scsi: use drain_workqueue Dan Williams
2011-12-17 2:33 ` [PATCH 04/24] libsas: remove unused ata_task_resp fields Dan Williams
2011-12-17 12:51 ` Sergei Shtylyov
2011-12-19 1:38 ` Jack Wang
2011-12-17 2:33 ` [PATCH 05/24] libsas: kill sas_slave_destroy Dan Williams
2011-12-17 2:33 ` [PATCH 06/24] libsas: fix domain_device leak Dan Williams
2011-12-19 2:32 ` Jack Wang
2011-12-17 2:33 ` [PATCH 07/24] libsas: fix leak of dev->sata_dev.identify_[packet_]device Dan Williams
2011-12-19 2:38 ` Jack Wang
2011-12-17 2:33 ` [PATCH 08/24] libsas: replace event locks with atomic bitops Dan Williams
2011-12-17 2:33 ` Dan Williams [this message]
2011-12-17 2:33 ` [PATCH 10/24] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
2011-12-17 2:34 ` [PATCH 11/24] libsas: use ->set_dmamode to notify lldds of NCQ parameters Dan Williams
2011-12-17 2:34 ` [PATCH 12/24] libsas: kill invocation of scsi_eh_finish_cmd from sas_ata_task_done Dan Williams
2011-12-17 2:34 ` [PATCH 13/24] libsas: close error handling vs sas_ata_task_done() race Dan Williams
2011-12-17 2:34 ` [PATCH 14/24] libsas: prevent double completion of scmds from eh Dan Williams
2011-12-17 13:08 ` Sergei Shtylyov
2011-12-18 19:19 ` Dan Williams
2011-12-17 13:13 ` Sergei Shtylyov
2011-12-18 19:24 ` Dan Williams
2011-12-17 2:34 ` [PATCH 15/24] libsas: fix timeout vs completion race Dan Williams
2011-12-17 2:34 ` [PATCH 16/24] libsas: let libata handle command timeouts Dan Williams
2011-12-17 2:34 ` [PATCH 17/24] libsas: defer SAS_TASK_NEED_DEV_RESET commands to libata Dan Williams
2011-12-17 2:34 ` [PATCH 18/24] libsas: use libata-eh-reset for sata rediscovery fis transmit failures Dan Williams
2011-12-17 2:34 ` [PATCH 19/24] libsas: execute transport link resets with libata-eh via host workqueue Dan Williams
2011-12-17 2:34 ` [PATCH 20/24] libsas: sas_phy_enable via transport_sas_phy_reset Dan Williams
2011-12-17 2:34 ` [PATCH 21/24] libsas: Remove redundant phy state notification calls Dan Williams
2011-12-17 2:35 ` [PATCH 22/24] libsas: add mutex for SMP task execution Dan Williams
2011-12-17 2:35 ` [PATCH 23/24] libsas: async ata-eh Dan Williams
2011-12-17 2:35 ` [PATCH 24/24] libsas: poll for ata device readiness after reset Dan Williams
2011-12-20 6:18 ` Jack Wang
2011-12-20 7:08 ` Williams, Dan J
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111217023354.15036.93552.stgit@localhost6.localdomain6 \
--to=dan.j.williams@intel.com \
--cc=jack_wang@usish.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=yuxiangl@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox