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

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

On 11/9/26 6:03 PM, HOYOUNG SEO wrote:
 > +static inline bool ufshcd_rpm_transition_in_progress(struct ufs_hba 
*hba)
 > +{
 > +#ifdef CONFIG_PM
 > +	enum rpm_status status =
 > +		READ_ONCE(hba->ufs_device_wlun->sdev_gendev.power.runtime_status);
 > +
 > +	return hba->pm_op_in_progress &&
 > +	       (status == RPM_SUSPENDING || status == RPM_RESUMING);

hba->pm_op_in_progress is modified by __ufshcd_wl_suspend() and
__ufshcd_wl_resume() without holding any lock. Please use READ_ONCE()
here and WRITE_ONCE() in the functions that modify this member, or at
least make the patch description consistent with the code ("Both
values are read without locking" while only one of the two reads uses
READ_ONCE()).

Additionally, since this function has only one caller, please make it
a static function in ufshcd.c instead of adding it to ufshcd-priv.h. A
name that makes it clear that this function checks whether a runtime
PM callback of the WLUN is in progress, e.g.
ufshcd_in_wl_rpm_callback(), would be more accurate.

Otherwise this patch looks good to me. Please note that the
correctness of this patch depends on patch 2/3, which needs to be
revised.

Thanks,

Bart.