On 7/17/24 01:11, Johan Hovold wrote:
This reverts commit 7a6bbc2829d4ab592c7e440a6f6f5deb3cd95db4.
The offending commit tried to suppress a double "Starting disk" message for some drivers, but instead started spamming the log with bogus messages every five seconds:
[ 311.798956] sd 0:0:0:0: [sda] Starting disk [ 316.919103] sd 0:0:0:0: [sda] Starting disk [ 322.040775] sd 0:0:0:0: [sda] Starting disk [ 327.161140] sd 0:0:0:0: [sda] Starting disk [ 332.281352] sd 0:0:0:0: [sda] Starting disk [ 337.401878] sd 0:0:0:0: [sda] Starting disk [ 342.521527] sd 0:0:0:0: [sda] Starting disk [ 345.850401] sd 0:0:0:0: [sda] Starting disk [ 350.967132] sd 0:0:0:0: [sda] Starting disk [ 356.090454] sd 0:0:0:0: [sda] Starting disk ...
on machines that do not actually stop the disk on runtime suspend (e.g. the Qualcomm sc8280xp CRD with UFS).
This is odd. If the disk is not being being suspended, why does the platform even enable runtime PM for it ? Are you sure about this ? Or is it simply that the runtime pm timer is set to a very low interval ?
It almost sound like what we need to do here is suppress this message for the runtime resume case, so something like:
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 2e933fd1de70..4261128bf1f3 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -4220,7 +4220,8 @@ static int sd_resume_common(struct device *dev, bool runtime) if (!sdkp) /* E.g.: runtime resume at the start of sd_probe() */ return 0;
- sd_printk(KERN_NOTICE, sdkp, "Starting disk\n"); + if (!runtime) + sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
if (!sd_do_start_stop(sdkp->device, runtime)) { sdkp->suspended = false;
However, I would like to make sure that this platform is not calling sd_resume_runtime() for nothing every 5s. If that is the case, then there is a more fundamental problem here and reverting this patch is only hiding that.
Let's just revert for now to address the regression.
Fixes: 7a6bbc2829d4 ("scsi: sd: Do not repeat the starting disk message") Cc: stable@vger.kernel.org Signed-off-by: Johan Hovold johan+linaro@kernel.org
drivers/scsi/sd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Hi,
I just noticed this regression that snuck into 6.10-final and tracked it down to 7a6bbc2829d4 ("scsi: sd: Do not repeat the starting disk message").
I wanted to get this out ASAP to address the immediate regression while someone who cares enough can work out a proper fix for the double start message (which seems less annoying).
Note that the offending commit is marked for stable.
Johan
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 1b7561abe05d..6b64af7d4927 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -4119,6 +4119,8 @@ static int sd_resume(struct device *dev) { struct scsi_disk *sdkp = dev_get_drvdata(dev);
- sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
- if (opal_unlock_from_suspend(sdkp->opal_dev)) { sd_printk(KERN_NOTICE, sdkp, "OPAL unlock failed\n"); return -EIO;
@@ -4135,13 +4137,12 @@ static int sd_resume_common(struct device *dev, bool runtime) if (!sdkp) /* E.g.: runtime resume at the start of sd_probe() */ return 0;
- sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
- if (!sd_do_start_stop(sdkp->device, runtime)) { sdkp->suspended = false; return 0; }
- sd_printk(KERN_NOTICE, sdkp, "Starting disk\n"); ret = sd_start_stop_device(sdkp, 1); if (!ret) { sd_resume(dev);