* [PATCH] scsi: Do not rescan devices with a suspended queue
@ 2023-10-04 8:58 Damien Le Moal
2023-10-09 6:17 ` Petr Tesařík
0 siblings, 1 reply; 5+ messages in thread
From: Damien Le Moal @ 2023-10-04 8:58 UTC (permalink / raw
To: Martin K . Petersen, linux-scsi, linux-ide; +Cc: Petr Tesarik
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.
Reported-by: Petr Tesarik <petr@tesarici.cz>
Fixes: ff48b37802e5 ("scsi: Do not attempt to rescan suspended devices")
Cc: stable@vger.kernel.org
Tested-by: Petr Tesarik <petr@tesarici.cz>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/scsi_scan.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 3db4d31a03a1..b05a55f498a2 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -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;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: Do not rescan devices with a suspended queue
2023-10-04 8:58 [PATCH] scsi: Do not rescan devices with a suspended queue Damien Le Moal
@ 2023-10-09 6:17 ` Petr Tesařík
2023-10-09 22:36 ` Damien Le Moal
0 siblings, 1 reply; 5+ messages in thread
From: Petr Tesařík @ 2023-10-09 6:17 UTC (permalink / raw
To: Martin K . Petersen, James E.J. Bottomley
Cc: Damien Le Moal, linux-scsi, linux-ide
Hi, (adding James)
On Wed, 4 Oct 2023 17:58:03 +0900
Damien Le Moal <dlemoal@kernel.org> wrote:
> 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.
FTR this fix is still needed for rc5. Is there any chance it goes into
fixes before v6.6 is final?
Petr T
> Reported-by: Petr Tesarik <petr@tesarici.cz>
> Fixes: ff48b37802e5 ("scsi: Do not attempt to rescan suspended
> devices") Cc: stable@vger.kernel.org
> Tested-by: Petr Tesarik <petr@tesarici.cz>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> drivers/scsi/scsi_scan.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 3db4d31a03a1..b05a55f498a2 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -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;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: Do not rescan devices with a suspended queue
2023-10-09 6:17 ` Petr Tesařík
@ 2023-10-09 22:36 ` Damien Le Moal
2023-10-10 1:43 ` Martin K. Petersen
0 siblings, 1 reply; 5+ messages in thread
From: Damien Le Moal @ 2023-10-09 22:36 UTC (permalink / raw
To: Petr Tesařík, Martin K . Petersen, James E.J. Bottomley
Cc: linux-scsi, linux-ide
On 10/9/23 15:17, Petr Tesařík wrote:
> Hi, (adding James)
>
> On Wed, 4 Oct 2023 17:58:03 +0900
> Damien Le Moal <dlemoal@kernel.org> wrote:
>
>> 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.
>
> 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 !
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: Do not rescan devices with a suspended queue
2023-10-09 22:36 ` Damien Le Moal
@ 2023-10-10 1:43 ` Martin K. Petersen
2023-10-10 2:23 ` Damien Le Moal
0 siblings, 1 reply; 5+ messages in thread
From: Martin K. Petersen @ 2023-10-10 1:43 UTC (permalink / raw
To: Damien Le Moal
Cc: Petr Tesařík, Martin K . Petersen, James E.J. Bottomley,
linux-scsi, linux-ide
Damien,
>> 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.
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: Do not rescan devices with a suspended queue
2023-10-10 1:43 ` Martin K. Petersen
@ 2023-10-10 2:23 ` Damien Le Moal
0 siblings, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2023-10-10 2:23 UTC (permalink / raw
To: Martin K. Petersen
Cc: Petr Tesařík, James E.J. Bottomley, linux-scsi,
linux-ide
On 10/10/23 10:43, Martin K. Petersen wrote:
>
> Damien,
>
>>> 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.
>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
OK. Thanks !
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-10 2:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-04 8:58 [PATCH] scsi: Do not rescan devices with a suspended queue Damien Le Moal
2023-10-09 6:17 ` Petr Tesařík
2023-10-09 22:36 ` Damien Le Moal
2023-10-10 1:43 ` Martin K. Petersen
2023-10-10 2:23 ` Damien Le Moal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).