From 0a84486d6c1da1c2738544d8fc1b07b1d3ce046f Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 16 Nov 2021 14:31:15 -0800 Subject: scsi: core: Remove Scsi_Host.shost_dev_attr_groups Simplify the scsi_host_alloc() implementation by setting the shost_class .dev_groups member instead of copying all host attribute group pointers into the shost_dev_attr_groups[] array. Link: https://lore.kernel.org/r/20211116223115.2103031-1-bvanassche@acm.org Cc: Steffen Maier Cc: Damien Le Moal Suggested-by: Benjamin Block Reviewed-by: Damien Le Moal Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- include/scsi/scsi_host.h | 6 ------ 1 file changed, 6 deletions(-) (limited to 'include/scsi') diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index ebe059badba0..72e1a347baa6 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -691,12 +691,6 @@ struct Scsi_Host { /* ldm bits */ struct device shost_gendev, shost_dev; - /* - * The array size 3 provides space for one attribute group defined by - * the SCSI core, one attribute group defined by the SCSI LLD and one - * terminating NULL pointer. - */ - const struct attribute_group *shost_dev_attr_groups[3]; /* * Points to the transport data (if any) which is allocated -- cgit v1.2.3 From fbefe22811c3140a686e407e114789ebf328a9a2 Mon Sep 17 00:00:00 2001 From: John Garry Date: Mon, 20 Dec 2021 19:21:24 +0800 Subject: scsi: libsas: Don't always drain event workqueue for HA resume 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. Link: https://lore.kernel.org/r/1639999298-244569-2-git-send-email-chenxiang66@hisilicon.com Signed-off-by: John Garry Signed-off-by: Xiang Chen Signed-off-by: Martin K. Petersen --- 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(-) (limited to 'include/scsi') diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 0239e2b4b84f..63059fb6d9ec 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -4950,7 +4950,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); -- cgit v1.2.3 From 4ea775abbb5c50c26edbf043d5a2ae7fde407f4a Mon Sep 17 00:00:00 2001 From: Xiang Chen Date: Mon, 20 Dec 2021 19:21:33 +0800 Subject: scsi: libsas: Add flag SAS_HA_RESUMING Add a flag SAS_HA_RESUMING and use it to indicate the state of resuming the host controller. Link: https://lore.kernel.org/r/1639999298-244569-11-git-send-email-chenxiang66@hisilicon.com Reviewed-by: John Garry Signed-off-by: Xiang Chen Signed-off-by: Martin K. Petersen --- drivers/scsi/libsas/sas_init.c | 2 ++ include/scsi/libsas.h | 1 + 2 files changed, 3 insertions(+) (limited to 'include/scsi') diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c index 974c4a305ece..069e40fc8411 100644 --- a/drivers/scsi/libsas/sas_init.c +++ b/drivers/scsi/libsas/sas_init.c @@ -362,6 +362,7 @@ void sas_prep_resume_ha(struct sas_ha_struct *ha) int i; set_bit(SAS_HA_REGISTERED, &ha->state); + set_bit(SAS_HA_RESUMING, &ha->state); /* clear out any stale link events/data from the suspension path */ for (i = 0; i < ha->num_phys; i++) { @@ -443,6 +444,7 @@ static void _sas_resume_ha(struct sas_ha_struct *ha, bool drain) scsi_unblock_requests(ha->core.shost); if (drain) sas_drain_work(ha); + clear_bit(SAS_HA_RESUMING, &ha->state); /* send event PORTE_BROADCAST_RCVD to identify some new inserted * disks for expander diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h index a795a2d9e5b1..698f2032807b 100644 --- a/include/scsi/libsas.h +++ b/include/scsi/libsas.h @@ -356,6 +356,7 @@ enum sas_ha_state { SAS_HA_DRAINING, SAS_HA_ATA_EH_ACTIVE, SAS_HA_FROZEN, + SAS_HA_RESUMING, }; struct sas_ha_struct { -- cgit v1.2.3