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@xxxxxxxxxxxxxxx
> Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
> ---
> 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);
--
Damien Le Moal
Western Digital Research
