FAILED: patch "[PATCH] scsi: sd: Do not issue commands to suspended disks on" failed to apply to 6.1-stable tree

FAILED: patch "[PATCH] scsi: sd: Do not issue commands to suspended disks on" failed to apply to 6.1-stable tree

[gregkh]

The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 99398d2070ab03d13f90b758ad397e19a65fffb0
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023100438-oblivion-stowing-20c5@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..

Possible dependencies:



thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 99398d2070ab03d13f90b758ad397e19a65fffb0 Mon Sep 17 00:00:00 2001
From: Damien Le Moal <dlemoal@kernel.org>
Date: Fri, 8 Sep 2023 17:03:15 +0900
Subject: [PATCH] scsi: sd: Do not issue commands to suspended disks on
 shutdown

If an error occurs when resuming a host adapter before the devices
attached to the adapter are resumed, the adapter low level driver may
remove the scsi host, resulting in a call to sd_remove() for the
disks of the host. This in turn results in a call to sd_shutdown() which
will issue a synchronize cache command and a start stop unit command to
spindown the disk. sd_shutdown() issues the commands only if the device
is not already runtime suspended but does not check the power state for
system-wide suspend/resume. That is, the commands may be issued with the
device in a suspended state, which causes PM resume to hang, forcing a
reset of the machine to recover.

Fix this by tracking the suspended state of a disk by introducing the
suspended boolean field in the scsi_disk structure. This flag is set to
true when the disk is suspended is sd_suspend_common() and resumed with
sd_resume(). When suspended is true, sd_shutdown() is not executed from
sd_remove().

Cc: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5a1b802d180f..83b6a3f3863b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3741,7 +3741,8 @@ static int sd_remove(struct device *dev)
 
 	device_del(&sdkp->disk_dev);
 	del_gendisk(sdkp->disk);
-	sd_shutdown(dev);
+	if (!sdkp->suspended)
+		sd_shutdown(dev);
 
 	put_disk(sdkp->disk);
 	return 0;
@@ -3872,6 +3873,9 @@ static int sd_suspend_common(struct device *dev, bool runtime)
 			ret = 0;
 	}
 
+	if (!ret)
+		sdkp->suspended = true;
+
 	return ret;
 }
 
@@ -3891,21 +3895,26 @@ static int sd_suspend_runtime(struct device *dev)
 static int sd_resume(struct device *dev, bool runtime)
 {
 	struct scsi_disk *sdkp = dev_get_drvdata(dev);
-	int ret;
+	int ret = 0;
 
 	if (!sdkp)	/* E.g.: runtime resume at the start of sd_probe() */
 		return 0;
 
-	if (!sd_do_start_stop(sdkp->device, runtime))
+	if (!sd_do_start_stop(sdkp->device, runtime)) {
+		sdkp->suspended = false;
 		return 0;
+	}
 
 	if (!sdkp->device->no_start_on_resume) {
 		sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
 		ret = sd_start_stop_device(sdkp, 1);
 	}
 
-	if (!ret)
+	if (!ret) {
 		opal_unlock_from_suspend(sdkp->opal_dev);
+		sdkp->suspended = false;
+	}
+
 	return ret;
 }
 
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 5eea762f84d1..409dda5350d1 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -131,6 +131,7 @@ struct scsi_disk {
 	u8		provisioning_mode;
 	u8		zeroing_mode;
 	u8		nr_actuators;		/* Number of actuators */
+	bool		suspended;	/* Disk is suspended (stopped) */
 	unsigned	ATO : 1;	/* state of disk ATO bit */
 	unsigned	cache_override : 1; /* temp override of WCE,RCD */
 	unsigned	WCE : 1;	/* state of disk WCE bit */

Re: FAILED: patch "[PATCH] scsi: sd: Do not issue commands to suspended disks on" failed to apply to 6.1-stable tree

[Damien Le Moal]

On 10/4/23 23:30, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 6.1-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> To reproduce the conflict and resubmit, you may use the following commands:
> 
> git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> git checkout FETCH_HEAD
> git cherry-pick -x 99398d2070ab03d13f90b758ad397e19a65fffb0
> # <resolve conflicts, build, test, etc.>
> git commit -s
> git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023100438-oblivion-stowing-20c5@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
> 
> Possible dependencies:

commit 3cc2ffe5c16dc65dfac354bc5b5bc98d3b397567

    scsi: sd: Differentiate system and runtime start/stop management

is missing, which is why this backport fails. Same for 5.15 and 5.10 failures.
We need to add that patch as well (and all the patches related to suspend/resume
from the last rc4 libata PR).
Should I prepare a series for the backport ?

> 
> 
> 
> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> From 99398d2070ab03d13f90b758ad397e19a65fffb0 Mon Sep 17 00:00:00 2001
> From: Damien Le Moal <dlemoal@kernel.org>
> Date: Fri, 8 Sep 2023 17:03:15 +0900
> Subject: [PATCH] scsi: sd: Do not issue commands to suspended disks on
>  shutdown
> 
> If an error occurs when resuming a host adapter before the devices
> attached to the adapter are resumed, the adapter low level driver may
> remove the scsi host, resulting in a call to sd_remove() for the
> disks of the host. This in turn results in a call to sd_shutdown() which
> will issue a synchronize cache command and a start stop unit command to
> spindown the disk. sd_shutdown() issues the commands only if the device
> is not already runtime suspended but does not check the power state for
> system-wide suspend/resume. That is, the commands may be issued with the
> device in a suspended state, which causes PM resume to hang, forcing a
> reset of the machine to recover.
> 
> Fix this by tracking the suspended state of a disk by introducing the
> suspended boolean field in the scsi_disk structure. This flag is set to
> true when the disk is suspended is sd_suspend_common() and resumed with
> sd_resume(). When suspended is true, sd_shutdown() is not executed from
> sd_remove().
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 5a1b802d180f..83b6a3f3863b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3741,7 +3741,8 @@ static int sd_remove(struct device *dev)
>  
>  	device_del(&sdkp->disk_dev);
>  	del_gendisk(sdkp->disk);
> -	sd_shutdown(dev);
> +	if (!sdkp->suspended)
> +		sd_shutdown(dev);
>  
>  	put_disk(sdkp->disk);
>  	return 0;
> @@ -3872,6 +3873,9 @@ static int sd_suspend_common(struct device *dev, bool runtime)
>  			ret = 0;
>  	}
>  
> +	if (!ret)
> +		sdkp->suspended = true;
> +
>  	return ret;
>  }
>  
> @@ -3891,21 +3895,26 @@ static int sd_suspend_runtime(struct device *dev)
>  static int sd_resume(struct device *dev, bool runtime)
>  {
>  	struct scsi_disk *sdkp = dev_get_drvdata(dev);
> -	int ret;
> +	int ret = 0;
>  
>  	if (!sdkp)	/* E.g.: runtime resume at the start of sd_probe() */
>  		return 0;
>  
> -	if (!sd_do_start_stop(sdkp->device, runtime))
> +	if (!sd_do_start_stop(sdkp->device, runtime)) {
> +		sdkp->suspended = false;
>  		return 0;
> +	}
>  
>  	if (!sdkp->device->no_start_on_resume) {
>  		sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
>  		ret = sd_start_stop_device(sdkp, 1);
>  	}
>  
> -	if (!ret)
> +	if (!ret) {
>  		opal_unlock_from_suspend(sdkp->opal_dev);
> +		sdkp->suspended = false;
> +	}
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 5eea762f84d1..409dda5350d1 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -131,6 +131,7 @@ struct scsi_disk {
>  	u8		provisioning_mode;
>  	u8		zeroing_mode;
>  	u8		nr_actuators;		/* Number of actuators */
> +	bool		suspended;	/* Disk is suspended (stopped) */
>  	unsigned	ATO : 1;	/* state of disk ATO bit */
>  	unsigned	cache_override : 1; /* temp override of WCE,RCD */
>  	unsigned	WCE : 1;	/* state of disk WCE bit */
> 

-- 
Damien Le Moal
Western Digital Research

Re: FAILED: patch "[PATCH] scsi: sd: Do not issue commands to suspended disks on" failed to apply to 6.1-stable tree

[Greg KH]

On Thu, Oct 05, 2023 at 08:33:14AM +0900, Damien Le Moal wrote:
> On 10/4/23 23:30, gregkh@linuxfoundation.org wrote:
> > 
> > The patch below does not apply to the 6.1-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> > 
> > To reproduce the conflict and resubmit, you may use the following commands:
> > 
> > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > git checkout FETCH_HEAD
> > git cherry-pick -x 99398d2070ab03d13f90b758ad397e19a65fffb0
> > # <resolve conflicts, build, test, etc.>
> > git commit -s
> > git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023100438-oblivion-stowing-20c5@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
> > 
> > Possible dependencies:
> 
> commit 3cc2ffe5c16dc65dfac354bc5b5bc98d3b397567
> 
>     scsi: sd: Differentiate system and runtime start/stop management
> 
> is missing, which is why this backport fails. Same for 5.15 and 5.10 failures.
> We need to add that patch as well (and all the patches related to suspend/resume
> from the last rc4 libata PR).
> Should I prepare a series for the backport ?

Please do, thanks.

greg k-h