Re: [PATCH v7 4/4] sd: change to auto suspend mode

2013-01-28 Thread Aaron Lu 
On Fri, Jan 18, 2013 at 04:25:10PM -0500, Alan Stern wrote:
 On Wed, 16 Jan 2013, Aaron Lu wrote:
 
  From: Lin Ming ming.m@intel.com
  
  Uses block layer runtime pm helper functions in
  scsi_runtime_suspend/resume.
  Remove scsi_autopm_* from sd open/release path and check_events path.
  And remove the quiesce call in runtime suspend path, as we know there is
  no request to quiesce for the device.
  
  Signed-off-by: Lin Ming ming.m@intel.com
  Signed-off-by: Aaron Lu aaron...@intel.com
 
  --- a/drivers/scsi/scsi_sysfs.c
  +++ b/drivers/scsi/scsi_sysfs.c
  @@ -893,6 +893,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
   */
  scsi_autopm_get_device(sdev);
   
  +   blk_pm_runtime_init(rq, sdev-sdev_gendev);
  +
  error = device_add(sdev-sdev_gendev);
  if (error) {
  sdev_printk(KERN_INFO, sdev,
 
 Someone just asked about the default autosuspend delay, and I realized
 your patch series doesn't set one.  Since we don't know the properties
 of the disk drive at this point (or even whether the device is a disk
 drive), the only safe course is to call
 
   pm_runtime_set_autosuspend_delay(sdev-sdev_gendev, -1);
 
 before calling blk_pm_runtime_init().  Then autosuspends will be 
 prevented until userspace writes a non-negative value into the device's 
 control/autosuspend_delay_ms file.

Shall we do it inside blk_pm_runtime_init? This way, we do not need to
do it for every driver. And for drivers that do know a proper value for
autosuspend delay, they can call pm_runtime_set_autosuspend_delay
somewhere after blk_pm_runtime_init.

Thanks,
Aaron

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

Re: [PATCH v7 4/4] sd: change to auto suspend mode

2013-01-28 Thread Alan Stern 
On Mon, 28 Jan 2013, Aaron Lu wrote:

  Someone just asked about the default autosuspend delay, and I realized
  your patch series doesn't set one.  Since we don't know the properties
  of the disk drive at this point (or even whether the device is a disk
  drive), the only safe course is to call
  
  pm_runtime_set_autosuspend_delay(sdev-sdev_gendev, -1);
  
  before calling blk_pm_runtime_init().  Then autosuspends will be 
  prevented until userspace writes a non-negative value into the device's 
  control/autosuspend_delay_ms file.
 
 Shall we do it inside blk_pm_runtime_init? This way, we do not need to
 do it for every driver. And for drivers that do know a proper value for
 autosuspend delay, they can call pm_runtime_set_autosuspend_delay
 somewhere after blk_pm_runtime_init.

Yes, that seems like a good approach.  Be sure to mention it in the 
kerneldoc.

Alan Stern

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

Re: [PATCH v7 4/4] sd: change to auto suspend mode

2013-01-21 Thread Aaron Lu 

On 01/19/2013 05:25 AM, Alan Stern wrote:

On Wed, 16 Jan 2013, Aaron Lu wrote:


From: Lin Mingming.m@intel.com

Uses block layer runtime pm helper functions in
scsi_runtime_suspend/resume.
Remove scsi_autopm_* from sd open/release path and check_events path.
And remove the quiesce call in runtime suspend path, as we know there is
no request to quiesce for the device.

Signed-off-by: Lin Mingming.m@intel.com
Signed-off-by: Aaron Luaaron...@intel.com



--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -893,6 +893,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 */
scsi_autopm_get_device(sdev);

+   blk_pm_runtime_init(rq,sdev-sdev_gendev);
+
error = device_add(sdev-sdev_gendev);
if (error) {
sdev_printk(KERN_INFO, sdev,


Someone just asked about the default autosuspend delay, and I realized
your patch series doesn't set one.  Since we don't know the properties
of the disk drive at this point (or even whether the device is a disk
drive), the only safe course is to call

pm_runtime_set_autosuspend_delay(sdev-sdev_gendev, -1);

before calling blk_pm_runtime_init().  Then autosuspends will be
prevented until userspace writes a non-negative value into the device's
control/autosuspend_delay_ms file.


OK, that would be safer, thanks for the suggestion.
I often set the autosuspend delay before allowing runtime PM for the
device, since we forbid it in scsi_sysfs_add_sdev.



The kerneldoc for blk_pm_runtime_init() should mention that the caller
needs to set the autosuspend delay beforehand.


OK.

Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 4/4] sd: change to auto suspend mode

2013-01-18 Thread Alan Stern 
On Wed, 16 Jan 2013, Aaron Lu wrote:

 From: Lin Ming ming.m@intel.com
 
 Uses block layer runtime pm helper functions in
 scsi_runtime_suspend/resume.
 Remove scsi_autopm_* from sd open/release path and check_events path.
 And remove the quiesce call in runtime suspend path, as we know there is
 no request to quiesce for the device.
 
 Signed-off-by: Lin Ming ming.m@intel.com
 Signed-off-by: Aaron Lu aaron...@intel.com

 --- a/drivers/scsi/scsi_sysfs.c
 +++ b/drivers/scsi/scsi_sysfs.c
 @@ -893,6 +893,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
*/
   scsi_autopm_get_device(sdev);
  
 + blk_pm_runtime_init(rq, sdev-sdev_gendev);
 +
   error = device_add(sdev-sdev_gendev);
   if (error) {
   sdev_printk(KERN_INFO, sdev,

Someone just asked about the default autosuspend delay, and I realized
your patch series doesn't set one.  Since we don't know the properties
of the disk drive at this point (or even whether the device is a disk
drive), the only safe course is to call

pm_runtime_set_autosuspend_delay(sdev-sdev_gendev, -1);

before calling blk_pm_runtime_init().  Then autosuspends will be 
prevented until userspace writes a non-negative value into the device's 
control/autosuspend_delay_ms file.

The kerneldoc for blk_pm_runtime_init() should mention that the caller 
needs to set the autosuspend delay beforehand.

Alan Stern

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

[PATCH v7 4/4] sd: change to auto suspend mode

2013-01-16 Thread Aaron Lu 
From: Lin Ming ming.m@intel.com

Uses block layer runtime pm helper functions in
scsi_runtime_suspend/resume.
Remove scsi_autopm_* from sd open/release path and check_events path.
And remove the quiesce call in runtime suspend path, as we know there is
no request to quiesce for the device.

Signed-off-by: Lin Ming ming.m@intel.com
Signed-off-by: Aaron Lu aaron...@intel.com
---
 drivers/scsi/scsi_pm.c| 35 +++
 drivers/scsi/scsi_sysfs.c |  2 ++
 drivers/scsi/sd.c | 12 
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 8f6b12c..d5ddc2c 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -147,15 +147,19 @@ static int scsi_bus_restore(struct device *dev)
 static int scsi_runtime_suspend(struct device *dev)
 {
int err = 0;
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
 
dev_dbg(dev, scsi_runtime_suspend\n);
if (scsi_is_sdev_device(dev)) {
-   err = scsi_dev_type_suspend(dev,
-   pm ? pm-runtime_suspend : NULL);
-   if (err == -EAGAIN)
-   pm_schedule_suspend(dev, jiffies_to_msecs(
-   round_jiffies_up_relative(HZ/10)));
+   struct scsi_device *sdev = to_scsi_device(dev);
+   struct request_queue *q = sdev-request_queue;
+
+   err = blk_pre_runtime_suspend(q);
+   if (err)
+   return err;
+
+   err = pm_generic_runtime_suspend(dev);
+
+   blk_post_runtime_suspend(q, err);
}
 
/* Insert hooks here for targets, hosts, and transport classes */
@@ -166,11 +170,16 @@ static int scsi_runtime_suspend(struct device *dev)
 static int scsi_runtime_resume(struct device *dev)
 {
int err = 0;
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
 
dev_dbg(dev, scsi_runtime_resume\n);
-   if (scsi_is_sdev_device(dev))
-   err = scsi_dev_type_resume(dev, pm ? pm-runtime_resume : NULL);
+   if (scsi_is_sdev_device(dev)) {
+   struct scsi_device *sdev = to_scsi_device(dev);
+   struct request_queue *q = sdev-request_queue;
+
+   blk_pre_runtime_resume(q);
+   err = pm_generic_runtime_resume(dev);
+   blk_post_runtime_resume(q, err);
+   }
 
/* Insert hooks here for targets, hosts, and transport classes */
 
@@ -185,10 +194,12 @@ static int scsi_runtime_idle(struct device *dev)
 
/* Insert hooks here for targets, hosts, and transport classes */
 
-   if (scsi_is_sdev_device(dev))
-   err = pm_schedule_suspend(dev, 100);
-   else
+   if (scsi_is_sdev_device(dev)) {
+   pm_runtime_mark_last_busy(dev);
+   err = pm_runtime_autosuspend(dev);
+   } else {
err = pm_runtime_suspend(dev);
+   }
return err;
 }
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 931a7d9..fff7637 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -893,6 +893,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 */
scsi_autopm_get_device(sdev);
 
+   blk_pm_runtime_init(rq, sdev-sdev_gendev);
+
error = device_add(sdev-sdev_gendev);
if (error) {
sdev_printk(KERN_INFO, sdev,
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8ca160e..70c202c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1121,10 +1121,6 @@ static int sd_open(struct block_device *bdev, fmode_t 
mode)
 
sdev = sdkp-device;
 
-   retval = scsi_autopm_get_device(sdev);
-   if (retval)
-   goto error_autopm;
-
/*
 * If the device is in error recovery, wait until it is done.
 * If the device is offline, then disallow any access to it.
@@ -1169,8 +1165,6 @@ static int sd_open(struct block_device *bdev, fmode_t 
mode)
return 0;
 
 error_out:
-   scsi_autopm_put_device(sdev);
-error_autopm:
scsi_disk_put(sdkp);
return retval;  
 }
@@ -1205,7 +1199,6 @@ static int sd_release(struct gendisk *disk, fmode_t mode)
 * XXX is followed by a rmmod sd_mod?
 */
 
-   scsi_autopm_put_device(sdev);
scsi_disk_put(sdkp);
return 0;
 }
@@ -1367,14 +1360,9 @@ static unsigned int sd_check_events(struct gendisk 
*disk, unsigned int clearing)
retval = -ENODEV;
 
if (scsi_block_when_processing_errors(sdp)) {
-   retval = scsi_autopm_get_device(sdp);
-   if (retval)
-   goto out;
-
sshdr  = kzalloc(sizeof(*sshdr), GFP_KERNEL);
retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES,
  sshdr);
-   scsi_autopm_put_device(sdp);