Re: [PATCH v2] scsi: ufs: core: handle PM SSU timeout before SCSI EH

[Bart Van Assche]

On 5/28/26 4:34 AM, Hongjie Fang wrote:
> @@ -9465,23 +9498,30 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
>   static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
>   {
>   	struct ufs_hba *hba = shost_priv(scmd->device->host);
> +	int ret;
>   
> -	if (!hba->system_suspending) {
> +	if (!hba->pm_op_in_progress || scmd->device != hba->ufs_device_wlun ||
> +	    scmd->cmnd[0] != START_STOP) {
>   		/* Activate the error handler in the SCSI core. */
>   		return SCSI_EH_NOT_HANDLED;
>   	}

Please don't make the code any more complex than necessary. If a power 
management operation is in progress it is guaranteed that no other SCSI
commands are in progress. See also blk_pre_runtime_suspend() and
blk_try_enter_queue(). Hence, for the above if-condition, testing
hba->pm_op_in_progress is sufficient.

>   	/*
> -	 * If we get here we know that no TMFs are outstanding and also that
> -	 * the only pending command is a START STOP UNIT command. Handle the
> -	 * timeout of that command directly to prevent a deadlock between
> -	 * ufshcd_set_dev_pwr_mode() and ufshcd_err_handler().
> +	 * PM START STOP UNIT commands are issued while a PM operation is in
> +	 * progress. Handle such timeouts directly to avoid entering regular
> +	 * SCSI EH, which may deadlock with the PM operation and may also make
> +	 * scsi_execute_cmd() retries fail while the host is still in recovery.
>   	 */

The original comment is fine, isn't it?

> -	return scsi_host_busy(hba->host) ? SCSI_EH_RESET_TIMER : SCSI_EH_DONE;
> +	if (ret)
> +		return SCSI_EH_NOT_HANDLED;

This is wrong because it may activate the SCSI error handler for a START
STOP UNIT command. We don't want this - we want the error to be
propagated to the scsi_execute_cmd() caller.

> +	WARN_ON_ONCE(!test_bit(SCMD_STATE_COMPLETE, &scmd->state));

This is also wrong. According to Documentation/process/coding-style.rst,
WARN*() should only be used for this-should-never-happen situations. If
the above statement is reached it is almost a certainty that
SCMD_STATE_COMPLETE is not set.

The rest of this patch looks good to me.

Thanks,

Bart.