diff mbox series

[01/15] libsas: Don't always drain event workqueue for HA resume

Message ID 1637117108-230103-2-git-send-email-chenxiang66@hisilicon.com (mailing list archive)
State Changes Requested
Headers show
Series Add runtime PM support for libsas | expand

Commit Message

chenxiang Nov. 17, 2021, 2:44 a.m. UTC 
From: John Garry <john.garry@huawei.com>

For the hisi_sas driver, if a directly attached disk is removed during
suspend, a hang will occur in the resume process:

The background is that in commit 16fd4a7c5917 ("scsi: hisi_sas: Add device
link between SCSI devices and hisi_hba"), it is ensured that the HBA
device cannot be runtime suspended when any SCSI device associated is
active.

Other drivers which use libsas don't worry about this as none support
runtime suspend.

The mentioned hang occurs when an disk is removed during suspend. In the
removal process - from PHYE_RESUME_TIMEOUT event processing - we call into
scsi_remove_device(), which is being processed in the HA event workqueue.
Here we wait for all suppliers of the SCSI device to resume, which
includes the HBA device (from the above commit). However the HBA device
cannot resume, as it is waiting for the PHYE_RESUME_TIMEOUT to be processed
(from calling sas_resume_ha() -> sas_drain_work()). This is the deadlock.

There does not appear to be any need for the sas_drain_work() to be called
at all in sas_resume_ha() as it is not syncing against anything, so allow
LLDDs to avoid this by providing a variant of sas_resume_ha() which does
"sync", i.e. doesn't drain the event workqueue.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 10 +++++++++-
 drivers/scsi/libsas/sas_init.c         | 17 +++++++++++++++--
 include/scsi/libsas.h                  |  1 +
 3 files changed, 25 insertions(+), 3 deletions(-)

Comments

Bart Van Assche Nov. 17, 2021, 4:14 a.m. UTC | #1 
On 11/16/21 18:44, chenxiang wrote:
> [ ... ]

Where is the cover letter of this patch series? Please always
include a cover letter with a patch series.

Thanks,

Bart.
Bart Van Assche Nov. 17, 2021, 5:06 a.m. UTC | #2 
On 11/16/21 8:14 PM, Bart Van Assche wrote:
> On 11/16/21 18:44, chenxiang wrote:
>> [ ... ]
> 
> Where is the cover letter of this patch series? Please always
> include a cover letter with a patch series.

Oops, I was too fast. Patches 01/15..15/15 arrived in my mailbox before the cover
letter. I just received the cover letter.

Thanks,

Bart.
diff mbox series

Patch

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 9ae47d1d9897..5a83373ac0b1 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -4942,7 +4942,15 @@  static int _resume_v3_hw(struct device *device)
 		return rc;
 	}
 	phys_init_v3_hw(hisi_hba);
-	sas_resume_ha(sha);
+
+	/*
+	 * 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);
 	clear_bit(HISI_SAS_RESETTING_BIT, &hisi_hba->flags);
 
 	return 0;
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index b640e09af6a4..43509d139241 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -387,7 +387,7 @@  static int phys_suspended(struct sas_ha_struct *ha)
 	return rc;
 }
 
-void sas_resume_ha(struct sas_ha_struct *ha)
+static void _sas_resume_ha(struct sas_ha_struct *ha, bool drain)
 {
 	const unsigned long tmo = msecs_to_jiffies(25000);
 	int i;
@@ -417,10 +417,23 @@  void sas_resume_ha(struct sas_ha_struct *ha)
 	 * flush out disks that did not return
 	 */
 	scsi_unblock_requests(ha->core.shost);
-	sas_drain_work(ha);
+	if (drain)
+		sas_drain_work(ha);
+}
+
+void sas_resume_ha(struct sas_ha_struct *ha)
+{
+	_sas_resume_ha(ha, true);
 }
 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 79e4903bd414..a795a2d9e5b1 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -660,6 +660,7 @@  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_set_phy_speed(struct sas_phy *phy, struct sas_phy_linkrates *rates);