Re: [PATCH v2] scsi: libsas: fix HA resume deadlock and hisi_sas disk-wake race
From: John Garry
Date: Mon Jul 13 2026 - 07:27:44 EST
On 13/07/2026 04:10, yangxingui wrote:
Kindly ping for review...
On 2026/7/2 11:32, Xingui Yang wrote:
Commit fbefe22811c3140 ("scsi: libsas: Don't always drain event workqueue
for HA resume") introduced sas_resume_ha_no_sync() to avoid a deadlock: the
PHYE_RESUME_TIMEOUT handler, running on the HA event workqueue, calls
sas_deform_port() -> sas_destruct_devices(), which removes SCSI devices and
waits for the host to become runtime-active. But the host cannot resume
until sas_resume_ha() -> sas_drain_work() returns, and the drain is blocked
on that very handler.
However skipping the drain reintroduces a race: hisi_sas returns from
resume before all PHY UP work and libsas discovery work finish. The
controller may then autosuspend while disks are still waking up. The disks
issue IO to a suspended controller, the IO fails, and the disks get
disabled.
Fix the deadlock at its source by moving the PHYE_RESUME_TIMEOUT
notification to after sas_drain_work(). By then the host resume is about to
complete, so device removal through device_link no longer blocks on the
resume and the cycle is broken.
With the deadlock gone, restore sas_resume_ha() (the draining variant) in
hisi_sas and remove sas_resume_ha_no_sync().
Fixes: fbefe22811c3140 ("scsi: libsas: Don't always drain event workqueue for HA resume")
Signed-off-by: Xingui Yang <yangxingui@xxxxxxxxxx>
Any idea why this problem has only been discovered after 5 years (from fbefe22811c3140 being merged)?
Is there some new test case?
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 9 +-------
drivers/scsi/libsas/sas_init.c | 32 ++++++++++++++------------
include/scsi/libsas.h | 1 -
3 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/ hisi_sas/hisi_sas_v3_hw.c
index 0687bdefcd63..c8673ae4e472 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -5263,14 +5263,7 @@ static int _resume_v3_hw(struct device *device)
}
phys_init_v3_hw(hisi_hba);
- /*
- * If a directly-attached disk is removed during suspend, a deadlock
- * may occur, as the PHYE_RESUME_TIMEOUT processing will require the
- * hisi_hba->device to be active, which can only happen when resume
- * completes. So don't wait for the HA event workqueue to drain upon
- * resume.
- */
- sas_resume_ha_no_sync(sha);
+ sas_resume_ha(sha);
clear_bit(HISI_SAS_RESETTING_BIT, &hisi_hba->flags);
dev_warn(dev, "end of resuming controller\n");
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/ sas_init.c
index 0bec236f0fb5..624850f1483d 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -410,7 +410,7 @@ static void sas_resume_insert_broadcast_ha(struct sas_ha_struct *ha)
}
}
-static void _sas_resume_ha(struct sas_ha_struct *ha, bool drain)
+static void _sas_resume_ha(struct sas_ha_struct *ha)
{
const unsigned long tmo = msecs_to_jiffies(25000);
int i;
@@ -426,6 +426,21 @@ static void _sas_resume_ha(struct sas_ha_struct *ha, bool drain)
dev_info(ha->dev, "waiting up to 25 seconds for %d phy%s to resume\n",
i, i > 1 ? "s" : "");
wait_event_timeout(ha->eh_wait_q, phys_suspended(ha) == 0, tmo);
+
+ /* all phys are back up or timed out, turn on i/o so we can
+ * flush out disks that did not return
+ */
+ scsi_unblock_requests(ha->shost);
+ sas_drain_work(ha);
+
+ /* Send PHYE_RESUME_TIMEOUT after sas_drain_work(). The handler
+ * calls sas_deform_port() -> sas_destruct_devices(), which removes
+ * SCSI devices and, for LLDDs using device_link() PM sync, waits
+ * for the host to be runtime-active. Sending it before the drain
+ * would deadlock: the drain waits for the handler, the handler
+ * waits for host resume, and host resume waits for the drain to
+ * finish.
+ */
for (i = 0; i < ha->num_phys; i++) {
struct asd_sas_phy *phy = ha->sas_phy[i];
@@ -436,12 +451,6 @@ static void _sas_resume_ha(struct sas_ha_struct *ha, bool drain)
}
}
- /* all phys are back up or timed out, turn on i/o so we can
- * flush out disks that did not return
- */
- scsi_unblock_requests(ha->shost);
- if (drain)
- sas_drain_work(ha);
clear_bit(SAS_HA_RESUMING, &ha->state);
sas_queue_deferred_work(ha);
@@ -453,17 +462,10 @@ static void _sas_resume_ha(struct sas_ha_struct *ha, bool drain)
void sas_resume_ha(struct sas_ha_struct *ha)
{
- _sas_resume_ha(ha, true);
+ _sas_resume_ha(ha);
}
EXPORT_SYMBOL(sas_resume_ha);
-/* A no-sync variant, which does not call sas_drain_ha(). */
-void sas_resume_ha_no_sync(struct sas_ha_struct *ha)
-{
- _sas_resume_ha(ha, false);
-}
-EXPORT_SYMBOL(sas_resume_ha_no_sync);
-
void sas_suspend_ha(struct sas_ha_struct *ha)
{
int i;
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 163f23c92b41..36d4cb567837 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -680,7 +680,6 @@ extern int sas_register_ha(struct sas_ha_struct *);
extern int sas_unregister_ha(struct sas_ha_struct *);
extern void sas_prep_resume_ha(struct sas_ha_struct *sas_ha);
extern void sas_resume_ha(struct sas_ha_struct *sas_ha);
-extern void sas_resume_ha_no_sync(struct sas_ha_struct *sas_ha);
extern void sas_suspend_ha(struct sas_ha_struct *sas_ha);
int sas_phy_reset(struct sas_phy *phy, int hard_reset);