| Message ID | 20230614071719.6372-7-michael.christie@oracle.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: Allow scsi_execute users to control retries | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 14/06/2023 08:16, Mike Christie wrote: > If the_reset is < 0, scsi_execute_cmd will not have set the sshdr, The code change below now means that we only access for the_result > 0, so should this be "If the_reset is <= 0 ..." > so we > can't access it. shouldn't access it, I'd say. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > drivers/scsi/sd.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 84edbc0a5747..a2daa96e5c87 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -2428,11 +2428,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, > the_result = scsi_execute_cmd(sdp, cmd, REQ_OP_DRV_IN, > buffer, RC16_LEN, SD_TIMEOUT, > sdkp->max_retries, &exec_args); > - > - if (media_not_present(sdkp, &sshdr)) > - return -ENODEV; > - > if (the_result > 0) { > + if (media_not_present(sdkp, &sshdr)) > + return -ENODEV; > + > sense_valid = scsi_sense_valid(&sshdr); > if (sense_valid && > sshdr.sense_key == ILLEGAL_REQUEST &&
On 6/15/23 10:41 AM, John Garry wrote: > On 14/06/2023 08:16, Mike Christie wrote: >> If the_reset is < 0, scsi_execute_cmd will not have set the sshdr, > > The code change below now means that we only access for the_result > 0, so should this be "If the_reset is <= 0 ..." > Yeah, will fix here and the other patches. >> so we >> can't access it. > > shouldn't access it, I'd say. That's fine with me. Will fix here and the other patches. > >> >> Signed-off-by: Mike Christie <michael.christie@oracle.com> >> --- >> drivers/scsi/sd.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >> index 84edbc0a5747..a2daa96e5c87 100644 >> --- a/drivers/scsi/sd.c >> +++ b/drivers/scsi/sd.c >> @@ -2428,11 +2428,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, >> the_result = scsi_execute_cmd(sdp, cmd, REQ_OP_DRV_IN, >> buffer, RC16_LEN, SD_TIMEOUT, >> sdkp->max_retries, &exec_args); >> - >> - if (media_not_present(sdkp, &sshdr)) >> - return -ENODEV; >> - >> if (the_result > 0) { >> + if (media_not_present(sdkp, &sshdr)) >> + return -ENODEV; >> + >> sense_valid = scsi_sense_valid(&sshdr); >> if (sense_valid && >> sshdr.sense_key == ILLEGAL_REQUEST && >
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 84edbc0a5747..a2daa96e5c87 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2428,11 +2428,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, the_result = scsi_execute_cmd(sdp, cmd, REQ_OP_DRV_IN, buffer, RC16_LEN, SD_TIMEOUT, sdkp->max_retries, &exec_args); - - if (media_not_present(sdkp, &sshdr)) - return -ENODEV; - if (the_result > 0) { + if (media_not_present(sdkp, &sshdr)) + return -ENODEV; + sense_valid = scsi_sense_valid(&sshdr); if (sense_valid && sshdr.sense_key == ILLEGAL_REQUEST &&
If the_reset is < 0, scsi_execute_cmd will not have set the sshdr, so we can't access it. Signed-off-by: Mike Christie <michael.christie@oracle.com> --- drivers/scsi/sd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)