Currently, for both non-stop clocksource and persistent clock
there is a corner case, when a driver failed to go suspend mode
rtc_resume() injects the sleeptime as timekeeping_rtc_skipresume()
returned 'false' due to which we can see mismatch in time between
system clock and other timers.
Success case:
{sleeptime_injected=true}
rtc_suspend() => timekeeping_suspend() => timekeeping_resume() =>
rtc_resume()
Failure case:
{failure in sleep path} {sleeptime_injected=false}
rtc_suspend() => rtc_resume()
Signed-off-by: Mukesh Ojha <[email protected]>
---
Changes from Resend:
* updated commit text.
kernel/time/timekeeping.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 427e33d..c022d82 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1511,9 +1511,6 @@ void __weak read_boot_clock64(struct timespec64 *ts)
ts->tv_nsec = 0;
}
-/* Flag for if timekeeping_resume() has injected sleeptime */
-static bool sleeptime_injected;
-
/* Flag for if there is a persistent clock on this platform */
static bool persistent_clock_exists;
@@ -1611,7 +1608,14 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
*/
bool timekeeping_rtc_skipresume(void)
{
- return sleeptime_injected;
+ struct timekeeper *tk = &tk_core.timekeeper;
+ bool skip_rtc_resume = false;
+
+ skip_rtc_resume = ((tk->tkr_mono.clock->flags &
+ CLOCK_SOURCE_SUSPEND_NONSTOP) ||
+ (persistent_clock_exists)) ? true : false;
+
+ return skip_rtc_resume;
}
/**
@@ -1671,8 +1675,8 @@ void timekeeping_resume(void)
unsigned long flags;
struct timespec64 ts_new, ts_delta;
cycle_t cycle_now, cycle_delta;
+ bool sleeptime_injected = false;
- sleeptime_injected = false;
read_persistent_clock64(&ts_new);
clockevents_resume();
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
On Tue, May 29, 2018 at 2:49 AM, Mukesh Ojha <[email protected]> wrote:
> Currently, for both non-stop clocksource and persistent clock
> there is a corner case, when a driver failed to go suspend mode
> rtc_resume() injects the sleeptime as timekeeping_rtc_skipresume()
> returned 'false' due to which we can see mismatch in time between
> system clock and other timers.
>
> Success case:
> {sleeptime_injected=true}
> rtc_suspend() => timekeeping_suspend() => timekeeping_resume() =>
> rtc_resume()
>
> Failure case:
> {failure in sleep path} {sleeptime_injected=false}
> rtc_suspend() => rtc_resume()
>
> Signed-off-by: Mukesh Ojha <[email protected]>
I'm not sure this patch makes sense yet (since I don't really see how
its used - mind cc'ing me on the patch that makes use of this?)
And more problematic, the patch doesn't seem to apply to mainline.
Could you respin and resend?
thanks
-john
Hi John,
On 5/30/2018 7:50 AM, John Stultz wrote:
> On Tue, May 29, 2018 at 2:49 AM, Mukesh Ojha <[email protected]> wrote:
>> Currently, for both non-stop clocksource and persistent clock
>> there is a corner case, when a driver failed to go suspend mode
>> rtc_resume() injects the sleeptime as timekeeping_rtc_skipresume()
>> returned 'false' due to which we can see mismatch in time between
>> system clock and other timers.
>>
>> Success case:
>> {sleeptime_injected=true}
>> rtc_suspend() => timekeeping_suspend() => timekeeping_resume() =>
>> rtc_resume()
>>
>> Failure case:
>> {failure in sleep path} {sleeptime_injected=false}
>> rtc_suspend() => rtc_resume()
>>
>> Signed-off-by: Mukesh Ojha <[email protected]>
> I'm not sure this patch makes sense yet (since I don't really see how
> its used
In a system where both non-stop clocksource and rtc exist
Timestamp mismatch seen between a (Gyro)timer based on non-stop clocksource
and system clock after attempting a sleep.
Here, System clock timestamps increase by some millisecond as compare to
other timer.
And we see this is possible only when timekeeping_resume() has not been
called
which is keeping sleeptime_injected to its default value(false). So, the
call to
timekeeping_rtc_skipresume() in rtc_resume() is false and thats ends up in
sleeptime injection via rtc_resume().
When timekeeping_resume() has not been called means one of the driver failed
during suspend in syscore_suspend() in kernel/power/suspend.c .
So if we let rtc inject the sleeptime here, we would see the timestamp
increase in case of
system clock.
> - mind cc'ing me on the patch that makes use of this?)
>
> And more problematic, the patch doesn't seem to apply to mainline.
> Could you respin and resend?
Will spin the patch again with some minor code change.
Thanks,
Mukesh
>
> thanks
> -john