linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aaron Lu <aaron.lu@intel.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Derek Basehore <dbasehore@chromium.org>,
	James Bottomley <JBottomley@Parallels.com>,
	Jeff Garzik <jgarzik@pobox.com>,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
	SCSI development list <linux-scsi@vger.kernel.org>,
	Linux-pm mailing list <linux-pm@vger.kernel.org>
Subject: Re: [PATCH 1/2] don't wait on disk to start on resume
Date: Mon, 04 Feb 2013 16:04:57 +0800	[thread overview]
Message-ID: <510F6BA9.1050701@intel.com> (raw)
In-Reply-To: <510E025A.9000103@intel.com>

On 02/03/2013 02:23 PM, Aaron Lu wrote:
> No, the modification is actually for disk.
> With v8 of block layer runtime PM, it is no longer the case runtime
> suspend is the same as system suspend for hard disk that utilize block
> layer runtime PM: we quiesce the device and run its suspend callback for
> the device during system suspend but we didn't touch the queue's
> rpm_status as we do in runtime_suspend callback. So I did some
> modifications to scsi_pm.c to make runtime suspend and system suspend do
> exactly the same thing for disk type scsi device, no matter if they are
> using block layer runtime PM or not.
> 
> Probably I had better post code here, this is a replacement for the
> patch 4 of v8 block layer runtime PM patchset(I omit the sd part, since
> it is irrevelant), please kindly review, see if you like it :-)
> 
> diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
> index 8f6b12c..d9956b6 100644
> --- a/drivers/scsi/scsi_pm.c
> +++ b/drivers/scsi/scsi_pm.c
> @@ -16,17 +16,44 @@
>  
>  #include "scsi_priv.h"
>  
> +static int sdev_blk_suspend(struct scsi_device *sdev)
> +{
> +	int err;
> +
> +	err = blk_pre_runtime_suspend(sdev->request_queue);
> +	if (err)
> +		return err;
> +	err = pm_generic_runtime_suspend(&sdev->sdev_gendev);
> +	blk_post_runtime_suspend(sdev->request_queue, err);
> +
> +	return err;
> +}
> +
>  static int scsi_dev_type_suspend(struct device *dev, int (*cb)(struct device *))
>  {
> +	struct scsi_device *sdev = to_scsi_device(dev);
>  	int err;
>  
> -	err = scsi_device_quiesce(to_scsi_device(dev));
> +	err = scsi_device_quiesce(sdev);
>  	if (err == 0) {
> -		if (cb) {
> +		if (sdev->request_queue->dev)
> +			err = sdev_blk_suspend(sdev);
> +		else if (cb)
>  			err = cb(dev);
> -			if (err)
> -				scsi_device_resume(to_scsi_device(dev));
> -		}
> +
> +		if (err)
> +			scsi_device_resume(sdev);
>  	}
>  	dev_dbg(dev, "scsi suspend: %d\n", err);
>  	return err;

I just realized that this will not work, as we can't request that there
is no request pending in the device's request queue when doing system
suspend. And if we can't request this, we will not be able to runtime
resume the device due to nr_pending may not be zero when new request
comes after system resumed, though this can be solved if we modify the
runtime resume policy.

During my test, I'm not doing any IO when suspend, so the nr_pending is
zero and it worked...

Please ignore my suggestion for this thread, sorry for the noise.

-Aaron


      parent reply	other threads:[~2013-02-04  8:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <Pine.LNX.4.44L0.1302011026070.1752-100000@iolanthe.rowland.org>
2013-02-02 10:45 ` [PATCH 1/2] don't wait on disk to start on resume Aaron Lu
2013-02-02 15:09   ` Alan Stern
2013-02-03  6:23     ` Aaron Lu
2013-02-04  0:07       ` dbasehore .
2013-02-04 14:27         ` Aaron Lu
2013-02-04  8:04       ` Aaron Lu [this message]

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=510F6BA9.1050701@intel.com \
    --to=aaron.lu@intel.com \
    --cc=JBottomley@Parallels.com \
    --cc=dbasehore@chromium.org \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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;
as well as URLs for NNTP newsgroup(s).