If scsi_execute_cmd returns < 0 it will not have set the sshdr, so callers of sd_sync_cache passing in a sshdr cannot access it at that time. The problem is sd_suspend_common doesn't know what scsi_execute_cmd returned due to sd_sync_cache converting errors to -EXYZ errors. The only error it cares about is ILLEGAL_REQUEST so this has sd_sync_cache convert that to a -EOPNOTSUPP which it can then check for. Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx> --- drivers/scsi/sd.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index e67a3d163b24..bb2e5885e41b 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1604,24 +1604,21 @@ static unsigned int sd_check_events(struct gendisk *disk, unsigned int clearing) return disk_changed ? DISK_EVENT_MEDIA_CHANGE : 0; } -static int sd_sync_cache(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr) +static int sd_sync_cache(struct scsi_disk *sdkp) { int retries, res; struct scsi_device *sdp = sdkp->device; const int timeout = sdp->request_queue->rq_timeout * SD_FLUSH_TIMEOUT_MULTIPLIER; - struct scsi_sense_hdr my_sshdr; + struct scsi_sense_hdr sshdr; const struct scsi_exec_args exec_args = { .req_flags = BLK_MQ_REQ_PM, - /* caller might not be interested in sense, but we need it */ - .sshdr = sshdr ? : &my_sshdr, + .sshdr = &sshdr, }; if (!scsi_device_online(sdp)) return -ENODEV; - sshdr = exec_args.sshdr; - for (retries = 3; retries > 0; --retries) { unsigned char cmd[16] = { 0 }; @@ -1646,15 +1643,19 @@ static int sd_sync_cache(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr) return res; if (scsi_status_is_check_condition(res) && - scsi_sense_valid(sshdr)) { - sd_print_sense_hdr(sdkp, sshdr); + scsi_sense_valid(&sshdr)) { + sd_print_sense_hdr(sdkp, &sshdr); /* we need to evaluate the error return */ - if (sshdr->asc == 0x3a || /* medium not present */ - sshdr->asc == 0x20 || /* invalid command */ - (sshdr->asc == 0x74 && sshdr->ascq == 0x71)) /* drive is password locked */ + if (sshdr.asc == 0x3a || /* medium not present */ + sshdr.asc == 0x20 || /* invalid command */ + (sshdr.asc == 0x74 && sshdr.ascq == 0x71)) /* drive is password locked */ /* this is no error here */ return 0; + + /* This drive doesn't support sync. */ + if (sshdr.sense_key == ILLEGAL_REQUEST) + return -EOPNOTSUPP; } switch (host_byte(res)) { @@ -3849,7 +3850,7 @@ static void sd_shutdown(struct device *dev) if (sdkp->WCE && sdkp->media_present) { sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n"); - sd_sync_cache(sdkp, NULL); + sd_sync_cache(sdkp); } if (system_state != SYSTEM_RESTART && sdkp->device->manage_start_stop) { @@ -3861,7 +3862,6 @@ static void sd_shutdown(struct device *dev) static int sd_suspend_common(struct device *dev, bool ignore_stop_errors) { struct scsi_disk *sdkp = dev_get_drvdata(dev); - struct scsi_sense_hdr sshdr; int ret = 0; if (!sdkp) /* E.g.: runtime suspend following sd_remove() */ @@ -3870,21 +3870,18 @@ static int sd_suspend_common(struct device *dev, bool ignore_stop_errors) if (sdkp->WCE && sdkp->media_present) { if (!sdkp->device->silence_suspend) sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n"); - ret = sd_sync_cache(sdkp, &sshdr); + ret = sd_sync_cache(sdkp); if (ret) { /* ignore OFFLINE device */ if (ret == -ENODEV) return 0; - if (!scsi_sense_valid(&sshdr) || - sshdr.sense_key != ILLEGAL_REQUEST) + if (ret != -EOPNOTSUPP) return ret; - /* - * sshdr.sense_key == ILLEGAL_REQUEST means this drive - * doesn't support sync. There's not much to do and - * suspend shouldn't fail. + * The drive doesn't support sync. There's not much to + * do and suspend shouldn't fail. */ ret = 0; } -- 2.25.1