diff mbox series

[v10,08/33] scsi: Use separate buf for START_STOP in sd_spinup_disk

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

Commit Message

Mike Christie July 14, 2023, 9:33 p.m. UTC
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>
---
 drivers/scsi/sd.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

John Garry July 17, 2023, 3:45 p.m. UTC | #1
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);
Bart Van Assche July 19, 2023, 6:41 p.m. UTC | #2
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.
Bart Van Assche July 19, 2023, 6:42 p.m. UTC | #3
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 mbox series

Patch

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);