Re: [PATCH v6 2/3] scsi: ufs: core: Limit early link recovery in the error handler to runtime PM

[Bart Van Assche]

On 11/5/26 6:15 PM, HOYOUNG SEO wrote:
> Only take the early path while a runtime PM operation is in progress.
> For the other PM operations the regular error handling is safe:
>
>   - the system PM callbacks hold host_sem from ufshcd_wl_suspend() until
>     ufshcd_wl_resume() returns, so the error handler waits for them;
>   - during shutdown and poweroff, hba->shutting_down makes the error
>     handler stop.

The second claim only holds for poweroff after patch 3/3 has been
applied. With this patch applied but not patch 3/3, an error that is
reported during hibernation poweroff makes the error handler skip the
early path, acquire host_sem (ufshcd_wl_poweroff() does not hold it) and
call ufshcd_err_handling_prepare() and ufshcd_reset_and_restore()
concurrently with __ufshcd_wl_suspend(). Please move the patch that
serializes the error handler against ufshcd_wl_poweroff() in front of
this patch so that the series remains bisectable.

Additionally, during shutdown errors that occur while the final START
STOP UNIT command is being processed are no longer handled by the error
handler. Recovery then only happens through ufshcd_eh_timed_out(). I'm
fine with that, but please mention this behavior change in the patch
description.

> -		if (hba->pm_op_in_progress) {
> +		if (hba->runtime_pm_in_progress) {

runtime_pm_in_progress is modified by the runtime PM callbacks and read
by the error handler without any locking. Please use READ_ONCE() here
and WRITE_ONCE() in ufshcd_wl_runtime_suspend() and
ufshcd_wl_runtime_resume() to annotate these data races.

Thanks,

Bart.