| Message ID | 20230714213419.95492-9-michael.christie@oracle.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: Allow scsi_execute users to control retries | expand |
On 14/07/2023 22:33, Mike Christie wrote: > We currently re-use the cmd buffer for the TUR and START_STOP commands > which requires us to reset the buffer when retrying. This has us use > separate buffers for the 2 commands so we can make them const and I think > it makes it easier to handle for retries but does not add too much extra > to the stack use. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: John Garry <john.g.garry@oracle.com> > --- > drivers/scsi/sd.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 245419fe9358..f75e2d7a864c 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -2267,14 +2267,13 @@ sd_spinup_disk(struct scsi_disk *sdkp) > * Issue command to spin up drive when not ready > */ > if (!spintime) { > + /* Return immediately and start spin cycle */ > + const u8 start_cmd[10] = { START_STOP, 1, 0, 0, > + sdkp->device->start_stop_pwr_cond ? > + 0x11 : 1 }; same comment as previous patch > + > sd_printk(KERN_NOTICE, sdkp, "Spinning up disk..."); > - cmd[0] = START_STOP; > - cmd[1] = 1; /* Return immediately */ > - memset((void *) &cmd[2], 0, 8); > - cmd[4] = 1; /* Start spin cycle */ > - if (sdkp->device->start_stop_pwr_cond) > - cmd[4] |= 1 << 4; > - scsi_execute_cmd(sdkp->device, cmd, > + scsi_execute_cmd(sdkp->device, start_cmd, > REQ_OP_DRV_IN, NULL, 0, > SD_TIMEOUT, sdkp->max_retries, > &exec_args);
On 7/17/23 08:45, John Garry wrote: >> + /* Return immediately and start spin cycle */ >> + const u8 start_cmd[10] = { START_STOP, 1, 0, 0, >> + sdkp->device->start_stop_pwr_cond ? >> + 0x11 : 1 }; > > same comment as previous patch I'm not sure that using designated initializers for initializing only the first five elements will improve readability ... Thanks, Bart.
On 7/14/23 14:33, Mike Christie wrote: > We currently re-use the cmd buffer for the TUR and START_STOP commands > which requires us to reset the buffer when retrying. This has us use > separate buffers for the 2 commands so we can make them const and I think > it makes it easier to handle for retries but does not add too much extra > to the stack use. Reviewed-by: Bart Van Assche <bvanassche@acm.org>
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 245419fe9358..f75e2d7a864c 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2267,14 +2267,13 @@ sd_spinup_disk(struct scsi_disk *sdkp) * Issue command to spin up drive when not ready */ if (!spintime) { + /* Return immediately and start spin cycle */ + const u8 start_cmd[10] = { START_STOP, 1, 0, 0, + sdkp->device->start_stop_pwr_cond ? + 0x11 : 1 }; + sd_printk(KERN_NOTICE, sdkp, "Spinning up disk..."); - cmd[0] = START_STOP; - cmd[1] = 1; /* Return immediately */ - memset((void *) &cmd[2], 0, 8); - cmd[4] = 1; /* Start spin cycle */ - if (sdkp->device->start_stop_pwr_cond) - cmd[4] |= 1 << 4; - scsi_execute_cmd(sdkp->device, cmd, + scsi_execute_cmd(sdkp->device, start_cmd, REQ_OP_DRV_IN, NULL, 0, SD_TIMEOUT, sdkp->max_retries, &exec_args);