Patchwork [isci,v3,1/4] libata: export ata_port suspend/resume infrastructure for sas

login
register
mail settings
Submitter Dan Williams
Date March 14, 2012, 7:13 a.m.
Message ID <20120314071353.7223.5361.stgit@dwillia2-linux.jf.intel.com>
Download mbox | patch
Permalink /patch/146562/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Dan Williams - March 14, 2012, 7:13 a.m.
Reuse ata_port_{suspend|resume}_common for sas.  This path is chosen
over adding coordination between ata-tranport and sas-transport because
libsas wants to revalidate the domain at resume-time at the host level.
It can not validate links have resumed properly until libata has had a
chance to perform its revalidation, and any sane placing of an ata_port
in the sas-transport model would delay it's resumption until after the
host.

Export the common portion of port suspend/resume (bypass pm_runtime),
and allow sas to perform these operations asynchronously (similar to the
libsas async-ata probe implmentation).  Async operation is determined by
having an external, rather than stack based, location for storing the
result of the operation.

Reviewed-by: Jacek Danecki <jacek.danecki@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/ata/libata-core.c |   58 ++++++++++++++++++++++++++++++++++++---------
 include/linux/libata.h    |   11 +++++++++
 2 files changed, 57 insertions(+), 12 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Garzik - April 21, 2012, 6:26 a.m.
On 03/14/2012 03:13 AM, Dan Williams wrote:
> Reuse ata_port_{suspend|resume}_common for sas.  This path is chosen
> over adding coordination between ata-tranport and sas-transport because
> libsas wants to revalidate the domain at resume-time at the host level.
> It can not validate links have resumed properly until libata has had a
> chance to perform its revalidation, and any sane placing of an ata_port
> in the sas-transport model would delay it's resumption until after the
> host.
>
> Export the common portion of port suspend/resume (bypass pm_runtime),
> and allow sas to perform these operations asynchronously (similar to the
> libsas async-ata probe implmentation).  Async operation is determined by
> having an external, rather than stack based, location for storing the
> result of the operation.
>
> Reviewed-by: Jacek Danecki<jacek.danecki@intel.com>
> Signed-off-by: Dan Williams<dan.j.williams@intel.com>
> ---
>   drivers/ata/libata-core.c |   58 ++++++++++++++++++++++++++++++++++++---------
>   include/linux/libata.h    |   11 +++++++++
>   2 files changed, 57 insertions(+), 12 deletions(-)

Now that libata's runtime PM problems seem to be fixed for the moment, 
we can revisit port PM here.  Just checking...  Is this patch still needed?



--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams - April 22, 2012, 1:06 a.m.
On Fri, Apr 20, 2012 at 11:26 PM, Jeff Garzik <jeff@garzik.org> wrote:
> On 03/14/2012 03:13 AM, Dan Williams wrote:
>>
>> Reuse ata_port_{suspend|resume}_common for sas.  This path is chosen
>> over adding coordination between ata-tranport and sas-transport because
>> libsas wants to revalidate the domain at resume-time at the host level.
>> It can not validate links have resumed properly until libata has had a
>> chance to perform its revalidation, and any sane placing of an ata_port
>> in the sas-transport model would delay it's resumption until after the
>> host.
>>
>> Export the common portion of port suspend/resume (bypass pm_runtime),
>> and allow sas to perform these operations asynchronously (similar to the
>> libsas async-ata probe implmentation).  Async operation is determined by
>> having an external, rather than stack based, location for storing the
>> result of the operation.
>>
>> Reviewed-by: Jacek Danecki<jacek.danecki@intel.com>
>> Signed-off-by: Dan Williams<dan.j.williams@intel.com>
>> ---
>>  drivers/ata/libata-core.c |   58
>> ++++++++++++++++++++++++++++++++++++---------
>>  include/linux/libata.h    |   11 +++++++++
>>  2 files changed, 57 insertions(+), 12 deletions(-)
>
>
> Now that libata's runtime PM problems seem to be fixed for the moment, we
> can revisit port PM here.  Just checking...  Is this patch still needed?

Yeah, this one is still needed.  libsas wants to validate links at the
host level.  I tried letting these routines be called "naturally" by
registering libsas-ata_ports with the ata-transport class, but it
hangs / got messy in cases where the link needed recovery
(SATA_PENDING), but the ata_port was still suspended.  Also, I think
the ata_port pm_runtime bits need a review for suitability for the
libsas case.

So, with this approach libsas does not register libsas-ata_ports with
the ata transport class and instead calls the core routines directly
(similar to how libata did before the dev_pm_ops rework).

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bd79c8b..a13c36d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5241,16 +5241,20 @@  bool ata_link_offline(struct ata_link *link)
 #ifdef CONFIG_PM
 static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
 			       unsigned int action, unsigned int ehi_flags,
-			       int wait)
+			       int *async)
 {
 	struct ata_link *link;
 	unsigned long flags;
-	int rc;
+	int rc = 0;
 
 	/* Previous resume operation might still be in
 	 * progress.  Wait for PM_PENDING to clear.
 	 */
 	if (ap->pflags & ATA_PFLAG_PM_PENDING) {
+		if (async) {
+			*async = -EAGAIN;
+			return 0;
+		}
 		ata_port_wait_eh(ap);
 		WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING);
 	}
@@ -5259,10 +5263,10 @@  static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
 	spin_lock_irqsave(ap->lock, flags);
 
 	ap->pm_mesg = mesg;
-	if (wait) {
-		rc = 0;
+	if (async)
+		ap->pm_result = async;
+	else
 		ap->pm_result = &rc;
-	}
 
 	ap->pflags |= ATA_PFLAG_PM_PENDING;
 	ata_for_each_link(link, ap, HOST_FIRST) {
@@ -5275,7 +5279,7 @@  static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
 	spin_unlock_irqrestore(ap->lock, flags);
 
 	/* wait and check result */
-	if (wait) {
+	if (!async) {
 		ata_port_wait_eh(ap);
 		WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING);
 	}
@@ -5285,9 +5289,8 @@  static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
 
 #define to_ata_port(d) container_of(d, struct ata_port, tdev)
 
-static int ata_port_suspend_common(struct device *dev, pm_message_t mesg)
+static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int *async)
 {
-	struct ata_port *ap = to_ata_port(dev);
 	unsigned int ehi_flags = ATA_EHI_QUIET;
 	int rc;
 
@@ -5302,10 +5305,17 @@  static int ata_port_suspend_common(struct device *dev, pm_message_t mesg)
 	if (mesg.event == PM_EVENT_SUSPEND)
 		ehi_flags |= ATA_EHI_NO_AUTOPSY | ATA_EHI_NO_RECOVERY;
 
-	rc = ata_port_request_pm(ap, mesg, 0, ehi_flags, 1);
+	rc = ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
 	return rc;
 }
 
+static int ata_port_suspend_common(struct device *dev, pm_message_t mesg)
+{
+	struct ata_port *ap = to_ata_port(dev);
+
+	return __ata_port_suspend_common(ap, mesg, NULL);
+}
+
 static int ata_port_suspend(struct device *dev)
 {
 	if (pm_runtime_suspended(dev))
@@ -5330,16 +5340,22 @@  static int ata_port_poweroff(struct device *dev)
 	return ata_port_suspend_common(dev, PMSG_HIBERNATE);
 }
 
-static int ata_port_resume_common(struct device *dev)
+static int __ata_port_resume_common(struct ata_port *ap, int *async)
 {
-	struct ata_port *ap = to_ata_port(dev);
 	int rc;
 
 	rc = ata_port_request_pm(ap, PMSG_ON, ATA_EH_RESET,
-		ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 1);
+		ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, async);
 	return rc;
 }
 
+static int ata_port_resume_common(struct device *dev)
+{
+	struct ata_port *ap = to_ata_port(dev);
+
+	return __ata_port_resume_common(ap, NULL);
+}
+
 static int ata_port_resume(struct device *dev)
 {
 	int rc;
@@ -5372,6 +5388,24 @@  static const struct dev_pm_ops ata_port_pm_ops = {
 	.runtime_idle = ata_port_runtime_idle,
 };
 
+/* sas ports don't participate in pm runtime management of ata_ports,
+ * and need to resume ata devices at the domain level, not the per-port
+ * level. sas suspend/resume is async to allow parallel port recovery
+ * since sas has multiple ata_port instances per Scsi_Host.
+ */
+int ata_sas_port_async_suspend(struct ata_port *ap, int *async)
+{
+	return __ata_port_suspend_common(ap, PMSG_SUSPEND, async);
+}
+EXPORT_SYMBOL_GPL(ata_sas_port_async_suspend);
+
+int ata_sas_port_async_resume(struct ata_port *ap, int *async)
+{
+	return __ata_port_resume_common(ap, async);
+}
+EXPORT_SYMBOL_GPL(ata_sas_port_async_resume);
+
+
 /**
  *	ata_host_suspend - suspend host
  *	@host: host to suspend
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 83ff256..e2da9c0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1013,6 +1013,17 @@  extern bool ata_link_offline(struct ata_link *link);
 #ifdef CONFIG_PM
 extern int ata_host_suspend(struct ata_host *host, pm_message_t mesg);
 extern void ata_host_resume(struct ata_host *host);
+extern int ata_sas_port_async_suspend(struct ata_port *ap, int *async);
+extern int ata_sas_port_async_resume(struct ata_port *ap, int *async);
+#else
+static inline int ata_sas_port_async_suspend(struct ata_port *ap, int *async)
+{
+	return 0;
+}
+static inline int ata_sas_port_async_resume(struct ata_port *ap, int *async)
+{
+	return 0;
+}
 #endif
 extern int ata_ratelimit(void);
 extern void ata_msleep(struct ata_port *ap, unsigned int msecs);