Re: [PATCH 1/2] Add manage_restart device attribute to scsi_disk

Damien Le Moal
2025-10-06T00:58:20Z

On 10/6/25 04:06, Markus Probst wrote:
> there is already manage_shutdown, manage_system_start_stop and
> manage_runtime_start_stop device attributes that allows the high-level
> device driver (sd) manage the device power state, expect for the
> system_state SYSTEM_RESTART. With this device attribute, it is possible to
> let the high-level device driver (sd) manage the device power state for
> SYSTEM_RESTART too.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> drivers/scsi/sd.c | 35 ++++++++++++++++++++++++++++++++++-
> include/scsi/scsi_device.h | 6 ++++++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 5b8668accf8e..a3e9c2e9d9f4 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -318,6 +318,36 @@ static ssize_t manage_shutdown_store(struct device *dev,
> }
> static DEVICE_ATTR_RW(manage_shutdown);
>
> +static ssize_t manage_restart_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct scsi_disk *sdkp = to_scsi_disk(dev);
> + struct scsi_device *sdp = sdkp->device;

sdp is not really needed.

> +
> + return sysfs_emit(buf, "%u\n", sdp->manage_restart);
> +}
> +
> +
> +static ssize_t manage_restart_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;

Same here.

> + bool v;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + if (kstrtobool(buf, &v))
> + return -EINVAL;
> +
> + sdp->manage_restart = v;
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(manage_restart);
> +
> static ssize_t
> allow_restart_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> @@ -654,6 +684,7 @@ static struct attribute *sd_disk_attrs[] = {
> &dev_attr_manage_system_start_stop.attr,
> &dev_attr_manage_runtime_start_stop.attr,
> &dev_attr_manage_shutdown.attr,
> + &dev_attr_manage_restart.attr,
> &dev_attr_protection_type.attr,
> &dev_attr_protection_mode.attr,
> &dev_attr_app_tag_own.attr,
> @@ -4175,7 +4206,9 @@ static void sd_shutdown(struct device *dev)
> (system_state == SYSTEM_POWER_OFF &&
> sdkp->device->manage_shutdown) ||
> (system_state == SYSTEM_RUNNING &&
> - sdkp->device->manage_runtime_start_stop)) {
> + sdkp->device->manage_runtime_start_stop) ||
> + (system_state == SYSTEM_RESTART &&
> + sdkp->device->manage_restart)) {
> sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
> sd_start_stop_device(sdkp, 0);
> }
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 6d6500148c4b..c7e657ac8b6d 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -178,6 +178,12 @@ struct scsi_device {
> */
> unsigned manage_shutdown:1;
>
> + /*
> + * If true, let the high-level device driver (sd) manage the device
> + * power state for system restart (reboot) operations.

What about cold boot ? Same is needed, no ? The name "manage_restart" is a bit
confusing since we already have manage_system_start_stop,
manage_runtime_start_stop and manage_shutdown. I do not have a better suggestion
though.

> + */
> + unsigned manage_restart:1;
> +
> /*
> * If set and if the device is runtime suspended, ask the high-level
> * device driver (sd) to force a runtime resume of the device.

--
Damien Le Moal
Western Digital Research