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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: libsas: hardware access on suspended host in smp_execute_task_sg()
--

commit b6ffaa3a7e18a8e3fec2c6c49ebb1c88915b9f8d
Author: Xingui Yang <yangxingui@huawei.com>

scsi: libsas: Fix SMP IO deadlock during HA resume

smp_execute_task_sg() calls pm_runtime_get_sync() on the host before
issuing an SMP command. When that command is itself issued from the
HA resume path, the get_sync() deadlocks: it waits for the ongoing
resume (the device is RPM_RESUMING), while the resume is blocked in
sas_drain_work() waiting for that same SMP IO to complete.

> The usage reference is still required. Discovery work normally runs
> inside an event worker's PM reference, taken at
> sas_notify_port_event() notify time and held until the handler has
> flushed the disco queue. sas_rediscover_ex_phy() however requeues
> DISCE_REVALIDATE_DOMAIN from within the revalidation worker itself,
> and flush_workqueue() does not wait for work items queued during
> execution, so that chained revalidation runs with no outer PM
> reference - without the get_noresume(), its SMP could race
> autosuspend.

[Severity: High]
Does pm_runtime_get_noresume() actually prevent this race if the device
has already autosuspended?

When chained revalidation occurs in sas_discover.c:sas_revalidate_domain(),
it runs without an outer PM reference. If the host autosuspends before
smp_execute_task_sg() runs, pm_runtime_get_noresume() will only increment
the usage counter and leave the device suspended.

> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 811c9eb4fef11..26c2099c28b97 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -62,7 +62,11 @@ static int smp_execute_task_sg(struct domain_device *dev,
>  		to_sas_internal(dev->port->ha->shost->transportt);
>  	struct sas_ha_struct *ha = dev->port->ha;
>  
> -	pm_runtime_get_sync(ha->dev);
> +	/*
> +	 * Non-blocking: a sync resume here would deadlock against
> +	 * sas_drain_work() during HA resume.
> +	 */
> +	pm_runtime_get_noresume(ha->dev);

[Severity: High]
If the host has already autosuspended when this is called, will this cause
a crash or hardware fault?

Because pm_runtime_get_noresume() does not wake suspended devices,
when lldd_execute_task() is called later in this function:

drivers/scsi/libsas/sas_expander.c:smp_execute_task_sg() {
    ...
		res = i->dft->lldd_execute_task(task, GFP_KERNEL);
    ...
}

hardware commands would be dispatched to a powered-down host.

>  	mutex_lock(&dev->ex_dev.cmd_mutex);
>  	for (retry = 0; retry < 3; retry++) {
>  		if (test_bit(SAS_DEV_GONE, &dev->state)) {

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