Re: [PATCH v1] ufs: core: fix deadlock when rtc update

["Peter Wang (王信友)" <peter.wang@mediatek.com>]

On Fri, 2024-07-12 at 10:34 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 7/12/24 2:43 AM, peter.wang@mediatek.com wrote:
> > Three have deadlock when runtime suspend wait flush rtc work,
> > and rtc work call ufshcd_rpm_put_sync to wait runtime resume.
> 
> "Three have"? The above description is very hard to understand.
> Please
> improve it.

Hi Bart,

Sorry, will improve the description next version.

> 
> > - /* Update RTC only when there are no requests in progress and
> UFSHCI is operational */
> > -if (!ufshcd_is_ufs_dev_busy(hba) && hba->ufshcd_state ==
> UFSHCD_STATE_OPERATIONAL)
> > + /*
> > +  * Update RTC only when
> > +  * 1. there are no requests in progress
> > +  * 2. UFSHCI is operational
> > +  * 3. pm operation is not in progress
> > +  */
> > +if (!ufshcd_is_ufs_dev_busy(hba) &&
> > +    hba->ufshcd_state == UFSHCD_STATE_OPERATIONAL &&
> > +    !hba->pm_op_in_progress)
> >   ufshcd_update_rtc(hba);
> >   
> >   if (ufshcd_is_ufs_dev_active(hba) && hba-
> >dev_info.rtc_update_period)
> 
> The above seems racy to me. I don't think there is any mechanism that
> prevents that hba->pm_op_in_progress is set after it has been checked
> and before ufshcd_update_rtc() is called. Has it been considered to
> add
> an ufshcd_rpm_get_sync_nowait() call before the hba-
> >pm_op_in_progress
> check and a ufshcd_rpm_put_sync() call after the ufshcd_update_rtc()
> call?
> 
> Thanks,
> 
> Bart.

Yes, check pm_op_in_progress still cannot guarantee the absence of
race 
conditions. But use ufshcd_rpm_get_sync_nowait might be a bit
complicated.
How about use ufshcd_rpm_get_if_active? I will update next version.

Thanks.
Peter