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

[yangxingui <yangxingui@huawei.com>]

On 2026/6/30 15:19, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] The `flush_workqueue(hisi_hba->wq)` placement is ineffective, failing to prevent the autosuspend race for newly discovered PHYs.
> 
> Pre-existing issues:
> - [High] PHY down events are silently dropped during resume because `HISI_SAS_RESETTING_BIT` remains set, permanently leaving libsas state out of sync.

I've sent v2 which removes the flush_workqueue() entirely, as it is 
unnecessary.

> commit ce8bc2a47295d4675bf4efa43c713662a8cd5696
> Author: Xingui Yang <yangxingui@huawei.com>
> 
> scsi: libsas: fix HA resume deadlock and hisi_sas disk-wake race
> 
> This patch resolves a deadlock during HA resume by moving the
> PHYE_RESUME_TIMEOUT notification to after sas_drain_work(). It restores
> the draining sas_resume_ha() behavior in hisi_sas and introduces a
> flush_workqueue() to prevent an autosuspend race.
> 
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
>> index 213d5b5dea94f..901f508e8be77 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
>> @@ -5262,14 +5262,8 @@ 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);
>> +	flush_workqueue(hisi_hba->wq);
> 
> [Severity: High]
> Does this flush_workqueue() placement miss newly queued discovery events?
> 
> sas_resume_ha() drains the libsas event_q and disco_q. By flushing hisi_hba->wq
> after this drain, any hisi_sas_phyup_pm_work items on hisi_hba->wq will queue
> PORTE_BYTES_DMAED events to event_q that bypass the drain.
> 
> Since hisi_sas_phyup_pm_work then calls pm_runtime_put_sync(), its PM reference
> is dropped. When _resume_v3_hw() returns, the system PM reference is also
> dropped, allowing the controller to autosuspend while the missed discovery
> works execute asynchronously and attempt to send IO to a suspended controller.
> 
> Should the flush be placed before sas_resume_ha() so the generated events are
> caught and synchronously processed by sas_drain_work()?

Given that the flush is a no-op in the normal case and the PM reference
counting already protects against the race in the late-arrival case, v2
removes the flush_workqueue() entirely and removes the corresponding
justification from the commit message.

> 
>>   	clear_bit(HISI_SAS_RESETTING_BIT, &hisi_hba->flags);
> 
> [Severity: High]
> This is a pre-existing issue, but are PHY down events silently dropped during
> resume because HISI_SAS_RESETTING_BIT remains set?
> 
> In _resume_v3_hw(), HISI_SAS_RESETTING_BIT is kept active across the
> sas_resume_ha() call. If a PHY loses its link during this time, the hardware
> interrupt handler drops the event:
> 
> drivers/scsi/hisi_sas/hisi_sas_main.c:hisi_sas_phy_down() {
> 		if (test_bit(HISI_SAS_RESETTING_BIT, &hisi_hba->flags) ||
> 		    phy->in_reset) {
> 			dev_info(dev, "ignore flutter phy%d down\n", phy_no);
> 			return;
> 		}
> }
> 
> When HISI_SAS_RESETTING_BIT is finally cleared here, it does not call
> hisi_sas_rescan_topology() to recover dropped events. Could this permanently
> leave libsas state out of sync if a PHY disconnects during the resume wait?

this is a pre-existing issue. This patch does not
introduce or worsen it: it restores the pre-fbefe22811c3140 behavior
where sas_resume_ha() (the draining variant) is used, so the
HISI_SAS_RESETTING_BIT window is the same as it was before that commit.
This issue should be addressed in a separate patch.