Messages in this thread | | | Subject | Re: [PATCH v4] time: Fix extra sleeptime injection when suspend fails | From | Mukesh Ojha <> | Date | Tue, 17 Jul 2018 11:59:11 +0530 |
| |
On 7/17/2018 2:31 AM, John Stultz wrote: > On Mon, Jul 16, 2018 at 1:40 PM, Mukesh Ojha <mojha@codeaurora.org> wrote: >> Currently, there exists a corner case assuming when there is >> only one clocksource e.g RTC, and system failed to go to >> suspend mode. While resume rtc_resume() injects the sleeptime >> as timekeeping_rtc_skipresume() returned 'false' (default value >> of sleeptime_injected) due to which we can see mismatch in >> timestamps. >> >> This issue can also come in a system where more than one >> clocksource are present and very first suspend fails. >> >> Fix this by handling the sleeptime_injected flag properly. >> >> Success case: >> ------------ >> {sleeptime_injected=false} >> rtc_suspend() => timekeeping_suspend() => timekeeping_resume() => >> >> (sleeptime injected) >> rtc_resume() >> >> Failure case: >> ------------ >> {failure in sleep path} {sleeptime_injected=false} >> rtc_suspend() => rtc_resume() >> >> sleeptime injected again which was not required as the suspend failed) >> >> Originally-by: Thomas Gleixner <tglx@linutronix.de> >> Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> >> --- >> Changes in V4: >> * Changes as suggested by John >> - Changed the variable name from sleeptime_injected to suspend_timing_needed >> - Changed the boolean logic. > Thanks so much for reworking and resending this again! > > >> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c >> index d37588f..ee455cc 100644 >> --- a/drivers/rtc/class.c >> +++ b/drivers/rtc/class.c >> @@ -102,7 +102,7 @@ static int rtc_resume(struct device *dev) >> struct timespec64 sleep_time; >> int err; >> >> - if (timekeeping_rtc_skipresume()) >> + if (!timekeeping_rtc_skipresume()) > > Hrm... So I'd have instead inverted the logic *in* > timekeeping_rtc_skipresume(), rather then here, but this looks to be > close enough and I can fix that bit up.
Will take care of yours and Thomas comment in v5.
> > Can you confirm you've validated this version of the patch resolves > the issue you reported? Yeah, I validated.
Thanks Mukesh
> > thanks > -john
| |