Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
On Fri, 6 Jul 2018, Mukesh Ojha wrote: > Hi Thomas, > > Could you raise a formal patch on this as you are the author now? I'm short of cycles ATM. Feel free to pick it up and write a nice changelog. Just add Originally-by: Thomas Gleixner Thanks, tglx
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
On Fri, 6 Jul 2018, Mukesh Ojha wrote: > Hi Thomas, > > Could you raise a formal patch on this as you are the author now? I'm short of cycles ATM. Feel free to pick it up and write a nice changelog. Just add Originally-by: Thomas Gleixner Thanks, tglx
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
Hi Thomas, Could you raise a formal patch on this as you are the author now? Thanks, Mukesh On 6/25/2018 8:34 PM, Thomas Gleixner wrote: On Mon, 25 Jun 2018, Mukesh Ojha wrote: On 6/23/2018 2:57 AM, Thomas Gleixner wrote: @@ -1671,7 +1685,6 @@ void timekeeping_resume(void) struct timespec64 ts_new, ts_delta; u64 cycle_now; -sleeptime_injected = false; read_persistent_clock64(_new); clockevents_resume(); @@ -1743,6 +1756,8 @@ int timekeeping_suspend(void) if (timekeeping_suspend_time.tv_sec || timekeeping_suspend_time.tv_nsec) persistent_clock_exists = true; +sleeptime_injected = false; I did not get the exact valid point of moving it from `timekeeping_suspend` to `timekeeping_resume`. It's the other way round. I move it from resume to suspend. Simply because it should only be set to 'false' when suspend is reached. It would work the other way round as well, but I felt it's inconsistent. Thanks, tglx
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
Hi Thomas, Could you raise a formal patch on this as you are the author now? Thanks, Mukesh On 6/25/2018 8:34 PM, Thomas Gleixner wrote: On Mon, 25 Jun 2018, Mukesh Ojha wrote: On 6/23/2018 2:57 AM, Thomas Gleixner wrote: @@ -1671,7 +1685,6 @@ void timekeeping_resume(void) struct timespec64 ts_new, ts_delta; u64 cycle_now; -sleeptime_injected = false; read_persistent_clock64(_new); clockevents_resume(); @@ -1743,6 +1756,8 @@ int timekeeping_suspend(void) if (timekeeping_suspend_time.tv_sec || timekeeping_suspend_time.tv_nsec) persistent_clock_exists = true; +sleeptime_injected = false; I did not get the exact valid point of moving it from `timekeeping_suspend` to `timekeeping_resume`. It's the other way round. I move it from resume to suspend. Simply because it should only be set to 'false' when suspend is reached. It would work the other way round as well, but I felt it's inconsistent. Thanks, tglx
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
On Mon, 25 Jun 2018, Mukesh Ojha wrote: > On 6/23/2018 2:57 AM, Thomas Gleixner wrote: > > @@ -1671,7 +1685,6 @@ void timekeeping_resume(void) > > struct timespec64 ts_new, ts_delta; > > u64 cycle_now; > > - sleeptime_injected = false; > > read_persistent_clock64(_new); > > clockevents_resume(); > > @@ -1743,6 +1756,8 @@ int timekeeping_suspend(void) > > if (timekeeping_suspend_time.tv_sec || > > timekeeping_suspend_time.tv_nsec) > > persistent_clock_exists = true; > > + sleeptime_injected = false; > > I did not get the exact valid point of moving it from `timekeeping_suspend` to > `timekeeping_resume`. It's the other way round. I move it from resume to suspend. Simply because it should only be set to 'false' when suspend is reached. It would work the other way round as well, but I felt it's inconsistent. Thanks, tglx
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
On Mon, 25 Jun 2018, Mukesh Ojha wrote: > On 6/23/2018 2:57 AM, Thomas Gleixner wrote: > > @@ -1671,7 +1685,6 @@ void timekeeping_resume(void) > > struct timespec64 ts_new, ts_delta; > > u64 cycle_now; > > - sleeptime_injected = false; > > read_persistent_clock64(_new); > > clockevents_resume(); > > @@ -1743,6 +1756,8 @@ int timekeeping_suspend(void) > > if (timekeeping_suspend_time.tv_sec || > > timekeeping_suspend_time.tv_nsec) > > persistent_clock_exists = true; > > + sleeptime_injected = false; > > I did not get the exact valid point of moving it from `timekeeping_suspend` to > `timekeeping_resume`. It's the other way round. I move it from resume to suspend. Simply because it should only be set to 'false' when suspend is reached. It would work the other way round as well, but I felt it's inconsistent. Thanks, tglx
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
Hi Thomas,
Thanks you very much for your time and reply.
On 6/23/2018 2:57 AM, Thomas Gleixner wrote:
On Wed, 30 May 2018, Mukesh Ojha 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'(sleeptime_injected=false) due to which we can
see mismatch in timestamps between system clock and other timers.
Fix this by updating sleeptime_injected=true for both non-stop
clocksource and persistent clock.
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()
I can see the problem.
Signed-off-by: Mukesh Ojha
---
Changes in v2:
* Updated the commit text.
* Removed extra variable and used the earlier static
variable 'sleeptime_injected'.
kernel/time/timekeeping.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49cbcee..2754c1b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
timekeeper *tk,
*/
bool timekeeping_rtc_skipresume(void)
{
+ struct timekeeper *tk = _core.timekeeper;
+ /*
+* This is to ensure that we don't end up injecting
+* the sleeptime via rtc_resume() for non-stop clocksource
+* when we fail to sleep.
+*/
+ if (!sleeptime_injected)
+ sleeptime_injected = ((tk->tkr_mono.clock->flags &
+ CLOCK_SOURCE_SUSPEND_NONSTOP) ||
+ (persistent_clock_exists)) ? true : false;
But this is really a horrible hack. The right thing to do is to keep track
whether timekeeping_suspend() has been reached in the first place. There is
a very simple way to do that. Uncompiled and completely untested patch
below, but you get the idea.
Yeah, missed completely the fact that the issue can also come where only
clocksource is RTC.
Thanks,
tglx
8<---
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4786df904c22..32ae9aea61c3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1510,8 +1510,20 @@ 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 reflecting whether timekeeping_resume() has injected sleeptime.
+ *
+ * The flag starts of true and is only cleared when a suspend reaches
+ * timekeeping_suspend(), timekeeping_resume() sets it when the timekeeper
+ * clocksource is not stopping across suspend and has been used to update
+ * sleep time. If the timekeeper clocksource has stopped then the flag
+ * stays false and is used by the RTC resume code to decide whether sleep
+ * time must be injected and if so the flag gets set then.
+ *
+ * If a suspend fails before reaching timekeeping_resume() then the flag
+ * stays true and prevents erroneous sleeptime injection.
+ */
+static bool sleeptime_injected = true;
This will prevent first sleep failure.
/* Flag for if there is a persistent clock on this platform */
static bool persistent_clock_exists;
@@ -1646,6 +1658,8 @@ void timekeeping_inject_sleeptime64(struct timespec64
*delta)
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
+ sleeptime_injected = true;
This will prevent further extra sleeptime injection if sleep fails
(valid for RTC only).
Looks good!
+
timekeeping_forward_now(tk);
__timekeeping_inject_sleeptime(tk, delta);
@@ -1671,7 +1685,6 @@ void timekeeping_resume(void)
struct timespec64 ts_new, ts_delta;
u64 cycle_now;
- sleeptime_injected = false;
read_persistent_clock64(_new);
clockevents_resume();
@@ -1743,6 +1756,8 @@ int timekeeping_suspend(void)
if (timekeeping_suspend_time.tv_sec || timekeeping_suspend_time.tv_nsec)
persistent_clock_exists = true;
+ sleeptime_injected = false;
I did not get the exact valid point of moving it from
`timekeeping_suspend` to `timekeeping_resume`.
Although it will not have any side effect.
+
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
timekeeping_forward_now(tk);
Thanks for the change; will check and update.
Cheers,
Mukesh
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
Hi Thomas,
Thanks you very much for your time and reply.
On 6/23/2018 2:57 AM, Thomas Gleixner wrote:
On Wed, 30 May 2018, Mukesh Ojha 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'(sleeptime_injected=false) due to which we can
see mismatch in timestamps between system clock and other timers.
Fix this by updating sleeptime_injected=true for both non-stop
clocksource and persistent clock.
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()
I can see the problem.
Signed-off-by: Mukesh Ojha
---
Changes in v2:
* Updated the commit text.
* Removed extra variable and used the earlier static
variable 'sleeptime_injected'.
kernel/time/timekeeping.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49cbcee..2754c1b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
timekeeper *tk,
*/
bool timekeeping_rtc_skipresume(void)
{
+ struct timekeeper *tk = _core.timekeeper;
+ /*
+* This is to ensure that we don't end up injecting
+* the sleeptime via rtc_resume() for non-stop clocksource
+* when we fail to sleep.
+*/
+ if (!sleeptime_injected)
+ sleeptime_injected = ((tk->tkr_mono.clock->flags &
+ CLOCK_SOURCE_SUSPEND_NONSTOP) ||
+ (persistent_clock_exists)) ? true : false;
But this is really a horrible hack. The right thing to do is to keep track
whether timekeeping_suspend() has been reached in the first place. There is
a very simple way to do that. Uncompiled and completely untested patch
below, but you get the idea.
Yeah, missed completely the fact that the issue can also come where only
clocksource is RTC.
Thanks,
tglx
8<---
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4786df904c22..32ae9aea61c3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1510,8 +1510,20 @@ 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 reflecting whether timekeeping_resume() has injected sleeptime.
+ *
+ * The flag starts of true and is only cleared when a suspend reaches
+ * timekeeping_suspend(), timekeeping_resume() sets it when the timekeeper
+ * clocksource is not stopping across suspend and has been used to update
+ * sleep time. If the timekeeper clocksource has stopped then the flag
+ * stays false and is used by the RTC resume code to decide whether sleep
+ * time must be injected and if so the flag gets set then.
+ *
+ * If a suspend fails before reaching timekeeping_resume() then the flag
+ * stays true and prevents erroneous sleeptime injection.
+ */
+static bool sleeptime_injected = true;
This will prevent first sleep failure.
/* Flag for if there is a persistent clock on this platform */
static bool persistent_clock_exists;
@@ -1646,6 +1658,8 @@ void timekeeping_inject_sleeptime64(struct timespec64
*delta)
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
+ sleeptime_injected = true;
This will prevent further extra sleeptime injection if sleep fails
(valid for RTC only).
Looks good!
+
timekeeping_forward_now(tk);
__timekeeping_inject_sleeptime(tk, delta);
@@ -1671,7 +1685,6 @@ void timekeeping_resume(void)
struct timespec64 ts_new, ts_delta;
u64 cycle_now;
- sleeptime_injected = false;
read_persistent_clock64(_new);
clockevents_resume();
@@ -1743,6 +1756,8 @@ int timekeeping_suspend(void)
if (timekeeping_suspend_time.tv_sec || timekeeping_suspend_time.tv_nsec)
persistent_clock_exists = true;
+ sleeptime_injected = false;
I did not get the exact valid point of moving it from
`timekeeping_suspend` to `timekeeping_resume`.
Although it will not have any side effect.
+
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
timekeeping_forward_now(tk);
Thanks for the change; will check and update.
Cheers,
Mukesh
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
On Wed, 30 May 2018, Mukesh Ojha 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'(sleeptime_injected=false) due to which we can
> see mismatch in timestamps between system clock and other timers.
>
> Fix this by updating sleeptime_injected=true for both non-stop
> clocksource and persistent clock.
>
> 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()
I can see the problem.
> Signed-off-by: Mukesh Ojha
> ---
> Changes in v2:
> * Updated the commit text.
> * Removed extra variable and used the earlier static
>variable 'sleeptime_injected'.
>
> kernel/time/timekeeping.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 49cbcee..2754c1b 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
> timekeeper *tk,
> */
> bool timekeeping_rtc_skipresume(void)
> {
> + struct timekeeper *tk = _core.timekeeper;
> + /*
> + * This is to ensure that we don't end up injecting
> + * the sleeptime via rtc_resume() for non-stop clocksource
> + * when we fail to sleep.
> + */
> + if (!sleeptime_injected)
> + sleeptime_injected = ((tk->tkr_mono.clock->flags &
> + CLOCK_SOURCE_SUSPEND_NONSTOP) ||
> + (persistent_clock_exists)) ? true : false;
But this is really a horrible hack. The right thing to do is to keep track
whether timekeeping_suspend() has been reached in the first place. There is
a very simple way to do that. Uncompiled and completely untested patch
below, but you get the idea.
Thanks,
tglx
8<---
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4786df904c22..32ae9aea61c3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1510,8 +1510,20 @@ 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 reflecting whether timekeeping_resume() has injected sleeptime.
+ *
+ * The flag starts of true and is only cleared when a suspend reaches
+ * timekeeping_suspend(), timekeeping_resume() sets it when the timekeeper
+ * clocksource is not stopping across suspend and has been used to update
+ * sleep time. If the timekeeper clocksource has stopped then the flag
+ * stays false and is used by the RTC resume code to decide whether sleep
+ * time must be injected and if so the flag gets set then.
+ *
+ * If a suspend fails before reaching timekeeping_resume() then the flag
+ * stays true and prevents erroneous sleeptime injection.
+ */
+static bool sleeptime_injected = true;
/* Flag for if there is a persistent clock on this platform */
static bool persistent_clock_exists;
@@ -1646,6 +1658,8 @@ void timekeeping_inject_sleeptime64(struct timespec64
*delta)
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
+ sleeptime_injected = true;
+
timekeeping_forward_now(tk);
__timekeeping_inject_sleeptime(tk, delta);
@@ -1671,7 +1685,6 @@ void timekeeping_resume(void)
struct timespec64 ts_new, ts_delta;
u64 cycle_now;
- sleeptime_injected = false;
read_persistent_clock64(_new);
clockevents_resume();
@@ -1743,6 +1756,8 @@ int timekeeping_suspend(void)
if (timekeeping_suspend_time.tv_sec || timekeeping_suspend_time.tv_nsec)
persistent_clock_exists = true;
+ sleeptime_injected = false;
+
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
timekeeping_forward_now(tk);
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
On Wed, 30 May 2018, Mukesh Ojha 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'(sleeptime_injected=false) due to which we can
> see mismatch in timestamps between system clock and other timers.
>
> Fix this by updating sleeptime_injected=true for both non-stop
> clocksource and persistent clock.
>
> 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()
I can see the problem.
> Signed-off-by: Mukesh Ojha
> ---
> Changes in v2:
> * Updated the commit text.
> * Removed extra variable and used the earlier static
>variable 'sleeptime_injected'.
>
> kernel/time/timekeeping.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 49cbcee..2754c1b 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
> timekeeper *tk,
> */
> bool timekeeping_rtc_skipresume(void)
> {
> + struct timekeeper *tk = _core.timekeeper;
> + /*
> + * This is to ensure that we don't end up injecting
> + * the sleeptime via rtc_resume() for non-stop clocksource
> + * when we fail to sleep.
> + */
> + if (!sleeptime_injected)
> + sleeptime_injected = ((tk->tkr_mono.clock->flags &
> + CLOCK_SOURCE_SUSPEND_NONSTOP) ||
> + (persistent_clock_exists)) ? true : false;
But this is really a horrible hack. The right thing to do is to keep track
whether timekeeping_suspend() has been reached in the first place. There is
a very simple way to do that. Uncompiled and completely untested patch
below, but you get the idea.
Thanks,
tglx
8<---
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4786df904c22..32ae9aea61c3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1510,8 +1510,20 @@ 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 reflecting whether timekeeping_resume() has injected sleeptime.
+ *
+ * The flag starts of true and is only cleared when a suspend reaches
+ * timekeeping_suspend(), timekeeping_resume() sets it when the timekeeper
+ * clocksource is not stopping across suspend and has been used to update
+ * sleep time. If the timekeeper clocksource has stopped then the flag
+ * stays false and is used by the RTC resume code to decide whether sleep
+ * time must be injected and if so the flag gets set then.
+ *
+ * If a suspend fails before reaching timekeeping_resume() then the flag
+ * stays true and prevents erroneous sleeptime injection.
+ */
+static bool sleeptime_injected = true;
/* Flag for if there is a persistent clock on this platform */
static bool persistent_clock_exists;
@@ -1646,6 +1658,8 @@ void timekeeping_inject_sleeptime64(struct timespec64
*delta)
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
+ sleeptime_injected = true;
+
timekeeping_forward_now(tk);
__timekeeping_inject_sleeptime(tk, delta);
@@ -1671,7 +1685,6 @@ void timekeeping_resume(void)
struct timespec64 ts_new, ts_delta;
u64 cycle_now;
- sleeptime_injected = false;
read_persistent_clock64(_new);
clockevents_resume();
@@ -1743,6 +1756,8 @@ int timekeeping_suspend(void)
if (timekeeping_suspend_time.tv_sec || timekeeping_suspend_time.tv_nsec)
persistent_clock_exists = true;
+ sleeptime_injected = false;
+
raw_spin_lock_irqsave(_lock, flags);
write_seqcount_begin(_core.seq);
timekeeping_forward_now(tk);
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
Any input on this ?
Thanks,
Mukesh
On 5/30/2018 5:14 PM, Mukesh Ojha 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'(sleeptime_injected=false) due to which we can
see mismatch in timestamps between system clock and other timers.
Fix this by updating sleeptime_injected=true for both non-stop
clocksource and persistent clock.
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
---
Changes in v2:
* Updated the commit text.
* Removed extra variable and used the earlier static
variable 'sleeptime_injected'.
kernel/time/timekeeping.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49cbcee..2754c1b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
timekeeper *tk,
*/
bool timekeeping_rtc_skipresume(void)
{
+ struct timekeeper *tk = _core.timekeeper;
+ /*
+* This is to ensure that we don't end up injecting
+* the sleeptime via rtc_resume() for non-stop clocksource
+* when we fail to sleep.
+*/
+ if (!sleeptime_injected)
+ sleeptime_injected = ((tk->tkr_mono.clock->flags &
+ CLOCK_SOURCE_SUSPEND_NONSTOP) ||
+ (persistent_clock_exists)) ? true : false;
+
return sleeptime_injected;
}
Re: [PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
Any input on this ?
Thanks,
Mukesh
On 5/30/2018 5:14 PM, Mukesh Ojha 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'(sleeptime_injected=false) due to which we can
see mismatch in timestamps between system clock and other timers.
Fix this by updating sleeptime_injected=true for both non-stop
clocksource and persistent clock.
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
---
Changes in v2:
* Updated the commit text.
* Removed extra variable and used the earlier static
variable 'sleeptime_injected'.
kernel/time/timekeeping.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49cbcee..2754c1b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
timekeeper *tk,
*/
bool timekeeping_rtc_skipresume(void)
{
+ struct timekeeper *tk = _core.timekeeper;
+ /*
+* This is to ensure that we don't end up injecting
+* the sleeptime via rtc_resume() for non-stop clocksource
+* when we fail to sleep.
+*/
+ if (!sleeptime_injected)
+ sleeptime_injected = ((tk->tkr_mono.clock->flags &
+ CLOCK_SOURCE_SUSPEND_NONSTOP) ||
+ (persistent_clock_exists)) ? true : false;
+
return sleeptime_injected;
}
[PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
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'(sleeptime_injected=false) due to which we can
see mismatch in timestamps between system clock and other timers.
Fix this by updating sleeptime_injected=true for both non-stop
clocksource and persistent clock.
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
---
Changes in v2:
* Updated the commit text.
* Removed extra variable and used the earlier static
variable 'sleeptime_injected'.
kernel/time/timekeeping.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49cbcee..2754c1b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
timekeeper *tk,
*/
bool timekeeping_rtc_skipresume(void)
{
+ struct timekeeper *tk = _core.timekeeper;
+ /*
+* This is to ensure that we don't end up injecting
+* the sleeptime via rtc_resume() for non-stop clocksource
+* when we fail to sleep.
+*/
+ if (!sleeptime_injected)
+ sleeptime_injected = ((tk->tkr_mono.clock->flags &
+ CLOCK_SOURCE_SUSPEND_NONSTOP) ||
+ (persistent_clock_exists)) ? true : false;
+
return sleeptime_injected;
}
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative
Project
[PATCH v2] time: Fix sleeptime injection for non-stop clocksource & persistent clock
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'(sleeptime_injected=false) due to which we can
see mismatch in timestamps between system clock and other timers.
Fix this by updating sleeptime_injected=true for both non-stop
clocksource and persistent clock.
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
---
Changes in v2:
* Updated the commit text.
* Removed extra variable and used the earlier static
variable 'sleeptime_injected'.
kernel/time/timekeeping.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49cbcee..2754c1b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1610,6 +1610,17 @@ static void __timekeeping_inject_sleeptime(struct
timekeeper *tk,
*/
bool timekeeping_rtc_skipresume(void)
{
+ struct timekeeper *tk = _core.timekeeper;
+ /*
+* This is to ensure that we don't end up injecting
+* the sleeptime via rtc_resume() for non-stop clocksource
+* when we fail to sleep.
+*/
+ if (!sleeptime_injected)
+ sleeptime_injected = ((tk->tkr_mono.clock->flags &
+ CLOCK_SOURCE_SUSPEND_NONSTOP) ||
+ (persistent_clock_exists)) ? true : false;
+
return sleeptime_injected;
}
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative
Project
