Re: [PATCH] ufs: core: Use link recovery when the h8 exit failure during runtime resume

[Bean Huo <huobean@gmail.com>]

On Wed, 2025-07-16 at 16:49 +0900, Seunghui Lee wrote:
> > -----Original Message-----
> > From: Seunghui Lee <sh043.lee@samsung.com>
> > Sent: Wednesday, July 16, 2025 4:01 PM
> > To: 'Bean Huo' <huobean@gmail.com>; alim.akhtar@samsung.com;
> > avri.altman@wdc.com; bvanassche@acm.org;
> > James.Bottomley@HansenPartnership.com; martin.petersen@oracle.com; linux-
> > scsi@vger.kernel.org; sdriver.sec@samsung.com
> > Subject: RE: [PATCH] ufs: core: Use link recovery when the h8 exit failure
> > during runtime resume
> > 
> > > -----Original Message-----
> > > From: Bean Huo <huobean@gmail.com>
> > > Sent: Wednesday, July 16, 2025 12:22 AM
> > > To: 이승희 <sh043.lee@samsung.com>; alim.akhtar@samsung.com;
> > > avri.altman@wdc.com; bvanassche@acm.org;
> > > James.Bottomley@HansenPartnership.com; martin.petersen@oracle.com;
> > > linux- scsi@vger.kernel.org; sdriver.sec@samsung.com
> > > Subject: Re: [PATCH] ufs: core: Use link recovery when the h8 exit
> > > failure during runtime resume
> > > 
> > > 
> > > You patched ufshcd_runtime_suspend() to return -EBUSY if
> > > eh_in_progress is true — meant to avoid suspend during error handling.
> > > I don't understand why here parent is not active?
> > 
> > After checking the RPM devices, I found that the parent device is
> > suspended due to the failure of ufshcd_wl_runtime_resume.
> > Even if we guarantee both the completion of ufshcd_runtime_suspend and the
> > error handling completion to avoid races, ufshcd_recover_pm_error can't
> > fully recover from a runtime pm failure scenario.
> > 
> > > 
> > > would like to try return -EAGAIN?
> > > 
> > > 
> > > Kind regards,
> > > Bean
> > > 
> > 
> > I've tried that as well, but it doesn't work either.
> > [  328.915154] [0:  kworker/u32:7:  941] ufs_device_wlun 0:0:0:49488:
> > runtime PM trying to activate child device 0:0:0:49488 but parent
> > (target0:0:0) is not active [  328.915156] [0:  kworker/u32:7:  941]
> > ufs_device_wlun 0:0:0:49488: ufshcd_recover_pm_error: rpm set_active ret(-
> > 16) [  328.915157] [0:  kworker/u32:7:  941] ufshcd-qcom 1d84000.ufshc:
> > ufshcd_recover_pm_error: rpm set_active ret(-11) Due to this error,
> > pm_request_resume(q->dev) for each scsi device can't execute.
> > This causes the I/O stack to become stuck.
> > 
> > This issue arises from the race condition between the runtime pm worker
> > and the error handler worker.
> > I believe it would be safer to handle recovery within the runtime pm
> > worker itself.
> > 
> > For reference, ufshcd_link_recovery is useful for the similar issue.
> > (resolving the race condition between the runtime pm worker and the SCSI
> > error handler worker) https://patchwork.kernel.org/project/linux-
> > scsi/patch/20230927033557.13801-1-peter.wang@mediatek.com/
> > 
> > I'll add Fixes tags and modify v2 as requested.
> > 
> > Thank you,
> > Seunghui Lee.
> > 
> 
> I would love to find the previous tag, but it's old code.
> And plus, it's hard to pick one commit for Fixes.
> Please understand this.
> 
> If this commit is okay, please review this commit.
> 
> Thank you,
> Seunghui Lee.



Based on my analysis, the tag might be:

commit 4db7a2360597 ("scsi: ufs: Fix concurrency of error handler and other error recovery paths"),
This commit added the error handling logic that calls ufshcd_set_link_broken() and ufshcd_schedule_eh_work()
when ret is non-zero in the ufshcd_uic_pwr_ctrl() function.

your patch adds a check for pm_op_in_progress to skip the error handler and use link recovery instead during PM operations.

Therefore, the appropriate Fixes tag for your patch would be:

Fixes: 4db7a2360597 ("scsi: ufs: Fix concurrency of error handler and other error recovery paths")

But, the problem is dd11376b9f1b ("scsi: ufs: Split the drivers/scsi/ufs directory") reorganzied the UFS code layout, 
it is true that not easy to add a proper tag, 

Or we can use both fix tags, or just the last one.

Let Bart comment on this,

Kind regards, 
Bean