Re: [PATCH 5/5] scsi: ufs: Add a sysfs attribute for triggering the UFS EH

[Bart Van Assche <bvanassche@acm.org>]

On 10/13/21 1:09 AM, Adrian Hunter wrote:
> On 13/10/2021 00:54, Bart Van Assche wrote:
>> Make it possible to test the impact of the UFS error handler on software
>> that submits SCSI commands to the UFS driver.
> 
> Are you sure this isn't better suited to debugfs?

I will convert this attribute into a debugfs attribute.

>> +	if (ufshcd_eh_in_progress(hba))
>> +		return -EBUSY;
> 
> Does it matter if ufshcd_eh_in_progress()?

The UFS error handler modifies hba->ufshcd_state and assumes that no 
other code modifies hba->ufshcd_state while the error handler is in 
progress. Hence this check.

>> +	} else {
>> +		return -EINVAL;
>> +	}
>> +
>> +	scsi_schedule_eh(hba->host);
> 
> Probably should be:
> 
> 	queue_work(hba->eh_wq, &hba->eh_work);

No. This patch is intended for Martin Petersen's 5.16/scsi-queue branch. 
The patch "Revert "scsi: ufs: Synchronize SCSI and UFS error handling"" 
has been queued on the 5.15/scsi-fixes branch only.

> However, it might be simpler to replace everything with:
> 
> 	spin_lock(hba->host->host_lock);
> 	hba->saved_err |= <something>;
> 	hba->saved_uic_err |= <something else>;
> 	ufshcd_schedule_eh_work(hba);
> 	spin_unlock(hba->host->host_lock);
> 
> Perhaps letting the user specify values to determine <something>
> and <something else>

I will look into this.

Thanks,

Bart.