* Re: [PATCH v3 2/2] Hard disk S3 resume time optimization
[not found] <20130823214340.GC11321@todd.e.brandt@intel.com>
@ 2013-08-24 8:21 ` Oliver Neukum
2013-08-27 14:50 ` Brandt, Todd E
0 siblings, 1 reply; 4+ messages in thread
From: Oliver Neukum @ 2013-08-24 8:21 UTC (permalink / raw)
To: Brandt, Todd E; +Cc: linux-ide, linux-scsi, tj, gregkh, stern
On Fri, 2013-08-23 at 14:43 -0700, Brandt, Todd E wrote:
> This is v3 of the non-blocking S3 resume patch. It's been broken into
> two pieces, this part is for the scsi subsystem. I've addressed Alan
> Stern's comments in particular by reformatting the call to conform
> to the proper style guidelines.
Hi,
you are changing the generic resume for all sd disks.
In particular, you are dropping the check of
sdkp->device->manage_start_stop
What happens if you use this patch on a real SCSI disk?
Regards
Oliver
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH v3 2/2] Hard disk S3 resume time optimization
2013-08-24 8:21 ` [PATCH v3 2/2] Hard disk S3 resume time optimization Oliver Neukum
@ 2013-08-27 14:50 ` Brandt, Todd E
0 siblings, 0 replies; 4+ messages in thread
From: Brandt, Todd E @ 2013-08-27 14:50 UTC (permalink / raw)
To: Oliver Neukum
Cc: linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
tj@kernel.org, gregkh@linuxfoundation.org,
stern@rowland.harvard.edu
All the same checks are preserved, including the manage_start_stop check:
+ if (!sdkp->device->manage_start_stop)
+ goto error;
That function is just a simplified version of sd_start_stop_device but with only the start functionality used.
-----Original Message-----
From: Oliver Neukum [mailto:oneukum@suse.de]
Sent: Saturday, August 24, 2013 1:21 AM
To: Brandt, Todd E
Cc: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org; tj@kernel.org; gregkh@linuxfoundation.org; stern@rowland.harvard.edu
Subject: Re: [PATCH v3 2/2] Hard disk S3 resume time optimization
On Fri, 2013-08-23 at 14:43 -0700, Brandt, Todd E wrote:
> This is v3 of the non-blocking S3 resume patch. It's been broken into
> two pieces, this part is for the scsi subsystem. I've addressed Alan
> Stern's comments in particular by reformatting the call to conform
> to the proper style guidelines.
Hi,
you are changing the generic resume for all sd disks.
In particular, you are dropping the check of
sdkp->device->manage_start_stop
What happens if you use this patch on a real SCSI disk?
Regards
Oliver
^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20130823223535.GB11706@todd.e.brandt@intel.com>]
* [PATCH v3 2/2] Hard disk S3 resume time optimization
[not found] <20130823223535.GB11706@todd.e.brandt@intel.com>
@ 2013-08-27 14:41 ` Brandt, Todd E
0 siblings, 0 replies; 4+ messages in thread
From: Brandt, Todd E @ 2013-08-27 14:41 UTC (permalink / raw)
To: linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org
This is v3 of the non-blocking S3 resume patch. It's been broken into
two pieces, this part is for the scsi subsystem. I've addressed Alan
Stern's comments in particular by reformatting the call to conform
to the proper style guidelines.
Note - the two patches will function separately but both are required
to achieve a performance improvement.
Signed-off-by: Todd Brandt <todd.e.brandt@intel.com>
drivers/scsi/sd.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 81 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 86fcf2c..d4bf784 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -107,6 +107,7 @@ static int sd_remove(struct device *);
static void sd_shutdown(struct device *);
static int sd_suspend(struct device *);
static int sd_resume(struct device *);
+static int sd_resume_async(struct device *);
static void sd_rescan(struct device *);
static int sd_done(struct scsi_cmnd *);
static int sd_eh_action(struct scsi_cmnd *, unsigned char *, int, int);
@@ -484,7 +485,7 @@ static struct class sd_disk_class = {
static const struct dev_pm_ops sd_pm_ops = {
.suspend = sd_suspend,
- .resume = sd_resume,
+ .resume = sd_resume_async,
.poweroff = sd_suspend,
.restore = sd_resume,
.runtime_suspend = sd_suspend,
@@ -3137,6 +3138,85 @@ done:
return ret;
}
+static void sd_resume_async_end(struct request *rq, int error)
+{
+ struct scsi_sense_hdr sshdr;
+ struct scsi_disk *sdkp = rq->end_io_data;
+ char *sense = rq->sense;
+
+ if (error) {
+ sd_printk(KERN_WARNING, sdkp, "START FAILED\n");
+ sd_print_result(sdkp, error);
+ if (sense && (driver_byte(error) & DRIVER_SENSE)) {
+ scsi_normalize_sense(sense,
+ SCSI_SENSE_BUFFERSIZE, &sshdr);
+ sd_print_sense_hdr(sdkp, &sshdr);
+ }
+ } else
+ sd_printk(KERN_NOTICE, sdkp, "START SUCCESS\n");
+
+ kfree(sense);
+ rq->sense = NULL;
+ rq->end_io_data = NULL;
+ __blk_put_request(rq->q, rq);
+ scsi_disk_put(sdkp);
+}
+
+static int sd_resume_async(struct device *dev)
+{
+ unsigned char cmd[6] = { START_STOP };
+ struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
+ struct request *req;
+ char *sense = NULL;
+ int ret = 0;
+
+ if (!sdkp->device->manage_start_stop)
+ goto error;
+
+ sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
+
+ cmd[4] |= 1;
+
+ if (sdkp->device->start_stop_pwr_cond)
+ cmd[4] |= 1 << 4; /* Active or Standby */
+
+ if (!scsi_device_online(sdkp->device)) {
+ ret = -ENODEV;
+ goto error;
+ }
+
+ req = blk_get_request(sdkp->device->request_queue, 0, __GFP_WAIT);
+ if (!req) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ sense = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_NOIO);
+ if (!sense) {
+ ret = -ENOMEM;
+ goto error_sense;
+ }
+
+ req->cmd_len = COMMAND_SIZE(cmd[0]);
+ memcpy(req->cmd, cmd, req->cmd_len);
+ req->sense = sense;
+ req->sense_len = 0;
+ req->retries = SD_MAX_RETRIES;
+ req->timeout = SD_TIMEOUT;
+ req->cmd_type = REQ_TYPE_BLOCK_PC;
+ req->cmd_flags |= REQ_PM | REQ_QUIET | REQ_PREEMPT;
+
+ req->end_io_data = sdkp;
+ blk_execute_rq_nowait(req->q, NULL, req, 1, sd_resume_async_end);
+ return 0;
+
+ error_sense:
+ __blk_put_request(req->q, req);
+ error:
+ scsi_disk_put(sdkp);
+ return ret;
+}
+
/**
* init_sd - entry point for this driver (both when built in or when
* a module).
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH/RESEND v2 1/2] Hard disk S3 resume time optimization
@ 2014-01-08 0:56 Todd E Brandt
2014-01-11 19:13 ` Tejun Heo
0 siblings, 1 reply; 4+ messages in thread
From: Todd E Brandt @ 2014-01-08 0:56 UTC (permalink / raw)
To: tj, James.Bottomley; +Cc: linux-ide, linux-scsi
On resume, the ATA port driver currently waits until the AHCI controller
finishes executing the port wakeup command. This patch changes the
ata_port_resume callback to issue the wakeup and then return immediately,
thus allowing the next device in the pm queue to resume. Any commands
issued to the AHCI hardware during the wakeup will be queued up and
executed once the port is physically online. Thus no information is lost.
This patch only changes the behavior of the resume callback, not restore,
thaw, or runtime-resume. This is because thaw and restore are used after a
suspend-to-disk, which means that an image needs to be read from swap and
reloaded into RAM. The swap disk will always need to be fully restored/thawed
in order for resume to continue.
The return value from ata_resume_async is now an indicator of whether
the resume was initiated, rather than if it was completed. I'm letting the ata
driver assume control over its own error reporting in this case (which it does
already). If you look at the ata_port resume code you'll see that the
ata_port_resume callback returns the status of the ahci_port_resume callback,
which is always 0. So I don't see any harm in ignoring it.
If somebody requests suspend while ATA port resume is still running, the
request is queued until the resume has completed. I've tested
that case very heavily. Basically if you do two suspends in a row (within
seconds of each other) you lose any performance benefit, but that's a use
case that should happen only very rarerly (and wouldn't be expected to
be of any power benefit since too many sequential suspends actually
takes more power than just letting the machine stay in run mode).
Signed-off-by: Todd Brandt <todd.e.brandt@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
drivers/ata/libata-core.c | 48 ++++++++++++++++++++++++++++++++----------------
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 75b9367..4819b93 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5312,7 +5312,7 @@ 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 *async)
- int *async)
+ bool async, int *async_result)
{
struct ata_link *link;
unsigned long flags;
@@ -5322,8 +5322,8 @@ static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
* progress. Wait for PM_PENDING to clear.
*/
if (ap->pflags & ATA_PFLAG_PM_PENDING) {
- if (async) {
- *async = -EAGAIN;
+ if (async && async_result) {
+ *async_result = -EAGAIN;
return 0;
}
ata_port_wait_eh(ap);
@@ -5335,7 +5335,7 @@ static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
ap->pm_mesg = mesg;
if (async)
- ap->pm_result = async;
+ ap->pm_result = async_result;
else
ap->pm_result = &rc;
@@ -5358,7 +5358,8 @@ static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
return rc;
}
-static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int *async)
+static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg,
+ bool async, int *async_result)
{
/*
* On some hardware, device fails to respond after spun down
@@ -5370,14 +5371,14 @@ static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int
*/
unsigned int ehi_flags = ATA_EHI_QUIET | ATA_EHI_NO_AUTOPSY |
ATA_EHI_NO_RECOVERY;
- return ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
+ return ata_port_request_pm(ap, mesg, 0, ehi_flags, async, async_result);
}
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);
+ return __ata_port_suspend_common(ap, mesg, false, NULL);
}
static int ata_port_suspend(struct device *dev)
@@ -5402,27 +5403,42 @@ static int ata_port_poweroff(struct device *dev)
}
static int __ata_port_resume_common(struct ata_port *ap, pm_message_t mesg,
- int *async)
+ bool async, int *async_result)
{
int rc;
rc = ata_port_request_pm(ap, mesg, ATA_EH_RESET,
- ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, async);
+ ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, async, async_result);
return rc;
}
-static int ata_port_resume_common(struct device *dev, pm_message_t mesg)
+static int ata_port_resume_common(struct device *dev, pm_message_t mesg,
+ bool async)
{
struct ata_port *ap = to_ata_port(dev);
- return __ata_port_resume_common(ap, mesg, NULL);
+ return __ata_port_resume_common(ap, mesg, async, NULL);
+}
+
+static int ata_port_resume_async(struct device *dev)
+{
+ int rc;
+
+ rc = ata_port_resume_common(dev, PMSG_RESUME, true);
+ if (!rc) {
+ pm_runtime_disable(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ }
+
+ return rc;
}
static int ata_port_resume(struct device *dev)
{
int rc;
- rc = ata_port_resume_common(dev, PMSG_RESUME);
+ rc = ata_port_resume_common(dev, PMSG_RESUME, false);
if (!rc) {
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
@@ -5463,12 +5479,12 @@ static int ata_port_runtime_suspend(struct device *dev)
static int ata_port_runtime_resume(struct device *dev)
{
- return ata_port_resume_common(dev, PMSG_AUTO_RESUME);
+ return ata_port_resume_common(dev, PMSG_AUTO_RESUME, false);
}
static const struct dev_pm_ops ata_port_pm_ops = {
.suspend = ata_port_suspend,
- .resume = ata_port_resume,
+ .resume = ata_port_resume_async,
.freeze = ata_port_do_freeze,
.thaw = ata_port_resume,
.poweroff = ata_port_poweroff,
@@ -5486,13 +5502,13 @@ static const struct dev_pm_ops ata_port_pm_ops = {
*/
int ata_sas_port_async_suspend(struct ata_port *ap, int *async)
{
- return __ata_port_suspend_common(ap, PMSG_SUSPEND, async);
+ return __ata_port_suspend_common(ap, PMSG_SUSPEND, true, 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, PMSG_RESUME, async);
+ return __ata_port_resume_common(ap, PMSG_RESUME, true, async);
}
EXPORT_SYMBOL_GPL(ata_sas_port_async_resume);
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH/RESEND v2 1/2] Hard disk S3 resume time optimization
2014-01-08 0:56 [PATCH/RESEND v2 1/2] " Todd E Brandt
@ 2014-01-11 19:13 ` Tejun Heo
2014-01-13 19:55 ` Todd E Brandt
0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2014-01-11 19:13 UTC (permalink / raw)
To: Todd E Brandt; +Cc: James.Bottomley, linux-ide, linux-scsi
Hello,
On Tue, Jan 07, 2014 at 04:56:07PM -0800, Todd E Brandt wrote:
> On resume, the ATA port driver currently waits until the AHCI controller
> finishes executing the port wakeup command. This patch changes the
Is there anything ahci specific about this? There shouldn't be.
> This patch only changes the behavior of the resume callback, not restore,
> thaw, or runtime-resume. This is because thaw and restore are used after a
> suspend-to-disk, which means that an image needs to be read from swap and
> reloaded into RAM. The swap disk will always need to be fully restored/thawed
> in order for resume to continue.
If the system has multiple devices, wouldn't that still reduce the
latency? Do we really need to deviate behavior among different
resume/unfreeze paths?
> The return value from ata_resume_async is now an indicator of whether
> the resume was initiated, rather than if it was completed. I'm letting the ata
> driver assume control over its own error reporting in this case (which it does
> already). If you look at the ata_port resume code you'll see that the
> ata_port_resume callback returns the status of the ahci_port_resume callback,
> which is always 0. So I don't see any harm in ignoring it.
I've been always kinda doubtful about the usefulness of resume return
code. It's not like there's anything resume path can do differently
upon being notified that resume of a device failed. Just reporting
success and deal with error conditions as usual should work fine,
right?
Well, in fact, I don't think even the return code of suspend is
useful. Failing suspend is most likely wrong. The only action PM
core can take is aborting suspend and AFAICS none of the libata
drivers has suspend failure conditions whose recoverability is made
worse by just proceeding with suspend. The hardware is recycled after
all anyway. The device is inoperational anyway, so aborting suspend
doens't buy us anything other than failing suspend, which is almost
always wrong. So, I'd actually prefer just removing all returns from
suspend/resume operations, but yeah this might be out of scope for
this series.
> +static int ata_port_resume_async(struct device *dev)
> +{
> + int rc;
> +
> + rc = ata_port_resume_common(dev, PMSG_RESUME, true);
> + if (!rc) {
> + pm_runtime_disable(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + }
> +
> + return rc;
> }
So, can't just everything become async? Are there cases where we
*need* synchronous PM behaviors?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH/RESEND v2 1/2] Hard disk S3 resume time optimization
2014-01-11 19:13 ` Tejun Heo
@ 2014-01-13 19:55 ` Todd E Brandt
2014-01-13 20:30 ` Tejun Heo
0 siblings, 1 reply; 4+ messages in thread
From: Todd E Brandt @ 2014-01-13 19:55 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-ide, linux-scsi, James.Bottomley, rjw
On Sat, Jan 11, 2014 at 02:13:15PM -0500, Tejun Heo wrote:
> Hello,
>
> On Tue, Jan 07, 2014 at 04:56:07PM -0800, Todd E Brandt wrote:
> > On resume, the ATA port driver currently waits until the AHCI controller
> > finishes executing the port wakeup command. This patch changes the
>
> Is there anything ahci specific about this? There shouldn't be.
>
> > This patch only changes the behavior of the resume callback, not restore,
> > thaw, or runtime-resume. This is because thaw and restore are used after a
> > suspend-to-disk, which means that an image needs to be read from swap and
> > reloaded into RAM. The swap disk will always need to be fully restored/thawed
> > in order for resume to continue.
>
> If the system has multiple devices, wouldn't that still reduce the
> latency? Do we really need to deviate behavior among different
> resume/unfreeze paths?
I see your point, why have two paths if one will do. The only thing that
worries me is that the PM resume from hibernate function doesn't have
an error handler. What happens when it tries to read the image from swap
and the disk is still spinning up? The scsi layer has an error handler so
it just keeps retrying every few seconds, but the PM core reads directly
from the swap disk's block device.
>
> > The return value from ata_resume_async is now an indicator of whether
> > the resume was initiated, rather than if it was completed. I'm letting the ata
> > driver assume control over its own error reporting in this case (which it does
> > already). If you look at the ata_port resume code you'll see that the
> > ata_port_resume callback returns the status of the ahci_port_resume callback,
> > which is always 0. So I don't see any harm in ignoring it.
>
> I've been always kinda doubtful about the usefulness of resume return
> code. It's not like there's anything resume path can do differently
> upon being notified that resume of a device failed. Just reporting
> success and deal with error conditions as usual should work fine,
> right?
>
> Well, in fact, I don't think even the return code of suspend is
> useful. Failing suspend is most likely wrong. The only action PM
> core can take is aborting suspend and AFAICS none of the libata
> drivers has suspend failure conditions whose recoverability is made
> worse by just proceeding with suspend. The hardware is recycled after
> all anyway. The device is inoperational anyway, so aborting suspend
> doens't buy us anything other than failing suspend, which is almost
> always wrong. So, I'd actually prefer just removing all returns from
> suspend/resume operations, but yeah this might be out of scope for
> this series.
I think the only potential value of the return is to tell the PM core that a
suspend or resume can't be initiated at all. But since that case never
actually comes up in the ata code, it would appear to be essentially useless.
>
> > +static int ata_port_resume_async(struct device *dev)
> > +{
> > + int rc;
> > +
> > + rc = ata_port_resume_common(dev, PMSG_RESUME, true);
> > + if (!rc) {
> > + pm_runtime_disable(dev);
> > + pm_runtime_set_active(dev);
> > + pm_runtime_enable(dev);
> > + }
> > +
> > + return rc;
> > }
>
> So, can't just everything become async? Are there cases where we
> *need* synchronous PM behaviors?
I think suspend still needs to be synchronous, because the PM core needs
to be sure that the disks are actually spun down before it can attempt
to shut the system down. I'm adding Raphael to the thread. Raphael, is
this correct?
>
> Thanks.
>
> --
> tejun
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH/RESEND v2 1/2] Hard disk S3 resume time optimization
2014-01-13 19:55 ` Todd E Brandt
@ 2014-01-13 20:30 ` Tejun Heo
2014-01-15 0:32 ` [PATCH v3 2/2] " Todd E Brandt
0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2014-01-13 20:30 UTC (permalink / raw)
To: Todd E Brandt; +Cc: linux-ide, linux-scsi, James.Bottomley, rjw
Hello,
On Mon, Jan 13, 2014 at 11:55:44AM -0800, Todd E Brandt wrote:
> I see your point, why have two paths if one will do. The only thing that
> worries me is that the PM resume from hibernate function doesn't have
> an error handler. What happens when it tries to read the image from swap
> and the disk is still spinning up? The scsi layer has an error handler so
The request gets blocked on EH.
> it just keeps retrying every few seconds, but the PM core reads directly
> from the swap disk's block device.
Why would that matter? Resume is handled by EH. While EH is in
progress, all commands are blocked. Am I missing something?
> > So, can't just everything become async? Are there cases where we
> > *need* synchronous PM behaviors?
>
> I think suspend still needs to be synchronous, because the PM core needs
> to be sure that the disks are actually spun down before it can attempt
> to shut the system down. I'm adding Raphael to the thread. Raphael, is
> this correct?
Yeah, we definitely should wait for suspend to complete before
entering suspend state. I was referring to the resume path.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3 2/2] Hard disk S3 resume time optimization
2014-01-13 20:30 ` Tejun Heo
@ 2014-01-15 0:32 ` Todd E Brandt
0 siblings, 0 replies; 4+ messages in thread
From: Todd E Brandt @ 2014-01-15 0:32 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-ide, linux-scsi, James.Bottomley, rjw
On resume, the SD driver currently waits until the block driver finishes
executing a disk start command with blk_execute_rq. This patch changes
the sd_resume callback to use blk_execute_rq_nowait instead, which allows
it to return immediately, thus allowing the next device in the pm queue
to resume. The return value of blk_execute_rq_nowait is handled in the
background by sd_resume_complete. Any commands issued to the scsi disk
during the startup will be queued up and executed once the disk is online.
Thus no information is lost.
This patch applies to all three resume callbacks: resume, restore, and
runtime-resume. There is only a performance benefit for resume, but for
simplicity both restore and runtime-resume use the same code path.
The execution flow has changed from a single, synchronous call, to two
calls: one called to start sd_resume asynchronously, the other called on
completion. Thus the dmesg log will now show two prints for each drive
resume, on resume start and complete. The scsi sense data is managed
the same, but is now analyzed and freed in sd_resume_complete.
This code copies a portion of sd_start_stop_device, scsi_execute_req_flags,
and scsi_execute directly into sd_resume: effectively circumventing
sd_start_stop_device to start disks. This is to enable only the START_STOP
command to use blk_execute_rq_nowait instead of blk_execute_rq. So
sd_start_stop_device is now only used to stop the device. The disk is
started from within the sd_resume call itself. Another approach might be to
create an async version of scsi_execute to better preserve the code path
but I'm reticent to allow any other scsi commands to execute asynchronously.
Signed-off-by: Todd Brandt <todd.e.brandt@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
drivers/scsi/sd.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 69725f7..c780f75 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3172,18 +3172,82 @@ static int sd_suspend_runtime(struct device *dev)
return sd_suspend_common(dev, false);
}
+static void sd_resume_complete(struct request *rq, int error)
+{
+ struct scsi_sense_hdr sshdr;
+ struct scsi_disk *sdkp = rq->end_io_data;
+ char *sense = rq->sense;
+
+ if (error) {
+ sd_printk(KERN_WARNING, sdkp, "START FAILED\n");
+ sd_print_result(sdkp, error);
+ if (sense && (driver_byte(error) & DRIVER_SENSE)) {
+ scsi_normalize_sense(sense,
+ SCSI_SENSE_BUFFERSIZE, &sshdr);
+ sd_print_sense_hdr(sdkp, &sshdr);
+ }
+ } else {
+ sd_printk(KERN_NOTICE, sdkp, "START SUCCESS\n");
+ }
+
+ kfree(sense);
+ rq->sense = NULL;
+ rq->end_io_data = NULL;
+ __blk_put_request(rq->q, rq);
+ scsi_disk_put(sdkp);
+}
+
static int sd_resume(struct device *dev)
{
+ unsigned char cmd[6] = { START_STOP };
struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
+ struct request *req;
+ char *sense = NULL;
int ret = 0;
if (!sdkp->device->manage_start_stop)
- goto done;
+ goto error;
sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
- ret = sd_start_stop_device(sdkp, 1);
-done:
+ cmd[4] |= 1;
+
+ if (sdkp->device->start_stop_pwr_cond)
+ cmd[4] |= 1 << 4; /* Active or Standby */
+
+ if (!scsi_device_online(sdkp->device)) {
+ ret = -ENODEV;
+ goto error;
+ }
+
+ req = blk_get_request(sdkp->device->request_queue, 0, __GFP_WAIT);
+ if (!req) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ sense = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_NOIO);
+ if (!sense) {
+ ret = -ENOMEM;
+ goto error_sense;
+ }
+
+ req->cmd_len = COMMAND_SIZE(cmd[0]);
+ memcpy(req->cmd, cmd, req->cmd_len);
+ req->sense = sense;
+ req->sense_len = 0;
+ req->retries = SD_MAX_RETRIES;
+ req->timeout = SD_TIMEOUT;
+ req->cmd_type = REQ_TYPE_BLOCK_PC;
+ req->cmd_flags |= REQ_PM | REQ_QUIET | REQ_PREEMPT;
+
+ req->end_io_data = sdkp;
+ blk_execute_rq_nowait(req->q, NULL, req, 1, sd_resume_complete);
+ return 0;
+
+ error_sense:
+ __blk_put_request(req->q, req);
+ error:
scsi_disk_put(sdkp);
return ret;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-15 0:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20130823214340.GC11321@todd.e.brandt@intel.com>
2013-08-24 8:21 ` [PATCH v3 2/2] Hard disk S3 resume time optimization Oliver Neukum
2013-08-27 14:50 ` Brandt, Todd E
[not found] <20130823223535.GB11706@todd.e.brandt@intel.com>
2013-08-27 14:41 ` Brandt, Todd E
2014-01-08 0:56 [PATCH/RESEND v2 1/2] " Todd E Brandt
2014-01-11 19:13 ` Tejun Heo
2014-01-13 19:55 ` Todd E Brandt
2014-01-13 20:30 ` Tejun Heo
2014-01-15 0:32 ` [PATCH v3 2/2] " Todd E Brandt
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).