On Thu, 2015-06-25 at 13:21 -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 22 Jun 2015, Jeff Chua wrote:
> > There's no need to wait for disk spin-up for USB SSD devices. This patch
>
> No, you have to, instead, wait for SSD firmware startup.
>
> And looking at the contents of sd_spinup_disk(), I don't think it is safe to
> just skip it, either. It would be be better to call it sd_start_device()...
>
> sd_spinup_disk() should be really fast on anything that properly implements
> TEST_UNIT_READY and returns "ok, I am ready" when it doesn't need further
> waits or START_STOP, etc...
>
> Anyway, if you get to see the "Spinning up disk..." printk, your unit did
> not report it was ready, and sd_spinup_disk tried to issue a START_STOP
> command to signal it to get ready for real work.
>
> There's at least one msleep(1000) in the START_STOP path, though.
It might be good to change the msleep(1000) there
to a shorter value like 100ms (or maybe less).
Perhaps something like this:
(with miscellaneous neatening)
o Remove unnecessary '.' continuation printk on delays
o Remove trailing whitespace
o Neaten whitespace and alignment
o Convert int spintime to bool and move for better alignment
o Remove "only do this at boot time" comment
o Remove unnecessary memset casts
o Move int retries declaration to inner loop
---
drivers/scsi/sd.c | 39 +++++++++++++++++----------------------
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3b2fcb4..c59ed65 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1744,22 +1744,20 @@ static void
sd_spinup_disk(struct scsi_disk *sdkp)
{
unsigned char cmd[10];
+ bool spintime = false;
unsigned long spintime_expire = 0;
- int retries, spintime;
unsigned int the_result;
struct scsi_sense_hdr sshdr;
int sense_valid = 0;
- spintime = 0;
-
- /* Spin up drives, as required. Only do this at boot time */
+ /* Spin up drives, as required. */
/* Spinup needs to be done for module loads too. */
do {
- retries = 0;
+ int retries = 0;
do {
cmd[0] = TEST_UNIT_READY;
- memset((void *) &cmd[1], 0, 9);
+ memset(&cmd[1], 0, 9);
the_result = scsi_execute_req(sdkp->device, cmd,
DMA_NONE, NULL, 0,
@@ -1777,15 +1775,15 @@ sd_spinup_disk(struct scsi_disk *sdkp)
if (the_result)
sense_valid = scsi_sense_valid(&sshdr);
retries++;
- } while (retries < 3 &&
+ } while (retries < 3 &&
(!scsi_status_is_good(the_result) ||
((driver_byte(the_result) & DRIVER_SENSE) &&
- sense_valid && sshdr.sense_key == UNIT_ATTENTION)));
+ sense_valid && sshdr.sense_key == UNIT_ATTENTION)));
if ((driver_byte(the_result) & DRIVER_SENSE) == 0) {
/* no sense, TUR either succeeded or failed
* with a status error */
- if(!spintime && !scsi_status_is_good(the_result)) {
+ if (!spintime && !scsi_status_is_good(the_result)) {
sd_print_result(sdkp, "Test Unit Ready failed",
the_result);
}
@@ -1812,7 +1810,7 @@ sd_spinup_disk(struct scsi_disk *sdkp)
sd_printk(KERN_NOTICE, sdkp, "Spinning up
disk...");
cmd[0] = START_STOP;
cmd[1] = 1; /* Return immediately */
- memset((void *) &cmd[2], 0, 8);
+ memset(&cmd[2], 0, 8);
cmd[4] = 1; /* Start spin cycle */
if (sdkp->device->start_stop_pwr_cond)
cmd[4] |= 1 << 4;
@@ -1821,11 +1819,8 @@ sd_spinup_disk(struct scsi_disk *sdkp)
SD_TIMEOUT, SD_MAX_RETRIES,
NULL);
spintime_expire = jiffies + 100 * HZ;
- spintime = 1;
+ spintime = true;
}
- /* Wait 1 second for next try */
- msleep(1000);
- printk(".");
/*
* Wait for USB flash devices with slow firmware.
@@ -1833,24 +1828,25 @@ sd_spinup_disk(struct scsi_disk *sdkp)
* occur here. It's characteristic of these devices.
*/
} else if (sense_valid &&
- sshdr.sense_key == UNIT_ATTENTION &&
- sshdr.asc == 0x28) {
+ sshdr.sense_key == UNIT_ATTENTION &&
+ sshdr.asc == 0x28) {
if (!spintime) {
spintime_expire = jiffies + 5 * HZ;
- spintime = 1;
+ spintime = true;
}
- /* Wait 1 second for next try */
- msleep(1000);
} else {
/* we don't understand the sense code, so it's
* probably pointless to loop */
- if(!spintime) {
+ if (!spintime) {
sd_printk(KERN_NOTICE, sdkp, "Unit Not
Ready\n");
sd_print_sense_hdr(sdkp, &sshdr);
}
break;
}
-
+
+ /* Wait a bit for next try */
+ msleep(100);
+
} while (spintime && time_before_eq(jiffies, spintime_expire));
if (spintime) {
@@ -1861,7 +1857,6 @@ sd_spinup_disk(struct scsi_disk *sdkp)
}
}
-
/*
* Determine whether disk supports Data Integrity Field.
*/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
