Linux ATA/IDE development
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: tj@kernel.org, JBottomley@Parallels.com
Cc: Len Brown <len.brown@intel.com>,
	linux-scsi@vger.kernel.org, Phillip Susi <psusi@ubuntu.com>,
	linux-ide@vger.kernel.org, Alan Stern <stern@rowland.harvard.edu>,
	Todd Brandt <todd.e.brandt@linux.intel.com>
Subject: [PATCH v5 3/3] scsi: async sd resume
Date: Wed, 05 Mar 2014 12:17:40 -0800	[thread overview]
Message-ID: <20140305201740.20088.85890.stgit@viggo.jf.intel.com> (raw)
In-Reply-To: <20140305201443.20088.85361.stgit@viggo.jf.intel.com>

async_schedule() sd resume work to allow disks and other devices to
resume in parallel.

This moves the entirety of scsi_device resume to an async context to
ensure that scsi_device_resume() remains ordered with respect to the
completion of the start/stop command.  For the duration of the resume,
new command submissions (that do not originate from the scsi-core) will
be deferred (BLKPREP_DEFER).

It adds a new ASYNC_DOMAIN_EXCLUSIVE(scsi_sd_pm_domain) as a container
of these operations.  Like scsi_sd_probe_domain it is flushed at
sd_remove() time to ensure async ops do not continue past the
end-of-life of the sdev.  The implementation explicitly refrains from
reusing scsi_sd_probe_domain directly for this purpose as it is flushed
at the end of dpm_resume(), potentially defeating some of the benefit.
Given sdevs are quiesced it is permissible for these resume operations
to bleed past the async_synchronize_full() calls made by the driver
core.

We defer the resolution of which pm callback to call until
scsi_dev_type_{suspend|resume} time and guarantee that the 'int
(*cb)(struct device *)' parameter is never NULL.  With this in place the
type of resume operation is encoded in the async function identifier.

Inspired by Todd's analysis and initial proposal [2]:
https://01.org/suspendresume/blogs/tebrandt/2013/hard-disk-resume-optimization-simpler-approach

Cc: Len Brown <len.brown@intel.com>
Cc: Phillip Susi <psusi@ubuntu.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Suggested-by: Todd Brandt <todd.e.brandt@linux.intel.com>
[djbw: kick all resume work to the async queue]
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/scsi.c      |    3 +
 drivers/scsi/scsi_pm.c   |  115 ++++++++++++++++++++++++++++++++++++----------
 drivers/scsi/scsi_priv.h |    1 
 drivers/scsi/sd.c        |    1 
 4 files changed, 95 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d8afec8317cf..990d4a302788 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -91,6 +91,9 @@ EXPORT_SYMBOL(scsi_logging_level);
 ASYNC_DOMAIN(scsi_sd_probe_domain);
 EXPORT_SYMBOL(scsi_sd_probe_domain);
 
+ASYNC_DOMAIN_EXCLUSIVE(scsi_sd_pm_domain);
+EXPORT_SYMBOL(scsi_sd_pm_domain);
+
 /* NB: These are exposed through /proc/scsi/scsi and form part of the ABI.
  * You may not alter any existing entry (although adding new ones is
  * encouraged once assigned by ANSI/INCITS T10
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 001e9ceda4c3..1cb211bf0383 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -18,17 +18,52 @@
 
 #ifdef CONFIG_PM_SLEEP
 
+#define do_pm_op(dev, op) \
+	dev->driver ? dev->driver->pm ? \
+	dev->driver->pm->op(dev) : 0 : 0
+
+static int do_scsi_suspend(struct device *dev)
+{
+	return do_pm_op(dev, suspend);
+}
+
+static int do_scsi_freeze(struct device *dev)
+{
+	return do_pm_op(dev, freeze);
+}
+
+static int do_scsi_poweroff(struct device *dev)
+{
+	return do_pm_op(dev, poweroff);
+}
+
+static int do_scsi_resume(struct device *dev)
+{
+	return do_pm_op(dev, resume);
+}
+
+static int do_scsi_thaw(struct device *dev)
+{
+	return do_pm_op(dev, thaw);
+}
+
+static int do_scsi_restore(struct device *dev)
+{
+	return do_pm_op(dev, restore);
+}
+
 static int scsi_dev_type_suspend(struct device *dev, int (*cb)(struct device *))
 {
 	int err;
 
+	/* flush pending in-flight resume operations, suspend is synchronous */
+	async_synchronize_full_domain(&scsi_sd_pm_domain);
+
 	err = scsi_device_quiesce(to_scsi_device(dev));
 	if (err == 0) {
-		if (cb) {
-			err = cb(dev);
-			if (err)
-				scsi_device_resume(to_scsi_device(dev));
-		}
+		err = cb(dev);
+		if (err)
+			scsi_device_resume(to_scsi_device(dev));
 	}
 	dev_dbg(dev, "scsi suspend: %d\n", err);
 	return err;
@@ -38,10 +73,16 @@ static int scsi_dev_type_resume(struct device *dev, int (*cb)(struct device *))
 {
 	int err = 0;
 
-	if (cb)
-		err = cb(dev);
+	err = cb(dev);
 	scsi_device_resume(to_scsi_device(dev));
 	dev_dbg(dev, "scsi resume: %d\n", err);
+
+	if (err == 0) {
+		pm_runtime_disable(dev);
+		pm_runtime_set_active(dev);
+		pm_runtime_enable(dev);
+	}
+
 	return err;
 }
 
@@ -66,20 +107,50 @@ scsi_bus_suspend_common(struct device *dev, int (*cb)(struct device *))
 	return err;
 }
 
+static void async_sdev_resume(void *dev, async_cookie_t cookie)
+{
+	scsi_dev_type_resume(dev, do_scsi_resume);
+}
+
+static void async_sdev_thaw(void *dev, async_cookie_t cookie)
+{
+	scsi_dev_type_resume(dev, do_scsi_thaw);
+}
+
+static void async_sdev_restore(void *dev, async_cookie_t cookie)
+{
+	scsi_dev_type_resume(dev, do_scsi_restore);
+}
+
+static async_func_t to_async_sdev_resume_fn(struct device *dev,
+		int (*cb)(struct device *))
+{
+	if (!scsi_is_sdev_device(dev))
+		return NULL;
+
+	if (cb == do_scsi_resume)
+		return async_sdev_resume;
+	else if (cb == do_scsi_thaw)
+		return async_sdev_thaw;
+	else if (cb == do_scsi_restore)
+		return async_sdev_restore;
+	else
+		return NULL;
+}
+
 static int
 scsi_bus_resume_common(struct device *dev, int (*cb)(struct device *))
 {
-	int err = 0;
-
-	if (scsi_is_sdev_device(dev))
-		err = scsi_dev_type_resume(dev, cb);
+	async_func_t fn = to_async_sdev_resume_fn(dev, cb);
 
-	if (err == 0) {
+	if (fn)
+		async_schedule_domain(fn, dev, &scsi_sd_pm_domain);
+	else {
 		pm_runtime_disable(dev);
 		pm_runtime_set_active(dev);
 		pm_runtime_enable(dev);
 	}
-	return err;
+	return 0;
 }
 
 static int scsi_bus_prepare(struct device *dev)
@@ -97,38 +168,32 @@ static int scsi_bus_prepare(struct device *dev)
 
 static int scsi_bus_suspend(struct device *dev)
 {
-	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	return scsi_bus_suspend_common(dev, pm ? pm->suspend : NULL);
+	return scsi_bus_suspend_common(dev, do_scsi_suspend);
 }
 
 static int scsi_bus_resume(struct device *dev)
 {
-	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	return scsi_bus_resume_common(dev, pm ? pm->resume : NULL);
+	return scsi_bus_resume_common(dev, do_scsi_resume);
 }
 
 static int scsi_bus_freeze(struct device *dev)
 {
-	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	return scsi_bus_suspend_common(dev, pm ? pm->freeze : NULL);
+	return scsi_bus_suspend_common(dev, do_scsi_freeze);
 }
 
 static int scsi_bus_thaw(struct device *dev)
 {
-	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	return scsi_bus_resume_common(dev, pm ? pm->thaw : NULL);
+	return scsi_bus_resume_common(dev, do_scsi_thaw);
 }
 
 static int scsi_bus_poweroff(struct device *dev)
 {
-	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	return scsi_bus_suspend_common(dev, pm ? pm->poweroff : NULL);
+	return scsi_bus_suspend_common(dev, do_scsi_poweroff);
 }
 
 static int scsi_bus_restore(struct device *dev)
 {
-	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	return scsi_bus_resume_common(dev, pm ? pm->restore : NULL);
+	return scsi_bus_resume_common(dev, do_scsi_restore);
 }
 
 #else /* CONFIG_PM_SLEEP */
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index f079a598bed4..a26fd44c7738 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -166,6 +166,7 @@ static inline int scsi_autopm_get_host(struct Scsi_Host *h) { return 0; }
 static inline void scsi_autopm_put_host(struct Scsi_Host *h) {}
 #endif /* CONFIG_PM_RUNTIME */
 
+extern struct async_domain scsi_sd_pm_domain;
 extern struct async_domain scsi_sd_probe_domain;
 
 /* 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 470954aba728..700c595c603e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3020,6 +3020,7 @@ static int sd_remove(struct device *dev)
 	devt = disk_devt(sdkp->disk);
 	scsi_autopm_get_device(sdkp->device);
 
+	async_synchronize_full_domain(&scsi_sd_pm_domain);
 	async_synchronize_full_domain(&scsi_sd_probe_domain);
 	blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn);
 	blk_queue_unprep_rq(sdkp->device->request_queue, NULL);


  parent reply	other threads:[~2014-03-05 20:17 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-05 20:17 [PATCH v5 0/3] Accelerate Storage Resume (2x or more) Dan Williams
2014-03-05 20:17 ` [PATCH v5 1/3] libata, libsas: kill pm_result and related cleanup Dan Williams
2014-03-10 20:29   ` Tejun Heo
2014-03-10 20:44     ` Tejun Heo
2014-03-05 20:17 ` [PATCH v5 2/3] libata: async resume Dan Williams
2014-12-19 23:46   ` Phillip Susi
2014-12-20  1:42     ` Phillip Susi
2014-03-05 20:17 ` Dan Williams [this message]
2014-03-07 15:42   ` [PATCH v5 3/3] scsi: async sd resume Alan Stern
2014-03-08  0:41     ` Dan Williams
2014-03-08  2:52     ` Dan Williams
2014-03-10 15:49       ` Alan Stern
2014-03-10 20:43       ` Tejun Heo
2014-03-10 20:56         ` Dan Williams
2014-03-10 20:59           ` Alan Stern
2014-03-10 21:06             ` Dan Williams
2014-03-10 22:41             ` Phillip Susi
2014-03-11  4:19             ` James Bottomley
2014-03-11  4:47               ` Dan Williams
2014-03-11  5:16                 ` James Bottomley
2014-03-11  5:26                   ` Dan Williams
2014-03-11  6:29                     ` James Bottomley
2014-03-14 20:11                       ` Dan Williams
2014-03-15 21:47                         ` James Bottomley
2014-03-15 23:35                           ` Dan Williams
2014-03-16 18:21                             ` James Bottomley
2014-03-17 18:20                               ` Dan Williams
2014-03-21  0:09                                 ` Dan Williams

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=20140305201740.20088.85890.stgit@viggo.jf.intel.com \
    --to=dan.j.williams@intel.com \
    --cc=JBottomley@Parallels.com \
    --cc=len.brown@intel.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=psusi@ubuntu.com \
    --cc=stern@rowland.harvard.edu \
    --cc=tj@kernel.org \
    --cc=todd.e.brandt@linux.intel.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