Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
To:     "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-scsi@vger.kernel.org, linux-doc@vger.kernel.org
From:   Simon Arlott <simon@octiron.net>
Subject: [PATCH] scsi: sd: stop SSD (non-rotational) disks before reboot
Message-ID: <499138c8-b6d5-ef4a-2780-4f750ed337d3@0882a8b5-c6c3-11e9-b005-00805fc181fe>
Date:   Wed, 17 Jun 2020 19:49:57 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

I need to use "reboot=p" on my desktop because one of the PCIe devices
does not appear after a warm boot. This results in a very cold boot
because the BIOS turns the PSU off and on.

The scsi sd shutdown process does not send a stop command to disks
before the reboot happens (stop commands are only sent for a shutdown).

The result is that all of my SSDs experience a sudden power loss on
every reboot, which is undesirable behaviour. These events are recorded
in the SMART attributes.

Avoiding a stop of the disk on a reboot is appropriate for HDDs because
they're likely to continue to be powered (and should not be told to spin
down only to spin up again) but the default behaviour for SSDs should
be changed to stop them before the reboot.

Add a "stop_before_reboot" module parameter that can be used to control
the shutdown behaviour of disks before a reboot. The default will be
to stop non-rotational disks (SSDs) only, but it can be configured to
stop all disks if it is known that power will be lost completely on a
reboot.

  sd_mod.stop_before_reboot=<integer>
    0 = never
    1 = non-rotational disks only (default)
    2 = all disks

The behaviour on shutdown is unchanged: all disks are unconditionally
stopped.

The disk I/O will be mostly quiescent at reboot time (and there is a
sync first) but this should be added to stable kernels to protect all
SSDs from unexpected power loss during a reboot by default. There is
the potential for an unexpected power loss to corrupt data depending
on the SSD model/firmware.

Cc: stable@vger.kernel.org
Signed-off-by: Simon Arlott <simon@octiron.net>
---
 Documentation/scsi/scsi-parameters.rst |  7 +++++++
 drivers/scsi/sd.c                      | 22 +++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/Documentation/scsi/scsi-parameters.rst b/Documentation/scsi/scsi-parameters.rst
index 9aba897c97ac..fd64d0d43861 100644
--- a/Documentation/scsi/scsi-parameters.rst
+++ b/Documentation/scsi/scsi-parameters.rst
@@ -101,6 +101,13 @@ parameters may be changed at runtime by the command
 			allowing boot to proceed.  none ignores them, expecting
 			user space to do the scan.
 
+	sd_mod.stop_before_reboot=
+			[SCSI] configure stop action for disks before a reboot
+			Format: <integer>
+			0 = never
+			1 = non-rotational disks only (default)
+			2 = all disks
+
 	sim710=		[SCSI,HW]
 			See header of drivers/scsi/sim710.c.
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d90fefffe31b..1cd652e037ab 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -98,6 +98,12 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_ZBC);
 
+static unsigned int stop_before_reboot = 1;
+
+module_param(stop_before_reboot, uint, 0644);
+MODULE_PARM_DESC(stop_before_reboot,
+		 "stop disks before reboot (1=non-rotational, 2=all)");
+
 #if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT)
 #define SD_MINORS	16
 #else
@@ -3576,9 +3582,19 @@ static void sd_shutdown(struct device *dev)
 		sd_sync_cache(sdkp, NULL);
 	}
 
-	if (system_state != SYSTEM_RESTART && sdkp->device->manage_start_stop) {
-		sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
-		sd_start_stop_device(sdkp, 0);
+	if (sdkp->device->manage_start_stop) {
+		bool stop_disk = (system_state != SYSTEM_RESTART);
+
+		if (stop_before_reboot > 1) { /* stop all disks */
+			stop_disk = true;
+		} else if (stop_before_reboot) { /* non-rotational only */
+			stop_disk |= blk_queue_nonrot(sdkp->disk->queue);
+		}
+
+		if (stop_disk) {
+			sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
+			sd_start_stop_device(sdkp, 0);
+		}
 	}
 }
 
-- 
2.17.1

-- 
Simon Arlott