* [PATCH] timekeeping: always make sure wall_to_monotonic is negative
@ 2015-03-16 19:28 Wang YanQing
2015-05-14 0:01 ` John Stultz
0 siblings, 1 reply; 2+ messages in thread
From: Wang YanQing @ 2015-03-16 19:28 UTC (permalink / raw)
To: tglx; +Cc: john.stultz, linux-kernel
When wall_to_monotonic become positive, then I get below two
errors on an IMX6 development board without enable RTC device:
1:execute exportfs -a generate:
"exportfs: /opt/nfs/arm does not support NFS export"
2:cat /proc/stat:
"btime 4294967236"
I can product the same issues on x86 with newest kernel in next
tree with below c code:
"
int main(void)
{
struct timeval val;
int ret;
val.tv_sec = 0;
val.tv_usec = 0;
ret = settimeofday(&val, NULL);
printf("ret:%d\n", ret);
return 0;
}
"
The reason is getboottime[64] return negative value, cause below
codes don't work:
nfs error:cache.h:get_expiry return negative value cause
cache_flush always clear entries just added in
ip_map_parse.
proc/stat error:
seq_printf use "unsigned long" to show
a negative number return by getboottime.
This patch fix it by validate new value of wall_to_monotonic
before assign it.
Signed-off-by: Wang YanQing <udknight@gmail.com>
---
kernel/time/timekeeping.c | 71 ++++++++++++++++++++++++++++++++++++-----------
1 file changed, 55 insertions(+), 16 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 91db941..799e323 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -96,11 +96,29 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec64 *ts)
tk_normalize_xtime(tk);
}
-static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
+static inline bool timespec64_negative(struct timespec64 *ts)
+{
+ if (ts->tv_sec > 0)
+ return false;
+ if (ts->tv_sec == 0 && ts->tv_nsec > 0)
+ return false;
+ return true;
+}
+
+static bool tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
{
struct timespec64 tmp;
/*
+ * The current time
+ * wall_to_monotonic is what we need to add to xtime (or xtime corrected
+ * for sub jiffie times) to get to monotonic time. Monotonic is pegged
+ * at zero at system boot time, so wall_to_monotonic will be negative.
+ */
+ if (!timespec64_negative(&wtm))
+ return false;
+
+ /*
* Verify consistency of: offset_real = -wall_to_monotonic
* before modifying anything
*/
@@ -111,6 +129,7 @@ static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
set_normalized_timespec64(&tmp, -wtm.tv_sec, -wtm.tv_nsec);
tk->offs_real = timespec64_to_ktime(tmp);
tk->offs_tai = ktime_add(tk->offs_real, ktime_set(tk->tai_offset, 0));
+ return true;
}
static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta)
@@ -796,8 +815,9 @@ EXPORT_SYMBOL(do_gettimeofday);
int do_settimeofday64(const struct timespec64 *ts)
{
struct timekeeper *tk = &tk_core.timekeeper;
- struct timespec64 ts_delta, xt;
+ struct timespec64 ts_delta, tmp;
unsigned long flags;
+ int ret = 0;
if (!timespec64_valid_strict(ts))
return -EINVAL;
@@ -807,23 +827,27 @@ int do_settimeofday64(const struct timespec64 *ts)
timekeeping_forward_now(tk);
- xt = tk_xtime(tk);
- ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
- ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
+ tmp = tk_xtime(tk);
+ ts_delta.tv_sec = ts->tv_sec - tmp.tv_sec;
+ ts_delta.tv_nsec = ts->tv_nsec - tmp.tv_nsec;
- tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
+ tmp = timespec64_sub(tk->wall_to_monotonic, ts_delta);
+ if (!tk_set_wall_to_mono(tk, tmp)) {
+ ret = -EINVAL;
+ goto out;
+ }
tk_set_xtime(tk, ts);
-
timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
-
+out:
write_seqcount_end(&tk_core.seq);
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
/* signal hrtimers about time change */
- clock_was_set();
+ if (!ret)
+ clock_was_set();
- return 0;
+ return ret;
}
EXPORT_SYMBOL(do_settimeofday64);
@@ -857,8 +881,13 @@ int timekeeping_inject_offset(struct timespec *ts)
goto error;
}
+ tmp = timespec64_sub(tk->wall_to_monotonic, ts64);
+ if (!tk_set_wall_to_mono(tk, tmp)) {
+ ret = -EINVAL;
+ goto error;
+ }
+
tk_xtime_add(tk, &ts64);
- tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts64));
error: /* even if we error out, we forwarded the time, so call update */
timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
@@ -1140,14 +1169,21 @@ static struct timespec64 timekeeping_suspend_time;
static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
struct timespec64 *delta)
{
+ struct timespec64 tmp;
+
if (!timespec64_valid_strict(delta)) {
printk_deferred(KERN_WARNING
"__timekeeping_inject_sleeptime: Invalid "
"sleep delta value!\n");
return;
}
+
+ tmp = timespec64_sub(tk->wall_to_monotonic, *delta);
+ if (!tk_set_wall_to_mono(tk, tmp)) {
+ WARN_ON(1);
+ return;
+ }
tk_xtime_add(tk, delta);
- tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, *delta));
tk_update_sleep_time(tk, timespec64_to_ktime(*delta));
tk_debug_account_sleep_time(delta);
}
@@ -1539,14 +1575,17 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
leap = second_overflow(tk->xtime_sec);
if (unlikely(leap)) {
struct timespec64 ts;
-
- tk->xtime_sec += leap;
+ bool ret;
ts.tv_sec = leap;
ts.tv_nsec = 0;
- tk_set_wall_to_mono(tk,
+ ret = tk_set_wall_to_mono(tk,
timespec64_sub(tk->wall_to_monotonic, ts));
-
+ if (!ret) {
+ WARN_ON_ONCE(1);
+ break;
+ }
+ tk->xtime_sec += leap;
__timekeeping_set_tai_offset(tk, tk->tai_offset - leap);
clock_set = TK_CLOCK_WAS_SET;
--
2.2.2.dirty
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] timekeeping: always make sure wall_to_monotonic is negative
2015-03-16 19:28 [PATCH] timekeeping: always make sure wall_to_monotonic is negative Wang YanQing
@ 2015-05-14 0:01 ` John Stultz
0 siblings, 0 replies; 2+ messages in thread
From: John Stultz @ 2015-05-14 0:01 UTC (permalink / raw)
To: Wang YanQing, Thomas Gleixner, John Stultz, lkml
On Mon, Mar 16, 2015 at 12:28 PM, Wang YanQing <udknight@gmail.com> wrote:
>
> When wall_to_monotonic become positive, then I get below two
> errors on an IMX6 development board without enable RTC device:
>
> 1:execute exportfs -a generate:
> "exportfs: /opt/nfs/arm does not support NFS export"
> 2:cat /proc/stat:
> "btime 4294967236"
>
> I can product the same issues on x86 with newest kernel in next
> tree with below c code:
> "
> int main(void)
> {
> struct timeval val;
> int ret;
>
> val.tv_sec = 0;
> val.tv_usec = 0;
> ret = settimeofday(&val, NULL);
> printf("ret:%d\n", ret);
> return 0;
> }
> "
> The reason is getboottime[64] return negative value, cause below
> codes don't work:
>
> nfs error:cache.h:get_expiry return negative value cause
> cache_flush always clear entries just added in
> ip_map_parse.
> proc/stat error:
> seq_printf use "unsigned long" to show
> a negative number return by getboottime.
>
> This patch fix it by validate new value of wall_to_monotonic
> before assign it.
Sorry for the slow response, and thanks for the problem report. Some
questions below...
So to restate the problem: setting the CLOCK_REALTIME back to a
timevalue that is less then the value of CLOCK_MONOTONIC causes
problems with various interfaces such as nfs and proc/stat.
Is that right?
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
> kernel/time/timekeeping.c | 71 ++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 91db941..799e323 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -96,11 +96,29 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec64 *ts)
> tk_normalize_xtime(tk);
> }
>
> -static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
> +static inline bool timespec64_negative(struct timespec64 *ts)
> +{
> + if (ts->tv_sec > 0)
> + return false;
> + if (ts->tv_sec == 0 && ts->tv_nsec > 0)
> + return false;
> + return true;
> +}
> +
> +static bool tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
> {
> struct timespec64 tmp;
>
> /*
> + * The current time
> + * wall_to_monotonic is what we need to add to xtime (or xtime corrected
> + * for sub jiffie times) to get to monotonic time. Monotonic is pegged
> + * at zero at system boot time, so wall_to_monotonic will be negative.
> + */
> + if (!timespec64_negative(&wtm))
> + return false;
> +
> + /*
> * Verify consistency of: offset_real = -wall_to_monotonic
> * before modifying anything
> */
> @@ -111,6 +129,7 @@ static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
> set_normalized_timespec64(&tmp, -wtm.tv_sec, -wtm.tv_nsec);
> tk->offs_real = timespec64_to_ktime(tmp);
> tk->offs_tai = ktime_add(tk->offs_real, ktime_set(tk->tai_offset, 0));
> + return true;
> }
>
> static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta)
> @@ -796,8 +815,9 @@ EXPORT_SYMBOL(do_gettimeofday);
> int do_settimeofday64(const struct timespec64 *ts)
> {
> struct timekeeper *tk = &tk_core.timekeeper;
> - struct timespec64 ts_delta, xt;
> + struct timespec64 ts_delta, tmp;
> unsigned long flags;
> + int ret = 0;
>
> if (!timespec64_valid_strict(ts))
> return -EINVAL;
> @@ -807,23 +827,27 @@ int do_settimeofday64(const struct timespec64 *ts)
>
> timekeeping_forward_now(tk);
>
> - xt = tk_xtime(tk);
> - ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
> - ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
> + tmp = tk_xtime(tk);
> + ts_delta.tv_sec = ts->tv_sec - tmp.tv_sec;
> + ts_delta.tv_nsec = ts->tv_nsec - tmp.tv_nsec;
>
> - tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
> + tmp = timespec64_sub(tk->wall_to_monotonic, ts_delta);
> + if (!tk_set_wall_to_mono(tk, tmp)) {
> + ret = -EINVAL;
> + goto out;
> + }
It seems like a simpler check at the top of do_settimeofday64() would
be easier then embedding the check in set_wall_to_mono and trying to
deal with it later in the process of adjusting time.
>
> tk_set_xtime(tk, ts);
> -
> timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
> -
> +out:
> write_seqcount_end(&tk_core.seq);
> raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
>
> /* signal hrtimers about time change */
> - clock_was_set();
> + if (!ret)
> + clock_was_set();
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL(do_settimeofday64);
>
> @@ -857,8 +881,13 @@ int timekeeping_inject_offset(struct timespec *ts)
> goto error;
> }
>
> + tmp = timespec64_sub(tk->wall_to_monotonic, ts64);
> + if (!tk_set_wall_to_mono(tk, tmp)) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
Similarly here. A simple check at the top before we make any
adjustments would make more sense to me.
> tk_xtime_add(tk, &ts64);
> - tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts64));
>
> error: /* even if we error out, we forwarded the time, so call update */
> timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
> @@ -1140,14 +1169,21 @@ static struct timespec64 timekeeping_suspend_time;
> static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
> struct timespec64 *delta)
> {
> + struct timespec64 tmp;
> +
> if (!timespec64_valid_strict(delta)) {
> printk_deferred(KERN_WARNING
> "__timekeeping_inject_sleeptime: Invalid "
> "sleep delta value!\n");
> return;
> }
> +
> + tmp = timespec64_sub(tk->wall_to_monotonic, *delta);
> + if (!tk_set_wall_to_mono(tk, tmp)) {
> + WARN_ON(1);
> + return;
> + }
Is this really possible? _inject_sleeptime should never be negative.
Or am I missing something?
> tk_xtime_add(tk, delta);
> - tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, *delta));
> tk_update_sleep_time(tk, timespec64_to_ktime(*delta));
> tk_debug_account_sleep_time(delta);
> }
> @@ -1539,14 +1575,17 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
> leap = second_overflow(tk->xtime_sec);
> if (unlikely(leap)) {
> struct timespec64 ts;
> -
> - tk->xtime_sec += leap;
> + bool ret;
>
> ts.tv_sec = leap;
> ts.tv_nsec = 0;
> - tk_set_wall_to_mono(tk,
> + ret = tk_set_wall_to_mono(tk,
> timespec64_sub(tk->wall_to_monotonic, ts));
How would adding time in accumulate_nsecs_to_secs ever cause
wall_to_monotonic to not be negative?
> -
> + if (!ret) {
> + WARN_ON_ONCE(1);
> + break;
> + }
> + tk->xtime_sec += leap;
> __timekeeping_set_tai_offset(tk, tk->tai_offset - leap);
>
> clock_set = TK_CLOCK_WAS_SET;
> --
> 2.2.2.dirty
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-05-14 0:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-16 19:28 [PATCH] timekeeping: always make sure wall_to_monotonic is negative Wang YanQing
2015-05-14 0:01 ` John Stultz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox