Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1676653 > unrolled thread

[PATCH] timekeeping: Use proper timekeeper for debug code

Back to article view | Back to linux.kernel

Contents

  [PATCH] timekeeping: Use proper timekeeper for debug code Stafford Horne <shorne@gmail.com> - 2017-06-28 15:30 +0200
    Re: [PATCH] timekeeping: Use proper timekeeper for debug code John Stultz <john.stultz@linaro.org> - 2017-06-28 20:00 +0200
      Re: [PATCH] timekeeping: Use proper timekeeper for debug code Stafford Horne <shorne@gmail.com> - 2017-06-29 00:40 +0200

#1676653 — [PATCH] timekeeping: Use proper timekeeper for debug code

When CONFIG_DEBUG_TIMEKEEPING is enabled the timekeeping_check_update()
function will update status like last_warning and underflow_seen on the
timekeeper.

If there are issues found this state is used to rate limit the warnings
that get printed.

This rate limiting doesn't really really work if stored in real_tk as
the shadow timekeeper is overwritten onto real_tk at the end of every
update_wall_time() call, resetting last_warning and other statuses.

Fix rate limiting by using the shadow_timekeeper for
timekeeping_check_update().

Fixes: commit 57d05a93ada7 ("time: Rework debugging variables so they aren't global")
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 kernel/time/timekeeping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 9652bc5..061088c 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2039,7 +2039,7 @@ void update_wall_time(void)
 		goto out;
 
 	/* Do some additional sanity checking */
-	timekeeping_check_update(real_tk, offset);
+	timekeeping_check_update(tk, offset);
 
 	/*
 	 * With NO_HZ we may have to accumulate many cycle_intervals
-- 
2.9.4

[toc] | [next] | [standalone]

#1676904

On Wed, Jun 28, 2017 at 6:21 AM, Stafford Horne <shorne@gmail.com> wrote:
> When CONFIG_DEBUG_TIMEKEEPING is enabled the timekeeping_check_update()
> function will update status like last_warning and underflow_seen on the
> timekeeper.
>
> If there are issues found this state is used to rate limit the warnings
> that get printed.
>
> This rate limiting doesn't really really work if stored in real_tk as
> the shadow timekeeper is overwritten onto real_tk at the end of every
> update_wall_time() call, resetting last_warning and other statuses.
>
> Fix rate limiting by using the shadow_timekeeper for
> timekeeping_check_update().
>
> Fixes: commit 57d05a93ada7 ("time: Rework debugging variables so they aren't global")
> Signed-off-by: Stafford Horne <shorne@gmail.com>

Interesting!  Thanks for submitting this! Out of curiosity, can you
also talk about the underlying issue that made you come across this
issue?

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1677246

On Wed, Jun 28, 2017 at 10:39:21AM -0700, John Stultz wrote:
> On Wed, Jun 28, 2017 at 6:21 AM, Stafford Horne <shorne@gmail.com> wrote:
> > When CONFIG_DEBUG_TIMEKEEPING is enabled the timekeeping_check_update()
> > function will update status like last_warning and underflow_seen on the
> > timekeeper.
> >
> > If there are issues found this state is used to rate limit the warnings
> > that get printed.
> >
> > This rate limiting doesn't really really work if stored in real_tk as
> > the shadow timekeeper is overwritten onto real_tk at the end of every
> > update_wall_time() call, resetting last_warning and other statuses.
> >
> > Fix rate limiting by using the shadow_timekeeper for
> > timekeeping_check_update().
> >
> > Fixes: commit 57d05a93ada7 ("time: Rework debugging variables so they aren't global")
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> 
> Interesting!  Thanks for submitting this! Out of curiosity, can you
> also talk about the underlying issue that made you come across this
> issue?

I am working on OpenRISC SMP support.  There was one issue where the
per_cpu internal timers (used as clocksource) were not in sync, this
pointed it out.

There is another issue right now when switching from jiffies to the
openrisc clocksource.  Which is maybe ok because each have different
starting points.

[    0.160000] clocksource: Switched to clocksource openrisc_timer
[    0.220000] INFO: timekeeping: Cycle offset (4294173293) is larger than the 'openrisc_timer' clock's 50% safety margin (2147483647)
[    0.220000]       timekeeping: Your kernel is still fine, but is feeling a bit nervous

Let me know if you want me to add this to the commit message in a v2.

-Stafford

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web