scsi: Do not rescan devices with a suspended queue
Damien Le Moal
maintainerPATCH 1/1 scsi: Do not rescan devices with a suspended queue
Commit ff48b37802e5 ("scsi: Do not attempt to rescan suspended devices") modified scsi_rescan_device() to avoid attempting rescanning a suspended device. However, the modification added a check to verify that a SCSI device is in the running state without checking if the device request queue (in the case of block device) is also running, thus allowing the exectuion of internal requests. Without checking the device request queue, commit ff48b37802e5 fix is incomplete and deadlocks on resume can still happen. Use blk_queue_pm_only() to check if the device request queue allows executing commands in addition to checking the SCSI device state.
--- drivers/scsi/scsi_scan.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/scsi/scsi_scan.c +6 -5
@@ -1627,12 +1627,13 @@ int scsi_rescan_device(struct scsi_device *sdev)device_lock (dev);/* -* Bail out if the device is not running. Otherwise, the rescan may -* block waiting for commands to be executed, with us holding the -* device lock. This can result in a potential deadlock in the power -* management core code when system resume is on-going. +* Bail out if the device or its queue are not running. Otherwise, +* the rescan may block waiting for commands to be executed, with us +* holding the device lock. This can result in a potential deadlock +* in the power management core code when system resume is on-going. */ -if (sdev->sdev_state !=SDEV_RUNNING ) { +if (sdev->sdev_state !=SDEV_RUNNING || +blk_queue_pm_only (sdev->request_queue)) { ret = -EWOULDBLOCK ; goto unlock; }
Signature · 1 line
2.41.0
Hi, (adding James)
Quoted 10 lines · On Wed, 4 Oct 2023 17:58:03 +0900 Damien Le Moal <[email protected]> wrote:
Commit
ff48b37802e5("scsi: Do not attempt to rescan suspended devices") modifiedscsi_rescan_device()to avoid attempting rescanning a suspended device. However, the modification added a check to verify that a SCSI device is in the running state without checking if the device request queue (in the case of block device) is also running, thus allowing the exectuion of internal requests. Without checking the device request queue, commitff48b37802e5fix is incomplete and deadlocks on resume can still happen. Useblk_queue_pm_only()to check if the device request queue allows executing commands in addition to checking the SCSI device state.
FTR this fix is still needed for rc5. Is there any chance it goes into fixes before v6.6 is final?
Petr T
Quoted 38 lines
Reported-by Petr Tesarik <[email protected]>Fixes ff48b37802e5 ("scsi: Do not attempt to rescan suspendeddevices") Cc: [email protected]
Tested-by Petr Tesarik <[email protected]>Signed-off-by Damien Le Moal <[email protected]>--- drivers/scsi/scsi_scan.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)drivers/scsi/scsi_scan.c +0
-1@@ -1627,12 +1627,13 @@ int scsi_rescan_device(struct scsi_device *sdev) device_lock(dev);/* -* Bail out if the device is not running. Otherwise, the rescan may
drivers/scsi/scsi_scan.c +0
-1-* block waiting for commands to be executed, with us holding the
drivers/scsi/scsi_scan.c +0
-1-* device lock. This can result in a potential deadlock in the power
drivers/scsi/scsi_scan.c +1
-1-* management core code when system resume is on-going. +* Bail out if the device or its queue are not running. Otherwise,
drivers/scsi/scsi_scan.c +1
-0+* the rescan may block waiting for commands to be executed, with us
drivers/scsi/scsi_scan.c +1
-0+* holding the device lock. This can result in a potential deadlock
drivers/scsi/scsi_scan.c +1
-0+* in the power management core code when system resume is on-going. */
drivers/scsi/scsi_scan.c +2
-1-if (sdev->sdev_state !=SDEV_RUNNING ) { +if (sdev->sdev_state !=SDEV_RUNNING || +blk_queue_pm_only (sdev->request_queue)) { ret = -EWOULDBLOCK ; goto unlock; }
Damien Le Moal
maintainerQuoted 16 lines · Petr Tesařík
Hi, (adding James)
Quoted 10 lines · On Wed, 4 Oct 2023 17:58:03 +0900 Damien Le Moal <[email protected]> wrote:
Commit
ff48b37802e5("scsi: Do not attempt to rescan suspended devices") modifiedscsi_rescan_device()to avoid attempting rescanning a suspended device. However, the modification added a check to verify that a SCSI device is in the running state without checking if the device request queue (in the case of block device) is also running, thus allowing the exectuion of internal requests. Without checking the device request queue, commitff48b37802e5fix is incomplete and deadlocks on resume can still happen. Useblk_queue_pm_only()to check if the device request queue allows executing commands in addition to checking the SCSI device state.
Quoted 2 lines
FTR this fix is still needed for rc5. Is there any chance it goes into fixes before v6.6 is final?
The patch is on the scsi list, not for libata. Martin will likely apply it this week.
Martin ? Please confirm !
Signature · 2 lines
Damien Le Moal Western Digital Research
Martin K. Petersen
Reviewed-byDamien,
Quoted 5 lines
Quoted 2 lines
FTR this fix is still needed for rc5. Is there any chance it goes into fixes before v6.6 is final?
The patch is on the scsi list, not for libata. Martin will likely apply it this week.
Patch looks good to me but I can't apply since the PM rework series went through your tree. My fixes branch is based on -rc1. So I'd prefer for you to take it.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Damien Le Moal
maintainerQuoted 6 lines · Martin K. Petersen
Damien,
Quoted 3 lines
Quoted 2 lines
FTR this fix is still needed for rc5. Is there any chance it goes into fixes before v6.6 is final?
Quoted 8 lines
Quoted 2 lines
The patch is on the scsi list, not for libata. Martin will likely apply it this week.
Patch looks good to me but I can't apply since the PM rework series went through your tree. My fixes branch is based on -rc1. So I'd prefer for you to take it.
Reviewed-by Martin K. Petersen <[email protected]>
OK. Thanks !
Signature · 2 lines
Damien Le Moal Western Digital Research