From: Tejun Heo <tj@kernel.org>
To: Maksim Rayskiy <maksim.rayskiy@gmail.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
linux-pm@lists.linux-foundation.org, linux-scsi@vger.kernel.org
Subject: Re: [linux-pm] [RFC] Deferred disk spinup during system resume
Date: Fri, 31 Dec 2010 12:45:50 +0100 [thread overview]
Message-ID: <20101231114550.GH18831@htj.dyndns.org> (raw)
In-Reply-To: <20101231112732.GG18831@htj.dyndns.org>
On Fri, Dec 31, 2010 at 12:27:32PM +0100, Tejun Heo wrote:
> On Thu, Dec 30, 2010 at 04:49:29PM -0800, Maksim Rayskiy wrote:
> > >> When resuming from system suspend, scsi disks are being spun up which
> > >> takes quite a lot of time (5+ seconds in my case). The spinup is done
> > >> synchronously, so this time adds up to overall system resume time.
> > >> Ours is an embedded platform and we are using flash-based rootfs, so
> > >> there is no immediate need in harddrive after resume. What is much
> > >> more important for us is to minimize time-to-full-power. To speed up
> > >> resume, we would like to have an option to defer the spinup or run it
> > >> in parallel with system resume. I could not find any existing
> > >> mechanism to do the trick, but I might have missed something.
> > >>
> > >> Can anybody comment on this?
> > >
> > > Do you use asynchronous suspend/resume?
> > >
> >
> > Yes, asynchronous suspend/resume is enabled - it saves about 0.5
> > second in my case. But resume blocks anyway because disk driver is
> > waiting on sd_resume() to complete.
> >
> > I am wondering if we could let the resume proceed while spinup is
> > going on, just mark the scsi device as quiescent to block any data
> > transfers.
>
> Yeah, it was asynchronous for a while when suspend/resume were
> implemented in libata proper. It's a bit trickier to do that with sd
> doing the higher level part. Hmmm... most SATA disks spin up
> automatically anyway so disabling START_STOP from sd should do the
> trick. Does the following achieve async resume?
>
> echo 0 > /sys/block/sdX/device/scsi_disk/h:c:i:l/manage_start_stop
>
> The problem is that the above would also prevent the kernel from
> issuing STANDBY_NOW on suspend or power down. Maybe we should just
> make start_stop_xlat schedule async EH action instead.
So, something like the following. It's completely untested. Proceed
with caution.
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 17a6378..d8d2aa8 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -3252,6 +3252,48 @@ static int ata_eh_maybe_retry_flush(struct ata_device *dev)
return rc;
}
+static int ata_eh_maybe_verify(struct ata_device *dev)
+{
+ struct ata_link *link = dev->link;
+ struct ata_taskfile tf;
+ unsigned int err_mask;
+ int rc = 0;
+
+ ata_eh_about_to_do(link, dev, ATA_EH_VERIFY);
+
+ ata_tf_init(dev, &tf);
+
+ if (dev->flags & ATA_DFLAG_LBA) {
+ tf.flags |= ATA_TFLAG_LBA;
+
+ tf.lbah = 0x0;
+ tf.lbam = 0x0;
+ tf.lbal = 0x0;
+ tf.device |= ATA_LBA;
+ } else {
+ /* CHS */
+ tf.lbal = 0x1; /* sect */
+ tf.lbam = 0x0; /* cyl low */
+ tf.lbah = 0x0; /* cyl high */
+ }
+
+ tf.command = ATA_CMD_VERIFY; /* READ VERIFY */
+ tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_LBA48 | ATA_TFLAG_DEVICE;
+ tf.protocol = ATA_PROT_NODATA;
+
+ err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
+ if (err_mask) {
+ ata_dev_printk(dev, KERN_WARNING,
+ "READ_VERIFY failed Emask 0x%x\n", err_mask);
+ rc = -EIO;
+ }
+
+ /* don't retry VERIFY */
+ ata_eh_done(link, dev, ATA_EH_VERIFY);
+
+ return rc;
+}
+
/**
* ata_eh_set_lpm - configure SATA interface power management
* @link: link to configure power management
@@ -3749,6 +3791,9 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
rc = ata_eh_maybe_retry_flush(dev);
if (rc)
goto rest_fail;
+ rc = ata_eh_maybe_verify(dev);
+ if (rc)
+ goto rest_fail;
}
config_lpm:
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 66aa4be..ae8d9ca 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1276,6 +1276,7 @@ int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth,
*/
static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
{
+ struct ata_device *dev = qc->dev;
struct scsi_cmnd *scmd = qc->scsicmd;
struct ata_taskfile *tf = &qc->tf;
const u8 *cdb = scmd->cmnd;
@@ -1294,23 +1295,9 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
goto invalid_fld; /* power conditions not supported */
if (cdb[4] & 0x1) {
- tf->nsect = 1; /* 1 sector, lba=0 */
-
- if (qc->dev->flags & ATA_DFLAG_LBA) {
- tf->flags |= ATA_TFLAG_LBA;
-
- tf->lbah = 0x0;
- tf->lbam = 0x0;
- tf->lbal = 0x0;
- tf->device |= ATA_LBA;
- } else {
- /* CHS */
- tf->lbal = 0x1; /* sect */
- tf->lbam = 0x0; /* cyl low */
- tf->lbah = 0x0; /* cyl high */
- }
-
- tf->command = ATA_CMD_VERIFY; /* READ VERIFY */
+ dev->link->eh_info.dev_action[dev->devno] |= ATA_EH_VERIFY;
+ ata_port_schedule_eh(dev->link->ap);
+ goto skip;
} else {
/* Some odd clown BIOSen issue spindown on power off (ACPI S4
* or S5) causing some drives to spin up and down again.
diff --git a/include/linux/libata.h b/include/linux/libata.h
index d947b12..219d491 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -326,6 +326,7 @@ enum {
ATA_EH_HARDRESET = (1 << 2), /* meaningful only in ->prereset */
ATA_EH_RESET = ATA_EH_SOFTRESET | ATA_EH_HARDRESET,
ATA_EH_ENABLE_LINK = (1 << 3),
+ ATA_EH_VERIFY = (1 << 4), /* used to force spin up */
ATA_EH_PARK = (1 << 5), /* unload heads and stop I/O */
ATA_EH_PERDEV_MASK = ATA_EH_REVALIDATE | ATA_EH_PARK,
next prev parent reply other threads:[~2010-12-31 11:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-30 19:40 [RFC] Deferred disk spinup during system resume Maksim Rayskiy
2010-12-30 22:46 ` [linux-pm] " Rafael J. Wysocki
2010-12-31 0:49 ` Maksim Rayskiy
2010-12-31 11:27 ` Tejun Heo
2010-12-31 11:45 ` Tejun Heo [this message]
-- strict thread matches above, loose matches on Subject: below --
2011-01-07 1:17 maksim.rayskiy
2011-01-07 3:16 ` Jeff Garzik
2011-01-07 3:46 ` Maksim Rayskiy
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=20101231114550.GH18831@htj.dyndns.org \
--to=tj@kernel.org \
--cc=linux-pm@lists.linux-foundation.org \
--cc=linux-scsi@vger.kernel.org \
--cc=maksim.rayskiy@gmail.com \
--cc=rjw@sisk.pl \
/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