| From 3cc2ffe5c16dc65dfac354bc5b5bc98d3b397567 Mon Sep 17 00:00:00 2001 |
| From: Damien Le Moal <dlemoal@kernel.org> |
| Date: Fri, 15 Sep 2023 10:02:41 +0900 |
| Subject: scsi: sd: Differentiate system and runtime start/stop management |
| |
| From: Damien Le Moal <dlemoal@kernel.org> |
| |
| commit 3cc2ffe5c16dc65dfac354bc5b5bc98d3b397567 upstream. |
| |
| The underlying device and driver of a SCSI disk may have different |
| system and runtime power mode control requirements. This is because |
| runtime power management affects only the SCSI disk, while system level |
| power management affects all devices, including the controller for the |
| SCSI disk. |
| |
| For instance, issuing a START STOP UNIT command when a SCSI disk is |
| runtime suspended and resumed is fine: the command is translated to a |
| STANDBY IMMEDIATE command to spin down the ATA disk and to a VERIFY |
| command to wake it up. The SCSI disk runtime operations have no effect |
| on the ata port device used to connect the ATA disk. However, for |
| system suspend/resume operations, the ATA port used to connect the |
| device will also be suspended and resumed, with the resume operation |
| requiring re-validating the device link and the device itself. In this |
| case, issuing a VERIFY command to spinup the disk must be done before |
| starting to revalidate the device, when the ata port is being resumed. |
| In such case, we must not allow the SCSI disk driver to issue START STOP |
| UNIT commands. |
| |
| Allow a low level driver to refine the SCSI disk start/stop management |
| by differentiating system and runtime cases with two new SCSI device |
| flags: manage_system_start_stop and manage_runtime_start_stop. These new |
| flags replace the current manage_start_stop flag. Drivers setting the |
| manage_start_stop are modifed to set both new flags, thus preserving the |
| existing start/stop management behavior. For backward compatibility, the |
| old manage_start_stop sysfs device attribute is kept as a read-only |
| attribute showing a value of 1 for devices enabling both new flags and 0 |
| otherwise. |
| |
| Fixes: 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Damien Le Moal <dlemoal@kernel.org> |
| Reviewed-by: Hannes Reinecke <hare@suse.de> |
| Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> |
| Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/ata/libata-scsi.c | 3 + |
| drivers/firewire/sbp2.c | 9 +++- |
| drivers/scsi/sd.c | 90 ++++++++++++++++++++++++++++++++++++--------- |
| include/scsi/scsi_device.h | 5 ++ |
| 4 files changed, 84 insertions(+), 23 deletions(-) |
| |
| --- a/drivers/ata/libata-scsi.c |
| +++ b/drivers/ata/libata-scsi.c |
| @@ -1106,7 +1106,8 @@ int ata_scsi_dev_config(struct scsi_devi |
| * will be woken up by ata_port_pm_resume() with a port reset |
| * and device revalidation. |
| */ |
| - sdev->manage_start_stop = 1; |
| + sdev->manage_system_start_stop = true; |
| + sdev->manage_runtime_start_stop = true; |
| sdev->no_start_on_resume = 1; |
| } |
| |
| --- a/drivers/firewire/sbp2.c |
| +++ b/drivers/firewire/sbp2.c |
| @@ -81,7 +81,8 @@ MODULE_PARM_DESC(exclusive_login, "Exclu |
| * |
| * - power condition |
| * Set the power condition field in the START STOP UNIT commands sent by |
| - * sd_mod on suspend, resume, and shutdown (if manage_start_stop is on). |
| + * sd_mod on suspend, resume, and shutdown (if manage_system_start_stop or |
| + * manage_runtime_start_stop is on). |
| * Some disks need this to spin down or to resume properly. |
| * |
| * - override internal blacklist |
| @@ -1517,8 +1518,10 @@ static int sbp2_scsi_slave_configure(str |
| |
| sdev->use_10_for_rw = 1; |
| |
| - if (sbp2_param_exclusive_login) |
| - sdev->manage_start_stop = 1; |
| + if (sbp2_param_exclusive_login) { |
| + sdev->manage_system_start_stop = true; |
| + sdev->manage_runtime_start_stop = true; |
| + } |
| |
| if (sdev->type == TYPE_ROM) |
| sdev->use_10_for_ms = 1; |
| --- a/drivers/scsi/sd.c |
| +++ b/drivers/scsi/sd.c |
| @@ -213,18 +213,32 @@ cache_type_store(struct device *dev, str |
| } |
| |
| static ssize_t |
| -manage_start_stop_show(struct device *dev, struct device_attribute *attr, |
| - char *buf) |
| +manage_start_stop_show(struct device *dev, |
| + struct device_attribute *attr, char *buf) |
| { |
| struct scsi_disk *sdkp = to_scsi_disk(dev); |
| struct scsi_device *sdp = sdkp->device; |
| |
| - return sprintf(buf, "%u\n", sdp->manage_start_stop); |
| + return sysfs_emit(buf, "%u\n", |
| + sdp->manage_system_start_stop && |
| + sdp->manage_runtime_start_stop); |
| } |
| +static DEVICE_ATTR_RO(manage_start_stop); |
| |
| static ssize_t |
| -manage_start_stop_store(struct device *dev, struct device_attribute *attr, |
| - const char *buf, size_t count) |
| +manage_system_start_stop_show(struct device *dev, |
| + struct device_attribute *attr, char *buf) |
| +{ |
| + struct scsi_disk *sdkp = to_scsi_disk(dev); |
| + struct scsi_device *sdp = sdkp->device; |
| + |
| + return sysfs_emit(buf, "%u\n", sdp->manage_system_start_stop); |
| +} |
| + |
| +static ssize_t |
| +manage_system_start_stop_store(struct device *dev, |
| + struct device_attribute *attr, |
| + const char *buf, size_t count) |
| { |
| struct scsi_disk *sdkp = to_scsi_disk(dev); |
| struct scsi_device *sdp = sdkp->device; |
| @@ -236,11 +250,42 @@ manage_start_stop_store(struct device *d |
| if (kstrtobool(buf, &v)) |
| return -EINVAL; |
| |
| - sdp->manage_start_stop = v; |
| + sdp->manage_system_start_stop = v; |
| |
| return count; |
| } |
| -static DEVICE_ATTR_RW(manage_start_stop); |
| +static DEVICE_ATTR_RW(manage_system_start_stop); |
| + |
| +static ssize_t |
| +manage_runtime_start_stop_show(struct device *dev, |
| + struct device_attribute *attr, char *buf) |
| +{ |
| + struct scsi_disk *sdkp = to_scsi_disk(dev); |
| + struct scsi_device *sdp = sdkp->device; |
| + |
| + return sysfs_emit(buf, "%u\n", sdp->manage_runtime_start_stop); |
| +} |
| + |
| +static ssize_t |
| +manage_runtime_start_stop_store(struct device *dev, |
| + struct device_attribute *attr, |
| + const char *buf, size_t count) |
| +{ |
| + struct scsi_disk *sdkp = to_scsi_disk(dev); |
| + struct scsi_device *sdp = sdkp->device; |
| + bool v; |
| + |
| + if (!capable(CAP_SYS_ADMIN)) |
| + return -EACCES; |
| + |
| + if (kstrtobool(buf, &v)) |
| + return -EINVAL; |
| + |
| + sdp->manage_runtime_start_stop = v; |
| + |
| + return count; |
| +} |
| +static DEVICE_ATTR_RW(manage_runtime_start_stop); |
| |
| static ssize_t |
| allow_restart_show(struct device *dev, struct device_attribute *attr, char *buf) |
| @@ -572,6 +617,8 @@ static struct attribute *sd_disk_attrs[] |
| &dev_attr_FUA.attr, |
| &dev_attr_allow_restart.attr, |
| &dev_attr_manage_start_stop.attr, |
| + &dev_attr_manage_system_start_stop.attr, |
| + &dev_attr_manage_runtime_start_stop.attr, |
| &dev_attr_protection_type.attr, |
| &dev_attr_protection_mode.attr, |
| &dev_attr_app_tag_own.attr, |
| @@ -3810,13 +3857,20 @@ static void sd_shutdown(struct device *d |
| sd_sync_cache(sdkp, NULL); |
| } |
| |
| - if (system_state != SYSTEM_RESTART && sdkp->device->manage_start_stop) { |
| + if (system_state != SYSTEM_RESTART && |
| + sdkp->device->manage_system_start_stop) { |
| sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n"); |
| sd_start_stop_device(sdkp, 0); |
| } |
| } |
| |
| -static int sd_suspend_common(struct device *dev, bool ignore_stop_errors) |
| +static inline bool sd_do_start_stop(struct scsi_device *sdev, bool runtime) |
| +{ |
| + return (sdev->manage_system_start_stop && !runtime) || |
| + (sdev->manage_runtime_start_stop && runtime); |
| +} |
| + |
| +static int sd_suspend_common(struct device *dev, bool runtime) |
| { |
| struct scsi_disk *sdkp = dev_get_drvdata(dev); |
| struct scsi_sense_hdr sshdr; |
| @@ -3848,12 +3902,12 @@ static int sd_suspend_common(struct devi |
| } |
| } |
| |
| - if (sdkp->device->manage_start_stop) { |
| + if (sd_do_start_stop(sdkp->device, runtime)) { |
| if (!sdkp->device->silence_suspend) |
| sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n"); |
| /* an error is not worth aborting a system sleep */ |
| ret = sd_start_stop_device(sdkp, 0); |
| - if (ignore_stop_errors) |
| + if (!runtime) |
| ret = 0; |
| } |
| |
| @@ -3865,23 +3919,23 @@ static int sd_suspend_system(struct devi |
| if (pm_runtime_suspended(dev)) |
| return 0; |
| |
| - return sd_suspend_common(dev, true); |
| + return sd_suspend_common(dev, false); |
| } |
| |
| static int sd_suspend_runtime(struct device *dev) |
| { |
| - return sd_suspend_common(dev, false); |
| + return sd_suspend_common(dev, true); |
| } |
| |
| -static int sd_resume(struct device *dev) |
| +static int sd_resume(struct device *dev, bool runtime) |
| { |
| struct scsi_disk *sdkp = dev_get_drvdata(dev); |
| - int ret = 0; |
| + int ret; |
| |
| if (!sdkp) /* E.g.: runtime resume at the start of sd_probe() */ |
| return 0; |
| |
| - if (!sdkp->device->manage_start_stop) |
| + if (!sd_do_start_stop(sdkp->device, runtime)) |
| return 0; |
| |
| if (!sdkp->device->no_start_on_resume) { |
| @@ -3899,7 +3953,7 @@ static int sd_resume_system(struct devic |
| if (pm_runtime_suspended(dev)) |
| return 0; |
| |
| - return sd_resume(dev); |
| + return sd_resume(dev, false); |
| } |
| |
| static int sd_resume_runtime(struct device *dev) |
| @@ -3926,7 +3980,7 @@ static int sd_resume_runtime(struct devi |
| "Failed to clear sense data\n"); |
| } |
| |
| - return sd_resume(dev); |
| + return sd_resume(dev, true); |
| } |
| |
| /** |
| --- a/include/scsi/scsi_device.h |
| +++ b/include/scsi/scsi_device.h |
| @@ -161,6 +161,10 @@ struct scsi_device { |
| * pass settings from slave_alloc to scsi |
| * core. */ |
| unsigned int eh_timeout; /* Error handling timeout */ |
| + |
| + bool manage_system_start_stop; /* Let HLD (sd) manage system start/stop */ |
| + bool manage_runtime_start_stop; /* Let HLD (sd) manage runtime start/stop */ |
| + |
| unsigned removable:1; |
| unsigned changed:1; /* Data invalid due to media change */ |
| unsigned busy:1; /* Used to prevent races */ |
| @@ -193,7 +197,6 @@ struct scsi_device { |
| unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */ |
| unsigned no_start_on_add:1; /* do not issue start on add */ |
| unsigned allow_restart:1; /* issue START_UNIT in error handler */ |
| - unsigned manage_start_stop:1; /* Let HLD (sd) manage start/stop */ |
| unsigned no_start_on_resume:1; /* Do not issue START_STOP_UNIT on resume */ |
| unsigned start_stop_pwr_cond:1; /* Set power cond. in START_STOP_UNIT */ |
| unsigned no_uld_attach:1; /* disable connecting to upper level drivers */ |