From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753183AbbFYQte (ORCPT ); Thu, 25 Jun 2015 12:49:34 -0400 Received: from smtprelay0146.hostedemail.com ([216.40.44.146]:45924 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752115AbbFYQrN (ORCPT ); Thu, 25 Jun 2015 12:47:13 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::::,RULES_HIT:41:69:355:379:541:599:960:968:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1535:1544:1593:1594:1605:1711:1730:1747:1777:1792:2194:2199:2393:2553:2559:2562:2692:2828:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4321:4605:5007:6119:6120:6261:7901:7903:8660:9592:10004:10848:11026:11232:11473:11658:11914:12043:12294:12296:12517:12519:12555:12663:12683:12740:13138:13141:13148:13230:13231:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: work84_78fc8b4f8ef07 X-Filterd-Recvd-Size: 5813 Message-ID: <1435250830.9377.25.camel@perches.com> Subject: Re: Stop SSD from waiting for "Spinning up disk..." From: Joe Perches To: Henrique de Moraes Holschuh Cc: Jeff Chua , Linux Kernel , Christoph Hellwig , Greg Kroah-Hartman Date: Thu, 25 Jun 2015 09:47:10 -0700 In-Reply-To: <20150625162143.GA27230@khazad-dum.debian.net> References: <20150625162143.GA27230@khazad-dum.debian.net> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.12.11-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. */