SSD becomes more and more popular, this makes it possible to put disk into
low power state more often. And request based runtime PM for scsi disk is
more useful than open/close based one because disk is normally mounted at
most time.
One known issue, because SCSI TEST_UNIT_READY will be put into request
queue every 2 seconds by default, this makes it hard for disk to sleep.
Maybe we can implement check_events callback in some other way?
[RFC 1/5] pm, runtime, Add resume notifier
[RFC 2/5] scsi, pm, rename scsi_autopm_get/put_xxx to
[RFC 3/5] scsi, pm, add pm_runtime_get/put in scsi request
[RFC 4/5] scsi, pm, use autosuspend for scsi runtime PM
[RFC 5/5] scsi, sd, pm, request based runtime PM support
Best Regards,
Huang Ying
Asynchronous interface is provided for runtime PM, when the requested
action (suspend/resume) is completed, the caller may need to be
notified.
Signed-off-by: Huang Ying <[email protected]>
---
drivers/base/power/runtime.c | 1 +
include/linux/pm.h | 2 ++
2 files changed, 3 insertions(+)
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -733,6 +733,7 @@ static int rpm_resume(struct device *dev
atomic_inc(&parent->power.child_count);
}
wake_up_all(&dev->power.wait_queue);
+ atomic_notifier_call_chain(&dev->power.notifier, RPM_REQ_RESUME, dev);
if (!retval)
rpm_idle(dev, RPM_ASYNC);
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -27,6 +27,7 @@
#include <linux/wait.h>
#include <linux/timer.h>
#include <linux/completion.h>
+#include <linux/notifier.h>
/*
* Callbacks for platform drivers to implement.
@@ -488,6 +489,7 @@ struct dev_pm_info {
unsigned long timer_expires;
struct work_struct work;
wait_queue_head_t wait_queue;
+ struct atomic_notifier_head notifier;
atomic_t usage_count;
atomic_t child_count;
unsigned int disable_depth:3;
Can reduce power consumption when disk is mounted but no read/write
request.
TODO: Request based runtime PM may be harmful for rotating HD, should
enable that only for SSD and fallback to open/close based runtime PM
otherwise.
Signed-off-by: Huang Ying <[email protected]>
---
drivers/scsi/scsi_lib.c | 3 ++-
drivers/scsi/scsi_priv.h | 1 +
drivers/scsi/sd.c | 43 +++++++++++++++++++++++++++++++++++++++++--
drivers/scsi/sd.h | 2 ++
4 files changed, 46 insertions(+), 3 deletions(-)
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -401,7 +401,7 @@ static inline int scsi_host_is_busy(stru
* Notes: The previous command was completely finished, start
* a new one if possible.
*/
-static void scsi_run_queue(struct request_queue *q)
+void scsi_run_queue(struct request_queue *q)
{
struct scsi_device *sdev = q->queuedata;
struct Scsi_Host *shost;
@@ -454,6 +454,7 @@ static void scsi_run_queue(struct reques
blk_run_queue(q);
}
+EXPORT_SYMBOL_GPL(scsi_run_queue);
void scsi_requeue_run_queue(struct work_struct *work)
{
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -87,6 +87,7 @@ extern struct request_queue *scsi_alloc_
extern void scsi_free_queue(struct request_queue *q);
extern int scsi_init_queue(void);
extern void scsi_exit_queue(void);
+extern void scsi_run_queue(struct request_queue *q);
struct request_queue;
struct request;
extern struct kmem_cache *scsi_sdb_cache;
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -66,6 +66,7 @@
#include "sd.h"
#include "scsi_logging.h"
+#include "scsi_priv.h"
MODULE_AUTHOR("Eric Youngdale");
MODULE_DESCRIPTION("SCSI disk (sd) driver");
@@ -981,6 +982,8 @@ static int sd_open(struct block_device *
scsi_set_medium_removal(sdev, SCSI_REMOVAL_PREVENT);
}
+ scsi_autopm_put_device_sync(sdev);
+
return 0;
error_out:
@@ -1020,7 +1023,6 @@ static int sd_release(struct gendisk *di
* XXX is followed by a "rmmod sd_mod"?
*/
- scsi_autopm_put_device_sync(sdev);
scsi_disk_put(sdkp);
return 0;
}
@@ -1071,7 +1073,7 @@ static int sd_ioctl(struct block_device
struct scsi_device *sdp = sdkp->device;
void __user *p = (void __user *)arg;
int error;
-
+
SCSI_LOG_IOCTL(1, sd_printk(KERN_INFO, sdkp, "sd_ioctl: disk=%s, "
"cmd=0x%x\n", disk->disk_name, cmd));
@@ -1079,6 +1081,10 @@ static int sd_ioctl(struct block_device
if (error < 0)
return error;
+ error = scsi_autopm_get_device_sync(sdp);
+ if (error)
+ return error;
+
/*
* If we are in the middle of error recovery, don't let anyone
* else try and use this device. Also, if error recovery fails, it
@@ -1108,6 +1114,7 @@ static int sd_ioctl(struct block_device
break;
}
out:
+ scsi_autopm_put_device_sync(sdp);
return error;
}
@@ -2365,10 +2372,15 @@ static int sd_revalidate_disk(struct gen
struct scsi_device *sdp = sdkp->device;
unsigned char *buffer;
unsigned flush = 0;
+ int error;
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp,
"sd_revalidate_disk\n"));
+ error = scsi_autopm_get_device_sync(sdp);
+ if (error)
+ return 0;
+
/*
* If the device is offline, don't try and read capacity or any
* of the other niceties.
@@ -2421,6 +2433,7 @@ static int sd_revalidate_disk(struct gen
kfree(buffer);
out:
+ scsi_autopm_put_device_sync(sdp);
return 0;
}
@@ -2490,6 +2503,29 @@ static int sd_format_disk_name(char *pre
return 0;
}
+static void sd_on_resume_work(struct work_struct *work)
+{
+ struct scsi_disk *sdkp = container_of(work, struct scsi_disk, work);
+ struct scsi_device *sdev = sdkp->device;
+
+ scsi_run_queue(sdev->request_queue);
+}
+
+/*
+ * Called with dev->power.lock held, scsi_run_queue will acquire the
+ * lock too, so delay to a work item to do that.
+ */
+static int sd_on_resume(struct notifier_block *nb, unsigned long event,
+ void *ptr)
+{
+ struct scsi_disk *sdkp = container_of(nb, struct scsi_disk, nb);
+
+ if (event == RPM_REQ_RESUME)
+ schedule_work(&sdkp->work);
+
+ return NOTIFY_DONE;
+}
+
/*
* The asynchronous part of sd_probe
*/
@@ -2543,6 +2579,9 @@ static void sd_probe_async(void *data, a
sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
sdp->removable ? "removable " : "");
+ INIT_WORK(&sdkp->work, sd_on_resume_work);
+ sdkp->nb.notifier_call = sd_on_resume;
+ atomic_notifier_chain_register(&dev->power.notifier, &sdkp->nb);
scsi_autopm_put_device_sync(sdp);
put_device(&sdkp->dev);
}
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -50,6 +50,8 @@ struct scsi_disk {
struct scsi_driver *driver; /* always &sd_template */
struct scsi_device *device;
struct device dev;
+ struct notifier_block nb;
+ struct work_struct work;
struct gendisk *disk;
atomic_t openers;
sector_t capacity; /* size in 512-byte sectors */
To support request based scsi device runtime PM.
Signed-off-by: Huang Ying <[email protected]>
---
drivers/scsi/scsi_lib.c | 10 ++++++++++
drivers/scsi/scsi_pm.c | 30 ++++++++++++++++++++++++++++++
drivers/scsi/scsi_priv.h | 8 ++++++++
3 files changed, 48 insertions(+)
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -314,6 +314,8 @@ void scsi_device_unbusy(struct scsi_devi
spin_unlock(shost->host_lock);
spin_lock(sdev->request_queue->queue_lock);
sdev->device_busy--;
+ if (!sdev->device_busy)
+ scsi_autopm_put_device(sdev);
spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags);
}
@@ -1418,6 +1420,8 @@ static void scsi_kill_request(struct req
* bump busy counts. To bump the counters, we need to dance
* with the locks as normal issue path does.
*/
+ if (!sdev->device_busy)
+ scsi_autopm_get_device_noresume(sdev);
sdev->device_busy++;
spin_unlock(sdev->request_queue->queue_lock);
spin_lock(shost->host_lock);
@@ -1520,6 +1524,10 @@ static void scsi_request_fn(struct reque
}
+ if (!sdev->device_busy) {
+ if (scsi_autopm_get_device(sdev))
+ break;
+ }
/*
* Remove the request from the request list.
*/
@@ -1600,6 +1608,8 @@ static void scsi_request_fn(struct reque
spin_lock_irq(q->queue_lock);
blk_requeue_request(q, req);
sdev->device_busy--;
+ if (sdev->device_busy == 0)
+ scsi_autopm_put_device_noidle(sdev);
out_delay:
if (sdev->device_busy == 0)
blk_delay_queue(q, SCSI_QUEUE_DELAY);
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -176,12 +176,42 @@ int scsi_autopm_get_device_sync(struct s
}
EXPORT_SYMBOL_GPL(scsi_autopm_get_device_sync);
+int scsi_autopm_get_device(struct scsi_device *sdev)
+{
+ int err;
+
+ err = pm_runtime_get(&sdev->sdev_gendev);
+ if (err < 0 && err != -EACCES && err != -EINPROGRESS &&
+ err != -EAGAIN)
+ pm_runtime_put_noidle(&sdev->sdev_gendev);
+ else
+ err = 0;
+ return err;
+}
+
+void scsi_autopm_get_device_noresume(struct scsi_device *sdev)
+{
+ pm_runtime_get_noresume(&sdev->sdev_gendev);
+}
+
void scsi_autopm_put_device_sync(struct scsi_device *sdev)
{
+ pm_runtime_mark_last_busy(&sdev->sdev_gendev);
pm_runtime_put_sync(&sdev->sdev_gendev);
}
EXPORT_SYMBOL_GPL(scsi_autopm_put_device_sync);
+void scsi_autopm_put_device(struct scsi_device *sdev)
+{
+ pm_runtime_mark_last_busy(&sdev->sdev_gendev);
+ pm_runtime_put(&sdev->sdev_gendev);
+}
+
+void scsi_autopm_put_device_noidle(struct scsi_device *sdev)
+{
+ pm_runtime_put_noidle(&sdev->sdev_gendev);
+}
+
void scsi_autopm_get_target_sync(struct scsi_target *starget)
{
pm_runtime_get_sync(&starget->dev);
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -151,11 +151,19 @@ static inline void scsi_netlink_exit(voi
extern const struct dev_pm_ops scsi_bus_pm_ops;
#endif
#ifdef CONFIG_PM_RUNTIME
+extern int scsi_autopm_get_device(struct scsi_device *);
+extern void scsi_autopm_get_device_noresume(struct scsi_device *);
+extern void scsi_autopm_put_device(struct scsi_device *);
+extern void scsi_autopm_put_device_noidle(struct scsi_device *);
extern void scsi_autopm_get_target_sync(struct scsi_target *);
extern void scsi_autopm_put_target_sync(struct scsi_target *);
extern int scsi_autopm_get_host_sync(struct Scsi_Host *);
extern void scsi_autopm_put_host_sync(struct Scsi_Host *);
#else
+extern int scsi_autopm_get_device(struct scsi_device *sdev) { return 0; }
+extern void scsi_autopm_get_device_noresume(struct scsi_device *sdev) {}
+extern void scsi_autopm_put_device(struct scsi_device *sdev) {}
+extern void scsi_autopm_put_device_noidle(struct scsi_device *sdev) {}
static inline void scsi_autopm_get_target_sync(struct scsi_target *t) {}
static inline void scsi_autopm_put_target_sync(struct scsi_target *t) {}
static inline int scsi_autopm_get_host_sync(struct Scsi_Host *h) { return 0; }
So that the timeout can be adjusted for each device, make it easy to
implement request and open/close based runtime PM.
Signed-off-by: Huang Ying <[email protected]>
---
drivers/scsi/scsi_pm.c | 2 +-
drivers/scsi/scsi_sysfs.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -157,7 +157,7 @@ static int scsi_runtime_idle(struct devi
/* Insert hooks here for targets, hosts, and transport classes */
if (scsi_is_sdev_device(dev))
- err = pm_schedule_suspend(dev, 100);
+ err = pm_runtime_autosuspend(dev);
else
err = pm_runtime_suspend(dev);
return err;
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -887,6 +887,8 @@ int scsi_sysfs_add_sdev(struct scsi_devi
pm_runtime_set_active(&sdev->sdev_gendev);
pm_runtime_forbid(&sdev->sdev_gendev);
pm_runtime_enable(&sdev->sdev_gendev);
+ pm_runtime_set_autosuspend_delay(&sdev->sdev_gendev, 100);
+ pm_runtime_use_autosuspend(&sdev->sdev_gendev);
scsi_autopm_put_target_sync(starget);
/* The following call will keep sdev active indefinitely, until
To distinguish with the asynchronous version we will add later.
Signed-off-by: Huang Ying <[email protected]>
---
drivers/scsi/hosts.c | 2 +-
drivers/scsi/scsi_error.c | 8 ++++----
drivers/scsi/scsi_pm.c | 16 ++++++++--------
drivers/scsi/scsi_priv.h | 16 ++++++++--------
drivers/scsi/scsi_scan.c | 28 ++++++++++++++--------------
drivers/scsi/scsi_sysfs.c | 6 +++---
drivers/scsi/sd.c | 10 +++++-----
drivers/scsi/sg.c | 6 +++---
include/scsi/scsi_device.h | 8 ++++----
9 files changed, 50 insertions(+), 50 deletions(-)
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -168,7 +168,7 @@ void scsi_remove_host(struct Scsi_Host *
}
spin_unlock_irqrestore(shost->host_lock, flags);
- scsi_autopm_get_host(shost);
+ scsi_autopm_get_host_sync(shost);
scsi_forget_host(shost);
mutex_unlock(&shost->scan_mutex);
scsi_proc_host_rm(shost);
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1812,7 +1812,7 @@ int scsi_error_handler(void *data)
* what we need to do to get it up and online again (if we can).
* If we fail, we end up taking the thing offline.
*/
- if (!shost->eh_noresume && scsi_autopm_get_host(shost) != 0) {
+ if (!shost->eh_noresume && scsi_autopm_get_host_sync(shost) != 0) {
SCSI_LOG_ERROR_RECOVERY(1,
printk(KERN_ERR "Error handler scsi_eh_%d "
"unable to autoresume\n",
@@ -1834,7 +1834,7 @@ int scsi_error_handler(void *data)
*/
scsi_restart_operations(shost);
if (!shost->eh_noresume)
- scsi_autopm_put_host(shost);
+ scsi_autopm_put_host_sync(shost);
set_current_state(TASK_INTERRUPTIBLE);
}
__set_current_state(TASK_RUNNING);
@@ -1938,7 +1938,7 @@ scsi_reset_provider(struct scsi_device *
unsigned long flags;
int rtn;
- if (scsi_autopm_get_host(shost) < 0)
+ if (scsi_autopm_get_host_sync(shost) < 0)
return FAILED;
scmd = scsi_get_command(dev, GFP_KERNEL);
@@ -1998,7 +1998,7 @@ scsi_reset_provider(struct scsi_device *
scsi_run_host_queues(shost);
scsi_next_command(scmd);
- scsi_autopm_put_host(shost);
+ scsi_autopm_put_host_sync(shost);
return rtn;
}
EXPORT_SYMBOL(scsi_reset_provider);
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -163,7 +163,7 @@ static int scsi_runtime_idle(struct devi
return err;
}
-int scsi_autopm_get_device(struct scsi_device *sdev)
+int scsi_autopm_get_device_sync(struct scsi_device *sdev)
{
int err;
@@ -174,25 +174,25 @@ int scsi_autopm_get_device(struct scsi_d
err = 0;
return err;
}
-EXPORT_SYMBOL_GPL(scsi_autopm_get_device);
+EXPORT_SYMBOL_GPL(scsi_autopm_get_device_sync);
-void scsi_autopm_put_device(struct scsi_device *sdev)
+void scsi_autopm_put_device_sync(struct scsi_device *sdev)
{
pm_runtime_put_sync(&sdev->sdev_gendev);
}
-EXPORT_SYMBOL_GPL(scsi_autopm_put_device);
+EXPORT_SYMBOL_GPL(scsi_autopm_put_device_sync);
-void scsi_autopm_get_target(struct scsi_target *starget)
+void scsi_autopm_get_target_sync(struct scsi_target *starget)
{
pm_runtime_get_sync(&starget->dev);
}
-void scsi_autopm_put_target(struct scsi_target *starget)
+void scsi_autopm_put_target_sync(struct scsi_target *starget)
{
pm_runtime_put_sync(&starget->dev);
}
-int scsi_autopm_get_host(struct Scsi_Host *shost)
+int scsi_autopm_get_host_sync(struct Scsi_Host *shost)
{
int err;
@@ -204,7 +204,7 @@ int scsi_autopm_get_host(struct Scsi_Hos
return err;
}
-void scsi_autopm_put_host(struct Scsi_Host *shost)
+void scsi_autopm_put_host_sync(struct Scsi_Host *shost)
{
pm_runtime_put_sync(&shost->shost_gendev);
}
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -151,15 +151,15 @@ static inline void scsi_netlink_exit(voi
extern const struct dev_pm_ops scsi_bus_pm_ops;
#endif
#ifdef CONFIG_PM_RUNTIME
-extern void scsi_autopm_get_target(struct scsi_target *);
-extern void scsi_autopm_put_target(struct scsi_target *);
-extern int scsi_autopm_get_host(struct Scsi_Host *);
-extern void scsi_autopm_put_host(struct Scsi_Host *);
+extern void scsi_autopm_get_target_sync(struct scsi_target *);
+extern void scsi_autopm_put_target_sync(struct scsi_target *);
+extern int scsi_autopm_get_host_sync(struct Scsi_Host *);
+extern void scsi_autopm_put_host_sync(struct Scsi_Host *);
#else
-static inline void scsi_autopm_get_target(struct scsi_target *t) {}
-static inline void scsi_autopm_put_target(struct scsi_target *t) {}
-static inline int scsi_autopm_get_host(struct Scsi_Host *h) { return 0; }
-static inline void scsi_autopm_put_host(struct Scsi_Host *h) {}
+static inline void scsi_autopm_get_target_sync(struct scsi_target *t) {}
+static inline void scsi_autopm_put_target_sync(struct scsi_target *t) {}
+static inline int scsi_autopm_get_host_sync(struct Scsi_Host *h) { return 0; }
+static inline void scsi_autopm_put_host_sync(struct Scsi_Host *h) {}
#endif /* CONFIG_PM_RUNTIME */
/*
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1510,18 +1510,18 @@ struct scsi_device *__scsi_add_device(st
starget = scsi_alloc_target(parent, channel, id);
if (!starget)
return ERR_PTR(-ENOMEM);
- scsi_autopm_get_target(starget);
+ scsi_autopm_get_target_sync(starget);
mutex_lock(&shost->scan_mutex);
if (!shost->async_scan)
scsi_complete_async_scans();
- if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host(shost) == 0) {
+ if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host_sync(shost) == 0) {
scsi_probe_and_add_lun(starget, lun, NULL, &sdev, 1, hostdata);
- scsi_autopm_put_host(shost);
+ scsi_autopm_put_host_sync(shost);
}
mutex_unlock(&shost->scan_mutex);
- scsi_autopm_put_target(starget);
+ scsi_autopm_put_target_sync(starget);
scsi_target_reap(starget);
put_device(&starget->dev);
@@ -1575,7 +1575,7 @@ static void __scsi_scan_target(struct de
starget = scsi_alloc_target(parent, channel, id);
if (!starget)
return;
- scsi_autopm_get_target(starget);
+ scsi_autopm_get_target_sync(starget);
if (lun != SCAN_WILD_CARD) {
/*
@@ -1601,7 +1601,7 @@ static void __scsi_scan_target(struct de
}
out_reap:
- scsi_autopm_put_target(starget);
+ scsi_autopm_put_target_sync(starget);
/* now determine if the target has any children at all
* and if not, nuke it */
scsi_target_reap(starget);
@@ -1636,9 +1636,9 @@ void scsi_scan_target(struct device *par
if (!shost->async_scan)
scsi_complete_async_scans();
- if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host(shost) == 0) {
+ if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host_sync(shost) == 0) {
__scsi_scan_target(parent, channel, id, lun, rescan);
- scsi_autopm_put_host(shost);
+ scsi_autopm_put_host_sync(shost);
}
mutex_unlock(&shost->scan_mutex);
}
@@ -1691,7 +1691,7 @@ int scsi_scan_host_selected(struct Scsi_
if (!shost->async_scan)
scsi_complete_async_scans();
- if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host(shost) == 0) {
+ if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host_sync(shost) == 0) {
if (channel == SCAN_WILD_CARD)
for (channel = 0; channel <= shost->max_channel;
channel++)
@@ -1699,7 +1699,7 @@ int scsi_scan_host_selected(struct Scsi_
rescan);
else
scsi_scan_channel(shost, channel, id, lun, rescan);
- scsi_autopm_put_host(shost);
+ scsi_autopm_put_host_sync(shost);
}
mutex_unlock(&shost->scan_mutex);
@@ -1841,7 +1841,7 @@ static int do_scan_async(void *_data)
do_scsi_scan_host(shost);
scsi_finish_async_scan(data);
- scsi_autopm_put_host(shost);
+ scsi_autopm_put_host_sync(shost);
return 0;
}
@@ -1856,20 +1856,20 @@ void scsi_scan_host(struct Scsi_Host *sh
if (strncmp(scsi_scan_type, "none", 4) == 0)
return;
- if (scsi_autopm_get_host(shost) < 0)
+ if (scsi_autopm_get_host_sync(shost) < 0)
return;
data = scsi_prep_async_scan(shost);
if (!data) {
do_scsi_scan_host(shost);
- scsi_autopm_put_host(shost);
+ scsi_autopm_put_host_sync(shost);
return;
}
p = kthread_run(do_scan_async, data, "scsi_scan_%d", shost->host_no);
if (IS_ERR(p))
do_scan_async(data);
- /* scsi_autopm_put_host(shost) is called in do_scan_async() */
+ /* scsi_autopm_put_host_sync(shost) is called in do_scan_async() */
}
EXPORT_SYMBOL(scsi_scan_host);
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -883,17 +883,17 @@ int scsi_sysfs_add_sdev(struct scsi_devi
transport_configure_device(&starget->dev);
device_enable_async_suspend(&sdev->sdev_gendev);
- scsi_autopm_get_target(starget);
+ scsi_autopm_get_target_sync(starget);
pm_runtime_set_active(&sdev->sdev_gendev);
pm_runtime_forbid(&sdev->sdev_gendev);
pm_runtime_enable(&sdev->sdev_gendev);
- scsi_autopm_put_target(starget);
+ scsi_autopm_put_target_sync(starget);
/* The following call will keep sdev active indefinitely, until
* its driver does a corresponding scsi_autopm_pm_device(). Only
* drivers supporting autosuspend will do this.
*/
- scsi_autopm_get_device(sdev);
+ scsi_autopm_get_device_sync(sdev);
error = device_add(&sdev->sdev_gendev);
if (error) {
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -936,7 +936,7 @@ static int sd_open(struct block_device *
sdev = sdkp->device;
- retval = scsi_autopm_get_device(sdev);
+ retval = scsi_autopm_get_device_sync(sdev);
if (retval)
goto error_autopm;
@@ -984,7 +984,7 @@ static int sd_open(struct block_device *
return 0;
error_out:
- scsi_autopm_put_device(sdev);
+ scsi_autopm_put_device_sync(sdev);
error_autopm:
scsi_disk_put(sdkp);
return retval;
@@ -1020,7 +1020,7 @@ static int sd_release(struct gendisk *di
* XXX is followed by a "rmmod sd_mod"?
*/
- scsi_autopm_put_device(sdev);
+ scsi_autopm_put_device_sync(sdev);
scsi_disk_put(sdkp);
return 0;
}
@@ -2543,7 +2543,7 @@ static void sd_probe_async(void *data, a
sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
sdp->removable ? "removable " : "");
- scsi_autopm_put_device(sdp);
+ scsi_autopm_put_device_sync(sdp);
put_device(&sdkp->dev);
}
@@ -2667,7 +2667,7 @@ static int sd_remove(struct device *dev)
struct scsi_disk *sdkp;
sdkp = dev_get_drvdata(dev);
- scsi_autopm_get_device(sdkp->device);
+ scsi_autopm_get_device_sync(sdkp->device);
async_synchronize_full();
blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn);
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -248,7 +248,7 @@ sg_open(struct inode *inode, struct file
if (retval)
goto sg_put;
- retval = scsi_autopm_get_device(sdp->device);
+ retval = scsi_autopm_get_device_sync(sdp->device);
if (retval)
goto sdp_put;
@@ -310,7 +310,7 @@ sg_open(struct inode *inode, struct file
retval = 0;
error_out:
if (retval) {
- scsi_autopm_put_device(sdp->device);
+ scsi_autopm_put_device_sync(sdp->device);
sdp_put:
scsi_device_put(sdp->device);
}
@@ -337,7 +337,7 @@ sg_release(struct inode *inode, struct f
sdp->exclude = 0;
wake_up_interruptible(&sdp->o_excl_wait);
- scsi_autopm_put_device(sdp->device);
+ scsi_autopm_put_device_sync(sdp->device);
kref_put(&sfp->f_ref, sg_remove_sfp);
return 0;
}
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -386,11 +386,11 @@ extern int scsi_execute_req(struct scsi_
int *resid);
#ifdef CONFIG_PM_RUNTIME
-extern int scsi_autopm_get_device(struct scsi_device *);
-extern void scsi_autopm_put_device(struct scsi_device *);
+extern int scsi_autopm_get_device_sync(struct scsi_device *);
+extern void scsi_autopm_put_device_sync(struct scsi_device *);
#else
-static inline int scsi_autopm_get_device(struct scsi_device *d) { return 0; }
-static inline void scsi_autopm_put_device(struct scsi_device *d) {}
+static inline int scsi_autopm_get_device_sync(struct scsi_device *d) { return 0; }
+static inline void scsi_autopm_put_device_sync(struct scsi_device *d) {}
#endif /* CONFIG_PM_RUNTIME */
static inline int __must_check scsi_device_reprobe(struct scsi_device *sdev)
On Mon, 6 Feb 2012, Huang Ying wrote:
> SSD becomes more and more popular, this makes it possible to put disk into
> low power state more often. And request based runtime PM for scsi disk is
> more useful than open/close based one because disk is normally mounted at
> most time.
>
> One known issue, because SCSI TEST_UNIT_READY will be put into request
> queue every 2 seconds by default, this makes it hard for disk to sleep.
> Maybe we can implement check_events callback in some other way?
>
> [RFC 1/5] pm, runtime, Add resume notifier
> [RFC 2/5] scsi, pm, rename scsi_autopm_get/put_xxx to
> [RFC 3/5] scsi, pm, add pm_runtime_get/put in scsi request
> [RFC 4/5] scsi, pm, use autosuspend for scsi runtime PM
> [RFC 5/5] scsi, sd, pm, request based runtime PM support
Your whole approach is at the wrong level. Runtime PM between I/O
requests for block devices should be implemented in the block layer,
not in the SCSI layer.
It also is much more difficult than your patches would indicate. For
example, some USB card readers indicate a media change every time they
resume; therefore they must not be suspended while the device file is
open.
Another difficulty arises because some drivers need to send SCSI
commands (such as SYNCHRONIZE CACHE) _while_ suspending or resuming.
Alan Stern
Hi, Alan,
On Mon, 2012-02-06 at 10:13 -0500, Alan Stern wrote:
> On Mon, 6 Feb 2012, Huang Ying wrote:
>
> > SSD becomes more and more popular, this makes it possible to put disk into
> > low power state more often. And request based runtime PM for scsi disk is
> > more useful than open/close based one because disk is normally mounted at
> > most time.
> >
> > One known issue, because SCSI TEST_UNIT_READY will be put into request
> > queue every 2 seconds by default, this makes it hard for disk to sleep.
> > Maybe we can implement check_events callback in some other way?
> >
> > [RFC 1/5] pm, runtime, Add resume notifier
> > [RFC 2/5] scsi, pm, rename scsi_autopm_get/put_xxx to
> > [RFC 3/5] scsi, pm, add pm_runtime_get/put in scsi request
> > [RFC 4/5] scsi, pm, use autosuspend for scsi runtime PM
> > [RFC 5/5] scsi, sd, pm, request based runtime PM support
>
> Your whole approach is at the wrong level. Runtime PM between I/O
> requests for block devices should be implemented in the block layer,
> not in the SCSI layer.
>
> It also is much more difficult than your patches would indicate. For
> example, some USB card readers indicate a media change every time they
> resume; therefore they must not be suspended while the device file is
> open.
>
> Another difficulty arises because some drivers need to send SCSI
> commands (such as SYNCHRONIZE CACHE) _while_ suspending or resuming.
Thank you very much for your valuable response! We will do more
research work on this topic, including your previous effort. :)
Best Regards,
Huang Ying
Am Montag, 6. Februar 2012, 16:13:37 schrieb Alan Stern:
> On Mon, 6 Feb 2012, Huang Ying wrote:
>
> > SSD becomes more and more popular, this makes it possible to put disk into
> > low power state more often. And request based runtime PM for scsi disk is
> > more useful than open/close based one because disk is normally mounted at
> > most time.
Yes.
> > One known issue, because SCSI TEST_UNIT_READY will be put into request
> > queue every 2 seconds by default, this makes it hard for disk to sleep.
> > Maybe we can implement check_events callback in some other way?
> >
> > [RFC 1/5] pm, runtime, Add resume notifier
> > [RFC 2/5] scsi, pm, rename scsi_autopm_get/put_xxx to
> > [RFC 3/5] scsi, pm, add pm_runtime_get/put in scsi request
> > [RFC 4/5] scsi, pm, use autosuspend for scsi runtime PM
> > [RFC 5/5] scsi, sd, pm, request based runtime PM support
>
> Your whole approach is at the wrong level. Runtime PM between I/O
> requests for block devices should be implemented in the block layer,
> not in the SCSI layer.
I must disagree. The block layer has no more information than the SCSI
layer and lacks everything the lower layers know.
> It also is much more difficult than your patches would indicate. For
> example, some USB card readers indicate a media change every time they
> resume; therefore they must not be suspended while the device file is
> open.
Actually they must not be suspended while they contain a medium,
lest you drive the GUI and the user mad.
> Another difficulty arises because some drivers need to send SCSI
> commands (such as SYNCHRONIZE CACHE) _while_ suspending or resuming.
It seems to me that most of these difficulties go away if we strictly
differentiate between host adapter and disks.
First, the sr driver cannot really suspend a disk. It can spin down a disk,
but that is not the same thing as suspending, because the disk is still
functional. It may just return special sense codes. The sr driver just
prepares devices for suspension.
It is true, that the sr driver probably does have a few conditions under
which a device should not be suspended (eg. error handling) but it
lacks positive knowledge about when we may suspend.
The same is also true for any higher layer.
The problem of needing to do IO for suspension goes away if we
treat the disk as always suspendable and use an active command
as a condition for not suspending the storage device as opposed to the disk
the problem goes away.
Regards
Oliver
On Sat, 11 Feb 2012, Oliver Neukum wrote:
> > Your whole approach is at the wrong level. Runtime PM between I/O
> > requests for block devices should be implemented in the block layer,
> > not in the SCSI layer.
>
> I must disagree. The block layer has no more information than the SCSI
> layer and lacks everything the lower layers know.
But the block layer handles all block devices, not just SCSI ones. You
would end up duplicating code unnecessarily.
What pertinent information is known by the SCSI and lower layers but
not the block layer?
> It seems to me that most of these difficulties go away if we strictly
> differentiate between host adapter and disks.
To be more precise, you mean "disk drives". You can suspend a drive,
but you can't suspend a disk.
> First, the sr driver cannot really suspend a disk. It can spin down a disk,
> but that is not the same thing as suspending, because the disk is still
> functional. It may just return special sense codes. The sr driver just
> prepares devices for suspension.
True. This is because the SCSI standard does not include any notion of
device suspension -- at least, not in the versions I'm aware of.
> It is true, that the sr driver probably does have a few conditions under
> which a device should not be suspended (eg. error handling) but it
> lacks positive knowledge about when we may suspend.
>
> The same is also true for any higher layer.
Then what's wrong with handling runtime suspend in the higher layers?
> The problem of needing to do IO for suspension goes away if we
> treat the disk as always suspendable and use an active command
> as a condition for not suspending the storage device as opposed to the disk
> the problem goes away.
I don't entirely understand. What's the difference between "the
storage device" and "the disk"?
However, using an active command as the condition is not the right
thing to do. It would use extra energy and slow everything down to
suspend and resume the device between every pair of commands that were
separated by a slight time delay. There needs to be a timeout.
Furthermore, if you use active commands as the condition for
suspending, what do you do when the act of suspending causes a command
to be sent? It is necessary to distinguish between ordinary commands
and those that are PM-related.
Alan Stern
Am Sonntag, 12. Februar 2012, 19:05:51 schrieb Alan Stern:
> On Sat, 11 Feb 2012, Oliver Neukum wrote:
>
> > > Your whole approach is at the wrong level. Runtime PM between I/O
> > > requests for block devices should be implemented in the block layer,
> > > not in the SCSI layer.
> >
> > I must disagree. The block layer has no more information than the SCSI
> > layer and lacks everything the lower layers know.
>
> But the block layer handles all block devices, not just SCSI ones. You
> would end up duplicating code unnecessarily.
>
> What pertinent information is known by the SCSI and lower layers but
> not the block layer?
It doesn't know what consequences suspending a host controller
has for the devices attached to it. It simply cannot know because
that depends on the hardware.
A very good example is the media change problem of the media readers.
For example an iSCSI virtual host controller can suspend while a command
is in flight if the network controller used can do remote wakeup well enough.
> > It seems to me that most of these difficulties go away if we strictly
> > differentiate between host adapter and disks.
>
> To be more precise, you mean "disk drives". You can suspend a drive,
> but you can't suspend a disk.
Yes.
> > It is true, that the sr driver probably does have a few conditions under
> > which a device should not be suspended (eg. error handling) but it
> > lacks positive knowledge about when we may suspend.
> >
> > The same is also true for any higher layer.
>
> Then what's wrong with handling runtime suspend in the higher layers?
They know some reasons to not suspend, but not all reasons.
> > The problem of needing to do IO for suspension goes away if we
> > treat the disk as always suspendable and use an active command
> > as a condition for not suspending the storage device as opposed to the disk
> > the problem goes away.
>
> I don't entirely understand. What's the difference between "the
> storage device" and "the disk"?
The storage device == a USB device that implements the storage class
the disk == a SCSI disk drive
> However, using an active command as the condition is not the right
> thing to do. It would use extra energy and slow everything down to
> suspend and resume the device between every pair of commands that were
> separated by a slight time delay. There needs to be a timeout.
Yes, we can use the same heuristics as everywhere.
command queued -> autopm_get
command finished -> autopm_put
but for the USB host adapter, not the sr device
> Furthermore, if you use active commands as the condition for
> suspending, what do you do when the act of suspending causes a command
> to be sent? It is necessary to distinguish between ordinary commands
> and those that are PM-related.
No. This problem goes away if you correctly make the distinction between
host controller/storage device and the disk drive.
You use the "active command standard" for the host controller.
Then you need not care about the commands needed to suspend a disk drive.
You cannot suspend the host controller while the disk drive is being suspended
anyway, as the tree constraint prevents it.
For the disk drive you just declare them busy restarting the timeout as a command
goes down to the hardware. There is an interesting case about what you do if
the generic layer wants to autosuspend an sr device which has a command queued.
I propose we catch that case in sr_suspend() and return -EBUSY in
the autosuspend with command in flight case.
And I admit we will need to special case the stupid card readers.
Regards
Oliver
On Sun, 12 Feb 2012, Oliver Neukum wrote:
> Yes, we can use the same heuristics as everywhere.
> command queued -> autopm_get
> command finished -> autopm_put
>
> but for the USB host adapter, not the sr device
I still don't fully understand. Are you suggesting that we use the
normal autosuspend timeout mechanism for the disk drive (for example,
spin down the disk if it hasn't been used for five minutes), and then
autosuspend a USB mass-storage device whenever its children are
suspended and no commands are in progress? (In fact, there can't be
any commands in progress if all the children are suspended.)
Or are you suggesting that we autosuspend a USB mass-storage device
between commands, regardless of the state of its children? Didn't we
discuss this approach a year or two ago and decide against it?
> > Furthermore, if you use active commands as the condition for
> > suspending, what do you do when the act of suspending causes a command
> > to be sent? It is necessary to distinguish between ordinary commands
> > and those that are PM-related.
>
> No. This problem goes away if you correctly make the distinction between
> host controller/storage device and the disk drive.
> You use the "active command standard" for the host controller.
> Then you need not care about the commands needed to suspend a disk drive.
> You cannot suspend the host controller while the disk drive is being suspended
> anyway, as the tree constraint prevents it.
>
> For the disk drive you just declare them busy restarting the timeout as a command
> goes down to the hardware. There is an interesting case about what you do if
> the generic layer wants to autosuspend an sr device which has a command queued.
> I propose we catch that case in sr_suspend() and return -EBUSY in
> the autosuspend with command in flight case.
The SCSI drivers don't know much about the request queue -- the block
layer manages it. That's another reason for handling this at the block
layer.
Is there some special reason you're talking about sr (the SCSI CD/DVD
driver) instead of sd (the SCSI disk driver)?
Alan Stern
Am Montag, 13. Februar 2012, 02:42:24 schrieb Alan Stern:
> On Sun, 12 Feb 2012, Oliver Neukum wrote:
>
> > Yes, we can use the same heuristics as everywhere.
> > command queued -> autopm_get
> > command finished -> autopm_put
> >
> > but for the USB host adapter, not the sr device
>
> I still don't fully understand. Are you suggesting that we use the
> normal autosuspend timeout mechanism for the disk drive (for example,
> spin down the disk if it hasn't been used for five minutes), and then
Yes. The key here is to realize that from a view point of functionality
they are not suspended. But they will be ready to be suspended.
> autosuspend a USB mass-storage device whenever its children are
> suspended and no commands are in progress? (In fact, there can't be
> any commands in progress if all the children are suspended.)
Really? It is currently true because the device must be opened to issue
commands.
In fact now that you put it this way, it seems to me that this is the
preconception we must shed.
> The SCSI drivers don't know much about the request queue -- the block
> layer manages it. That's another reason for handling this at the block
> layer.
That was refering to SCSI requests.
> Is there some special reason you're talking about sr (the SCSI CD/DVD
> driver) instead of sd (the SCSI disk driver)?
My stupidity.
Regards
Oliver
On Mon, 13 Feb 2012, Oliver Neukum wrote:
> Am Montag, 13. Februar 2012, 02:42:24 schrieb Alan Stern:
> > On Sun, 12 Feb 2012, Oliver Neukum wrote:
> >
> > > Yes, we can use the same heuristics as everywhere.
> > > command queued -> autopm_get
> > > command finished -> autopm_put
> > >
> > > but for the USB host adapter, not the sr device
> >
> > I still don't fully understand. Are you suggesting that we use the
> > normal autosuspend timeout mechanism for the disk drive (for example,
> > spin down the disk if it hasn't been used for five minutes), and then
>
> Yes. The key here is to realize that from a view point of functionality
> they are not suspended. But they will be ready to be suspended.
We appear to be using the word "suspended" in different ways.
When I say a device is suspended, I mean that dev->power.runtime_status
is set to RPM_SUSPENDED, the subsystem's or driver's runtime_suspend
method has been called, and the driver is not ready to carry out I/O
requests immediately (it would have to call pm_runtime_get first). I
do not necessarily mean that the device is at a low power setting.
It seems that you are using the word "suspended" to mean something
else, but it's hard to tell what. Regardless, let's stick to my
meaning. Once the runtime_suspend method for the drive has returned
successfully, the drive is indeed suspended.
> > autosuspend a USB mass-storage device whenever its children are
> > suspended and no commands are in progress? (In fact, there can't be
> > any commands in progress if all the children are suspended.)
>
> Really? It is currently true because the device must be opened to issue
> commands.
> In fact now that you put it this way, it seems to me that this is the
> preconception we must shed.
It is true because all SCSI commands are directed toward one of the
children, i.e., to a particular LUN. If that LUN is suspended then
its driver won't allow any commands to be sent to it. As you say,
right now this is true because the device file must be opened before
commands can be issued -- but even if we remove that restriction, it
will still be true that commands won't be issued while the device is
suspended.
It's not a preconception; it is a basic design principle of the
kernel's Runtime PM implementation.
> > The SCSI drivers don't know much about the request queue -- the block
> > layer manages it. That's another reason for handling this at the block
> > layer.
>
> That was refering to SCSI requests.
That's the same thing. The SCSI layer doesn't have its own request
queues; it uses the queues provided by the block layer.
Alan Stern
On Sun, 12 Feb 2012, Oliver Neukum wrote:
> Am Sonntag, 12. Februar 2012, 19:05:51 schrieb Alan Stern:
> > On Sat, 11 Feb 2012, Oliver Neukum wrote:
> >
> > > > Your whole approach is at the wrong level. Runtime PM between I/O
> > > > requests for block devices should be implemented in the block layer,
> > > > not in the SCSI layer.
> > >
> > > I must disagree. The block layer has no more information than the SCSI
> > > layer and lacks everything the lower layers know.
> >
> > But the block layer handles all block devices, not just SCSI ones. You
> > would end up duplicating code unnecessarily.
> >
> > What pertinent information is known by the SCSI and lower layers but
> > not the block layer?
>
> It doesn't know what consequences suspending a host controller
> has for the devices attached to it. It simply cannot know because
> that depends on the hardware.
>
> A very good example is the media change problem of the media readers.
> For example an iSCSI virtual host controller can suspend while a command
> is in flight if the network controller used can do remote wakeup well enough.
I don't see how this is relevant. We're talking about suspending the
drive, not suspending the host adapter. Runtime PM for an iSCSI host
adapter can be implemented to suspend in the middle of commands if it
wants; that has no bearing on this discussion and patch set.
> > Then what's wrong with handling runtime suspend in the higher layers?
>
> They know some reasons to not suspend, but not all reasons.
So the lower layers can do their own autopm_get when necessary.
> > > The problem of needing to do IO for suspension goes away if we
> > > treat the disk as always suspendable and use an active command
> > > as a condition for not suspending the storage device as opposed to the disk
> > > the problem goes away.
> >
> > I don't entirely understand. What's the difference between "the
> > storage device" and "the disk"?
>
> The storage device == a USB device that implements the storage class
> the disk == a SCSI disk drive
And in the case of some other sort of SCSI transport, I suppose "the
storage device" includes the host adapter or equivalent.
> > However, using an active command as the condition is not the right
> > thing to do. It would use extra energy and slow everything down to
> > suspend and resume the device between every pair of commands that were
> > separated by a slight time delay. There needs to be a timeout.
>
> Yes, we can use the same heuristics as everywhere.
> command queued -> autopm_get
> command finished -> autopm_put
>
> but for the USB host adapter, not the sr device
Didn't we discuss this a long time ago and decide it was a bad
approach?
> > Furthermore, if you use active commands as the condition for
> > suspending, what do you do when the act of suspending causes a command
> > to be sent? It is necessary to distinguish between ordinary commands
> > and those that are PM-related.
>
> No. This problem goes away if you correctly make the distinction between
> host controller/storage device and the disk drive.
> You use the "active command standard" for the host controller.
> Then you need not care about the commands needed to suspend a disk drive.
> You cannot suspend the host controller while the disk drive is being suspended
> anyway, as the tree constraint prevents it.
Heh, you're talking about ignoring the tree constraint anyway if you're
going to suspend the USB device while the children beneath it remain
unchanged.
>
> For the disk drive you just declare them busy restarting the timeout as a command
> goes down to the hardware. There is an interesting case about what you do if
> the generic layer wants to autosuspend an sr device which has a command queued.
> I propose we catch that case in sr_suspend() and return -EBUSY in
> the autosuspend with command in flight case.
>
> And I admit we will need to special case the stupid card readers.
>
> Regards
> Oliver
>