[v1] scsi: libsas: Fix SMP IO deadlock during HA resume

[PATCH] scsi: libsas: Fix SMP IO deadlock during HA resume

Posted by Xingui Yang 1 month ago

During sas_resume_ha() -> sas_drain_work(), ATA EH will trigger SMP IOs via
sas_phy_reset() -> sas_smp_phy_control() for expander-attached ATA devices.
Since ha->dev is RPM_RESUMING at that point, pm_runtime_get_sync() blocks
waiting for the resume to finish, but the resume is blocked in
sas_drain_work() waiting for the SMP IO — a deadlock.

Skip the PM get/put when SAS_HA_RESUMING is set. The hardware is already
initialized by the LLDD resume callback before sas_resume_ha() runs, so no
explicit resume is needed.

Only hisi_sas enables runtime PM among libsas LLDDs, so other drivers
(pm8001, isci, aic94xx, mvsas) are unaffected.

Fixes: 0da7ca4c4fd9 ("scsi: libsas: Resume host while sending SMP I/Os")
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 811c9eb4fef1..77ae2ae7b2c4 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -61,8 +61,17 @@ static int smp_execute_task_sg(struct domain_device *dev,
 	struct sas_internal *i =
 		to_sas_internal(dev->port->ha->shost->transportt);
 	struct sas_ha_struct *ha = dev->port->ha;
-
-	pm_runtime_get_sync(ha->dev);
+	bool skip_pm = test_bit(SAS_HA_RESUMING, &ha->state);
+
+	/*
+	 * Skip PM get/put during HA resume to avoid deadlock: the host is
+	 * RPM_RESUMING and the drain waits for this SMP IO to finish, but
+	 * pm_runtime_get_sync() would block on RPM_RESUMING. Safe because
+	 * hardware is already initialized by the LLDD before call
+	 * sas_resume_ha().
+	 */
+	if (!skip_pm)
+		pm_runtime_get_sync(ha->dev);
 	mutex_lock(&dev->ex_dev.cmd_mutex);
 	for (retry = 0; retry < 3; retry++) {
 		if (test_bit(SAS_DEV_GONE, &dev->state)) {
@@ -135,7 +144,8 @@ static int smp_execute_task_sg(struct domain_device *dev,
 		}
 	}
 	mutex_unlock(&dev->ex_dev.cmd_mutex);
-	pm_runtime_put_sync(ha->dev);
+	if (!skip_pm)
+		pm_runtime_put_sync(ha->dev);
 
 	BUG_ON(retry == 3 && task != NULL);
 	sas_free_task(task);
-- 
2.43.0

Re: [PATCH] scsi: libsas: Fix SMP IO deadlock during HA resume

Posted by yangxingui 2 weeks, 3 days ago

Hi John & Jason,

Kindly ping for review...

On 2026/8/26 11:28, Xingui Yang wrote:
> During sas_resume_ha() -> sas_drain_work(), ATA EH will trigger SMP IOs via
> sas_phy_reset() -> sas_smp_phy_control() for expander-attached ATA devices.
> Since ha->dev is RPM_RESUMING at that point, pm_runtime_get_sync() blocks
> waiting for the resume to finish, but the resume is blocked in
> sas_drain_work() waiting for the SMP IO — a deadlock.
> 
> Skip the PM get/put when SAS_HA_RESUMING is set. The hardware is already
> initialized by the LLDD resume callback before sas_resume_ha() runs, so no
> explicit resume is needed.
> 
> Only hisi_sas enables runtime PM among libsas LLDDs, so other drivers
> (pm8001, isci, aic94xx, mvsas) are unaffected.
> 
> Fixes: 0da7ca4c4fd9 ("scsi: libsas: Resume host while sending SMP I/Os")
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> ---
>   drivers/scsi/libsas/sas_expander.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 811c9eb4fef1..77ae2ae7b2c4 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -61,8 +61,17 @@ static int smp_execute_task_sg(struct domain_device *dev,
>   	struct sas_internal *i =
>   		to_sas_internal(dev->port->ha->shost->transportt);
>   	struct sas_ha_struct *ha = dev->port->ha;
> -
> -	pm_runtime_get_sync(ha->dev);
> +	bool skip_pm = test_bit(SAS_HA_RESUMING, &ha->state);
> +
> +	/*
> +	 * Skip PM get/put during HA resume to avoid deadlock: the host is
> +	 * RPM_RESUMING and the drain waits for this SMP IO to finish, but
> +	 * pm_runtime_get_sync() would block on RPM_RESUMING. Safe because
> +	 * hardware is already initialized by the LLDD before call
> +	 * sas_resume_ha().
> +	 */
> +	if (!skip_pm)
> +		pm_runtime_get_sync(ha->dev);
>   	mutex_lock(&dev->ex_dev.cmd_mutex);
>   	for (retry = 0; retry < 3; retry++) {
>   		if (test_bit(SAS_DEV_GONE, &dev->state)) {
> @@ -135,7 +144,8 @@ static int smp_execute_task_sg(struct domain_device *dev,
>   		}
>   	}
>   	mutex_unlock(&dev->ex_dev.cmd_mutex);
> -	pm_runtime_put_sync(ha->dev);
> +	if (!skip_pm)
> +		pm_runtime_put_sync(ha->dev);
>   
>   	BUG_ON(retry == 3 && task != NULL);
>   	sas_free_task(task);
>

Re: [PATCH] scsi: libsas: Fix SMP IO deadlock during HA resume

Posted by John Garry 1 week, 4 days ago

On 9/11/26 02:00, yangxingui wrote:
> Hi John & Jason,
> 
> Kindly ping for review...

What about the sashiko comment:

- [High] Bypassing PM reference counting via a global state flag creates 
a race condition where concurrent independent threads can submit SMP 
I/Os without holding a PM reference, risking premature hardware suspend.

It looks correct.

> 
> On 2026/8/26 11:28, Xingui Yang wrote:
>> During sas_resume_ha() -> sas_drain_work(), 

This is poorly described - what does during sas_resume_ha() -> 
sas_drain_work() even mean?

> ATA EH will trigger SMP 
>> IOs via
>> sas_phy_reset() -> sas_smp_phy_control() for expander-attached ATA 
>> devices.

What commands are these specifically?

>> Since ha->dev is RPM_RESUMING at that point, pm_runtime_get_sync() blocks
>> waiting for the resume to finish, but the resume is blocked in
>> sas_drain_work() waiting for the SMP IO — a deadlock.
>>
>> Skip the PM get/put when SAS_HA_RESUMING is set. The hardware is already
>> initialized by the LLDD resume callback before sas_resume_ha() runs, 
>> so no
>> explicit resume is needed.
>>
>> Only hisi_sas enables runtime PM among libsas LLDDs, so other drivers
>> (pm8001, isci, aic94xx, mvsas) are unaffected.
>>
>> Fixes: 0da7ca4c4fd9 ("scsi: libsas: Resume host while sending SMP I/Os")
>> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
>> ---
>>   drivers/scsi/libsas/sas_expander.c | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/ 
>> sas_expander.c
>> index 811c9eb4fef1..77ae2ae7b2c4 100644
>> --- a/drivers/scsi/libsas/sas_expander.c
>> +++ b/drivers/scsi/libsas/sas_expander.c
>> @@ -61,8 +61,17 @@ static int smp_execute_task_sg(struct domain_device 
>> *dev,
>>       struct sas_internal *i =
>>           to_sas_internal(dev->port->ha->shost->transportt);
>>       struct sas_ha_struct *ha = dev->port->ha;
>> -
>> -    pm_runtime_get_sync(ha->dev);
>> +    bool skip_pm = test_bit(SAS_HA_RESUMING, &ha->state);
>> +
>> +    /*
>> +     * Skip PM get/put during HA resume to avoid deadlock: the host is
>> +     * RPM_RESUMING and the drain waits for this SMP IO to finish, but
>> +     * pm_runtime_get_sync() would block on RPM_RESUMING. Safe because
>> +     * hardware is already initialized by the LLDD before call
>> +     * sas_resume_ha().
>> +     */
>> +    if (!skip_pm)
>> +        pm_runtime_get_sync(ha->dev);
>>       mutex_lock(&dev->ex_dev.cmd_mutex);
>>       for (retry = 0; retry < 3; retry++) {
>>           if (test_bit(SAS_DEV_GONE, &dev->state)) {
>> @@ -135,7 +144,8 @@ static int smp_execute_task_sg(struct 
>> domain_device *dev,
>>           }
>>       }
>>       mutex_unlock(&dev->ex_dev.cmd_mutex);
>> -    pm_runtime_put_sync(ha->dev);
>> +    if (!skip_pm)
>> +        pm_runtime_put_sync(ha->dev);
>>       BUG_ON(retry == 3 && task != NULL);
>>       sas_free_task(task);
>>

Re: [PATCH] scsi: libsas: Fix SMP IO deadlock during HA resume

Posted by yangxingui 1 week, 3 days ago

Hi John

Thank you for the review.

On 2026/9/16 21:58, John Garry wrote:
> On 9/11/26 02:00, yangxingui wrote:
>> Hi John & Jason,
>>
>> Kindly ping for review...
> 
> What about the sashiko comment:
> 
> - [High] Bypassing PM reference counting via a global state flag creates 
> a race condition where concurrent independent threads can submit SMP 
> I/Os without holding a PM reference, risking premature hardware suspend.
> 
> It looks correct.

Agreed. Although the possibility is very slim.
The SMP IO which sas_drain_work() waits on cannot outlive the
SAS_HA_RESUMING window, but a BSG request submitted during the window
takes no PM reference for the duration of its IO, and the controller
may autosuspend underneath it. v2 takes the reference with
pm_runtime_get_noresume()/pm_runtime_put_noidle() instead: the usage
counter stays elevated for the IO duration, without waiting for the
already ongoing resume.

> 
>>
>> On 2026/8/26 11:28, Xingui Yang wrote:
>>> During sas_resume_ha() -> sas_drain_work(), 
> 
> This is poorly described - what does during sas_resume_ha() -> 
> sas_drain_work() even mean?

The deadlock happens during a runtime resume, while
sas_resume_ha() is inside sas_drain_work(), flushing the event and
discovery workqueues. That flush includes the DISCE_RESUME work queued
when the phys come back up: sas_resume_devices() -> sas_resume_sata()
resumes the ATA ports through the libata error handler and waits for
it in sas_ata_flush_pm_eh(). So the resume is blocked in
sas_drain_work() waiting on that error handling - and the error
handling's reset of the expander-attached ATA device blocks on the
host being RPM_RESUMING, i.e. on the very resume which is waiting on
it.

> 
>> ATA EH will trigger SMP
>>> IOs via
>>> sas_phy_reset() -> sas_smp_phy_control() for expander-attached ATA 
>>> devices.
> 
> What commands are these specifically?

SMP PHY CONTROL (hard reset or link reset) commands, sent to the
expander which owns the phy of the ATA device being recovered:

   ata_eh_recover() -> ata_eh_reset() -> sas_ata_hard_reset()
     -> lldd_I_T_nexus_reset() -> sas_phy_reset()
       -> sas_smp_phy_control() -> smp_execute_task_sg()

A directly-attached device would take the lldd_control_phy() path
instead, which is why only the EXP topology deadlocks.

Thanks,
Xingui
.