Re: [PATCH v2] scsi: libsas: fix HA resume deadlock and hisi_sas disk-wake race

[John Garry <john.g.garry@oracle.com>]

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@huawei.com>

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);
>>
>