Re: [PATCH 8/8] scsi: ufs: Fix deadlock between power management and error handler

Bart Van Assche
2022-09-27T17:54:38Z

On 9/27/22 10:06, Adrian Hunter wrote:
>> + /*
>> + * Serialize suspend/resume and error handling because a deadlock
>> + * occurs if the error handler runs concurrently with
>> + * ufshcd_set_dev_pwr_mode().
>> + */
>> + if (mutex_trylock(&system_transition_mutex)) {
>
> This is effectively disabling the UFS driver's error handler work.
> It would be better to add the ability to do that explicitly within
> the UFS driver and avoid using system_transition_mutex.

I will modify this patch such that system_transition_mutex is not used.

>> +static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
>> +{
>> + struct ufs_hba *hba = shost_priv(scmd->device->host);
>> +
>> + if (!hba->system_suspending) {
>
> Is a PM notifier needed - couldn't hba->system_suspending
> have been set in ufshcd_wl_suspend() ?

I will look into this.

> Doesn't resume have the same issue ?

The member variable "system_suspending" covers both suspending and
resuming. Do you perhaps want me to rename it into
system_suspending_or_resuming?

>> + /*
>> + * Calling the error handler directly when suspending or
>> + * resuming the system since the callers of this function hold
>> + * hba->host_sem in that case.
>
> Runtime PM doesn't hold host_sem

I will look into whether the deadlock avoidance mechanism needs to be
extended to runtime PM.

Thanks,

Bart.