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

["hoyoung seo" <hy50.seo@samsung.com>]

Hi Peter,

Thank you for the review, and for the Reviewed-by on patch 1. I will add
it. Note that I have already posted v7, which reorders the series
following Bart's comments:
https://lore.kernel.org/linux-scsi/cover.1793943246.git.hy50.seo@samsung.com/

On your question about making __ufshcd_wl_resume() trigger the error
handler and wait for it instead of calling ufshcd_link_recovery()
directly: I think that is a different design from the one in this
series, and I see a few problems with it. I may be missing something.

 - While the system PM callbacks run, host_sem is held from
   ufshcd_wl_suspend() until ufshcd_wl_resume() returns. If
   __ufshcd_wl_resume() waited for the error handler, the error handler
   could not wait for host_sem (that would be a deadlock), so it would
   have to keep taking the early link recovery path during system
   resume. This series does the opposite, as Bart suggested: for system
   PM the error handler waits for host_sem, and the early path is only
   used for runtime PM.

 - The link recovery was put into the resume path on purpose. Commit
   35dabf4503b9 ("scsi: ufs: core: Use link recovery when h8 exit fails
   during runtime resume") says that the error handler got stuck when the
   h8 exit failed during runtime resume, and commit 62c015373e1c moved it
   to __ufshcd_wl_resume(). As far as I can see this is because
   ufshcd_err_handling_prepare() waits for the runtime PM operation that
   is in progress. Waiting for the error handler from the resume path
   would bring that dependency back.

 - __ufshcd_wl_resume() needs to know whether the recovery succeeded to
   decide how to continue, while the error handler returns no result, so
   it would have to look at the HBA state afterwards. It would also need
   to avoid queueing the error handler again if it is already running,
   similar to what __ufshcd_wl_suspend() does with
   ufshcd_eh_in_progress(), to prevent a second recovery.

 - Other callers of ufshcd_link_recovery() (ufshcd_eh_timed_out(),
   ufshcd_eh_host_reset_handler(), ufs_mtk_resume()) would still call it
   directly.

For these reasons I would prefer to keep the direct call in the resume
path and fix the race on the error handler side, as in this series. If
you see a way to wait for the error handler without the runtime resume
dependency, I am happy to look at it as a follow-up.

Thanks,
Hoyoung

> -----Original Message-----
> From: Peter Wang <peter.wang@mediatek.com>
> Sent: Friday, November 6, 2026 4:11 PM
> To: HOYOUNG SEO <hy50.seo@samsung.com>; linux-scsi@vger.kernel.org; linux-
> kernel@vger.kernel.org; bvanassche@acm.org; mkp@kernel.org;
> martin.petersen@oracle.com; stanleyjhu@google.com; alim.akhtar@samsung.com;
> kwangwon.min@samsung.com; kwmad.kim@samsung.com; cpgs@samsung.com;
> h10.kim@samsung.com
> Subject: Re: [PATCH v6 2/3] scsi: ufs: core: Limit early link recovery in
> the error handler to runtime PM
> 
> On Fri, 2026-11-06 at 11:15 +0900, HOYOUNG SEO wrote:
> > ufshcd_err_handler() recovers the link without taking hba->host_sem
> > whenever hba->pm_op_in_progress is set. That early path was added by
> > commit f966e02ae521 ("scsi: ufs: core: Fix runtime suspend error
> > deadlock") because the regular error handling would wait for a runtime
> > PM operation that cannot complete while the error handler is waiting.
> >
> > pm_op_in_progress is also set during system suspend, system resume and
> > shutdown, where the early path is not needed and runs concurrently
> > with the PM path itself. For example, if a LINERESET is reported and
> > the
> > hibern8 exit fails in __ufshcd_wl_resume() during system resume, both
> > ufshcd_err_handler() and __ufshcd_wl_resume() call
> > ufshcd_link_recovery(), and the host and the device may be reset at
> > the same time.
> >
> 
> Hi HOYOUNG,
> 
> Since ufshcd_err_handler already calls ufshcd_link_recovery, wouldn’t it
> be better if __ufshcd_wl_resume just triggers the err handler and waits
> for the kworker to finish, rather than calling ufshcd_link_recovery
> directly?
> 
> Thanks
> Peter