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

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

[Xingui Yang]

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [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.

Pre-existing issues:
- [High] The return value of pm_runtime_get_sync() is ignored, which can lead to accessing powered-off hardware on failure.
--

commit ecfcb25c960bd684b2b9ddb5e30db0c49c3a78b4
Author: Xingui Yang <yangxingui@huawei.com>
scsi: libsas: Fix SMP IO deadlock during HA resume

This commit attempts to fix a deadlock during sas_resume_ha() by
skipping pm_runtime_get_sync() and pm_runtime_put_sync() when the
SAS_HA_RESUMING flag is set. It targets expander-attached ATA devices
triggered by ATA EH via sas_phy_reset().

> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 811c9eb4fef11..77ae2ae7b2c45 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);

[Severity: High]
Does bypassing PM reference counting via a global state flag create a race
condition here in smp_execute_task_sg() in drivers/scsi/libsas/sas_expander.c?

Concurrent independent threads could submit SMP I/Os without holding a PM
reference, risking premature hardware suspend.

For example, if a concurrent userspace SMP request via bsg executes during
sas_resume_ha(), smp_execute_task_sg() could evaluate test_bit for
SAS_HA_RESUMING to true and skip pm_runtime_get_sync(). Meanwhile,
sas_resume_ha() might unblock SCSI requests via scsi_unblock_requests() and
eventually clear SAS_HA_RESUMING, dropping the resume thread's PM reference.

Could this allow the device to autosuspend while the BSG thread is still
waiting for the SMP IO to complete, potentially causing a system crash, bus
fault, or IO timeout?

> +
> +	/*
> +	 * 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);

[Severity: High]
This isn't a bug introduced by this patch, but the return value of
pm_runtime_get_sync() is ignored here in smp_execute_task_sg().

If pm_runtime_get_sync() fails to resume the device due to a hardware fault
or disabled runtime PM, the function will blindly proceed to allocate and
issue SMP tasks to the LLDD. Accessing suspended or unclocked hardware
registers can cause unrecoverable bus faults or system lockups.

Should the negative error code be checked before proceeding to access
hardware?

>  	mutex_lock(&dev->ex_dev.cmd_mutex);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826032845.1632348-1-yangxingui@huawei.com?part=1

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

[yangxingui]

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

[John Garry]

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

[yangxingui]

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
.