Blob Blame History Raw
From: Bart Van Assche <bvanassche@acm.org>
Date: Wed, 6 Oct 2021 14:54:53 -0700
Subject: [PATCH] scsi: core: pm: Only runtime resume if necessary
Git-commit: 9131bff6a9f1d4c6ab2d4673974a9659d8d24dc7
Patch-mainline: v5.16-rc1
References: jsc#PED-1559

The following query shows which drivers define callbacks that are called by
the power management support code in the SCSI core (scsi_pm.c):

$ git grep -nHEwA16 "$(echo $(git grep -h 'scsi_register_driver(&' |
      sed 's/.*&//;s/\..*//') | sed 's/ /|/g')" |
    grep '\.pm[[:blank:]]*=[[:blank:]]'
drivers/scsi/sd.c-620-			.pm = &sd_pm_ops,
drivers/scsi/sr.c-100-			.pm = &sr_pm_ops,
drivers/scsi/ufs/ufshcd.c-9765-		.pm = &ufshcd_wl_pm_ops,

Since unconditionally runtime resuming a device during system resume is not
necessary, remove that code. Modify the SCSI disk (sd) driver such that it
follows the same approach as the UFS driver, namely to skip system suspend
and resume for devices that are runtime suspended. The CD-ROM code does not
need to be updated since its PM callbacks do not affect the device power
state.

This patch has been tested as follows:

[ shell 1 ]

cd /sys/kernel/debug/tracing
grep -E 'blk_(pre|post)_runtime|runtime_(suspend|resume)|autosuspend_delay|pm_runtime_(get|put)' available_filter_functions |
  while read a b; do echo "$a"; done |
  grep -v __pm_runtime_resume >set_ftrace_filter
echo function > current_tracer
echo 1 > tracing_on
cat trace_pipe

[ shell 2 ]

cd /sys/block/sr0
 # Increase the event poll interval to make it easier to derive from the
 # tracing output whether runtime power actions are the result of sg_inq.
echo 30000 > events_poll_msecs
cd device/power
 # Enable runtime power management.
echo auto > control
echo 1000 > autosuspend_delay_ms
sleep 1
 # Verify in shell 1 that sr0 has been runtime suspended
sg_inq /dev/sr0
eject /dev/sr0
sg_inq /dev/sr0
 # Disable runtime power management.
echo on > control

cd /sys/block/sda/device/power
echo auto > control
echo 1000 > autosuspend_delay_ms
sleep 1
 # Verify in shell 1 that sr0 has been runtime suspended
sg_inq /dev/sda

Link: https://lore.kernel.org/r/20211006215453.3318929-4-bvanassche@acm.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/scsi_pm.c | 69 +++++++++---------------------------------
 drivers/scsi/sd.c      |  6 ++++
 2 files changed, 20 insertions(+), 55 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 50b6bad4df79..b5a858c29488 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -66,71 +66,30 @@ static int scsi_dev_type_suspend(struct device *dev,
 	return err;
 }
 
-static int scsi_dev_type_resume(struct device *dev,
-		int (*cb)(struct device *, const struct dev_pm_ops *))
-{
-	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	int err = 0;
-
-	err = cb(dev, pm);
-	scsi_device_resume(to_scsi_device(dev));
-	dev_dbg(dev, "scsi resume: %d\n", err);
-
-	if (err == 0) {
-		pm_runtime_disable(dev);
-		err = pm_runtime_set_active(dev);
-		pm_runtime_enable(dev);
-
-		/*
-		 * Forcibly set runtime PM status of request queue to "active"
-		 * to make sure we can again get requests from the queue
-		 * (see also blk_pm_peek_request()).
-		 *
-		 * The resume hook will correct runtime PM status of the disk.
-		 */
-		if (!err && scsi_is_sdev_device(dev)) {
-			struct scsi_device *sdev = to_scsi_device(dev);
-
-			blk_set_runtime_active(sdev->request_queue);
-		}
-	}
-
-	return err;
-}
-
 static int
 scsi_bus_suspend_common(struct device *dev,
 		int (*cb)(struct device *, const struct dev_pm_ops *))
 {
-	int err = 0;
+	if (!scsi_is_sdev_device(dev))
+		return 0;
 
-	if (scsi_is_sdev_device(dev)) {
-		/*
-		 * All the high-level SCSI drivers that implement runtime
-		 * PM treat runtime suspend, system suspend, and system
-		 * hibernate nearly identically. In all cases the requirements
-		 * for runtime suspension are stricter.
-		 */
-		if (pm_runtime_suspended(dev))
-			return 0;
-
-		err = scsi_dev_type_suspend(dev, cb);
-	}
-
-	return err;
+	return scsi_dev_type_suspend(dev, cb);
 }
 
 static int scsi_bus_resume_common(struct device *dev,
 		int (*cb)(struct device *, const struct dev_pm_ops *))
 {
-	if (scsi_is_sdev_device(dev)) {
-		scsi_dev_type_resume(dev, cb);
-	} else {
-		pm_runtime_disable(dev);
-		pm_runtime_set_active(dev);
-		pm_runtime_enable(dev);
-	}
-	return 0;
+	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+	int err;
+
+	if (!scsi_is_sdev_device(dev))
+		return 0;
+
+	err = cb(dev, pm);
+	scsi_device_resume(to_scsi_device(dev));
+	dev_dbg(dev, "scsi resume: %d\n", err);
+
+	return err;
 }
 
 static int scsi_bus_prepare(struct device *dev)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index fcf84e54c279..4a78ab7185eb 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3660,6 +3660,9 @@ static int sd_suspend_common(struct device *dev, bool ignore_stop_errors)
 
 static int sd_suspend_system(struct device *dev)
 {
+	if (pm_runtime_suspended(dev))
+		return 0;
+
 	return sd_suspend_common(dev, true);
 }
 
@@ -3688,6 +3691,9 @@ static int sd_resume(struct device *dev)
 
 static int sd_resume_system(struct device *dev)
 {
+	if (pm_runtime_suspended(dev))
+		return 0;
+
 	return sd_resume(dev);
 }
 
-- 
2.35.3