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

[yangxingui <yangxingui@huawei.com>]

On 2026/7/16 16:47, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing `sas_resume_ha_no_sync()` and restoring `sas_resume_ha()` reintroduces a resume deadlock for hardware events like `PHYE_LOSS_OF_SIGNAL`.
> --
> 
> --- Patch [1]: [PATCH v3] scsi: libsas: fix HA resume deadlock and hisi_sas disk-wake race ---
> commit f62439ea23dd5d0db3e1f9ebd2572dcb77f2890b
> Author: Xingui Yang <yangxingui@huawei.com>
> 
> scsi: libsas: fix HA resume deadlock and hisi_sas disk-wake race
> 
> This patch moves the PHYE_RESUME_TIMEOUT notification after sas_drain_work()
> to fix a deadlock in the resume path. It then removes sas_resume_ha_no_sync()
> and restores the draining sas_resume_ha() in hisi_sas.
> 
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
>> index 213d5b5dea94f..8a2500993e19d 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> [ ... ]
>> @@ -5261,15 +5261,7 @@ static int _resume_v3_hw(struct device *device)
>>   		return rc;
>>   	}
>>   	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 0bec236f0fb59..c3f3d05b46dea 100644
>> --- a/drivers/scsi/libsas/sas_init.c
>> +++ b/drivers/scsi/libsas/sas_init.c
> [ ... ]
>> @@ -426,6 +426,23 @@ 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 and drain
>> +	 * pending work.
>> +	 */
>> +	scsi_unblock_requests(ha->shost);
>> +	sas_drain_work(ha);
> 
> [Severity: High]
> Could this reintroduce a resume deadlock for hardware events like
> PHYE_LOSS_OF_SIGNAL?
> 
> During host resume in _resume_v3_hw(), hardware is re-enabled via
> phys_init_v3_hw() just before calling sas_resume_ha(). If a hardware event
> fires during the 25-second wait window, the interrupt handler will queue it.
> 
> Looking at sas_defer_event() in sas_event.c, events for suspended phys are
> not deferred:
> 
> drivers/scsi/libsas/sas_event.c:sas_defer_event() {
>      ...
>      if (test_bit(SAS_HA_RESUMING, &ha->state) && !phy->suspended) {
>          ... deferred = true;
>      }
>      ...
> }
> 
> Since it isn't deferred, the event worker executes it immediately:
> sas_phye_loss_of_signal() -> sas_deform_port() -> sas_destruct_devices() ->
> sas_rphy_delete() -> device_del().
> 
> Because hisi_sas uses device links with DL_FLAG_PM_RUNTIME, device_del()
> triggers a synchronous resume (pm_runtime_get_sync()), which blocks waiting
> for the host to finish resuming.
> 
> At the same time, this sas_drain_work() call blocks waiting for the event
> worker to finish, resulting in a deadlock. Could a hardware event fire
> during this wait and cause this scenario?

The deadlock cannot occur due to a contradiction in the triggering 
conditions.

For PHYE_LOSS_OF_SIGNAL to execute synchronously during
sas_drain_work(), it must bypass sas_defer_event(), which requires
phy->suspended == 1 (since SAS_HA_RESUMING is still set during
drain). But PHYE_LOSS_OF_SIGNAL also requires an active link on that
phy to generate a hardware interrupt — and phy->suspended == 1 means
the phy was deactivated by lldd_port_deformed() during suspend, with
no active link to lose.

We have verified that removing a disk during suspend does not generate
a phy loss event — the phy is already deactivated.

During resume, if the phy came back up (suspended == 0), the event is
deferred by sas_defer_event() and does not execute during drain. If
the phy never came up (suspended == 1), there is no link and no
interrupt. Either way, the deadlock condition cannot be reached.

Thanks,
Xingui