Messages in this thread | | | Date | Tue, 2 Jun 2015 23:12:38 +0800 | From | Wang YanQing <> | Subject | Re: [PATCH v2] timekeeping: always make sure wall_to_monotonic isn't positive |
| |
On Mon, Jun 01, 2015 at 04:55:48PM -0700, John Stultz wrote: > On Sat, May 30, 2015 at 10:10 PM, Wang YanQing <udknight@gmail.com> wrote: > > I meet two issues on an IMX6 development board without enable > > RTC device(so timekeeping_init will initialize the boot time > > and monotonic to 0). > > > > Issue 1:exportfs -a generate: > > "exportfs: /opt/nfs/arm does not support NFS export" > > Issue 2:cat /proc/stat: > > "btime 4294967236" > > > > Exact reproduction of the same issues on x86 after run below > > code: > > " int main(void) > > { > > struct timeval val; > > int ret; > > > > val.tv_sec = 0; > > val.tv_usec = 0; > > ret = settimeofday(&val, NULL); > > return 0; > > } > > " > > Reason: > > The reason is positive wall_to_monotonic push boot time back to the time > > before Epoch, getboottime will return negative value. > > > > In issue 1: > > negative boot time cause get_expiry overflow time_t when input expire > > time is 2147483647, then cache_flush always clear entries just added > > in ip_map_parse. > > In issue 2: > > show_stat use "unsigned long" to print > > negative value return by getboottime. > > > > This patch fix these two issues. > > If there is two issues, we probably should have two patches, each > clearly fixing one issue. If there is one problem with multiple > symptoms, then a single patch is fine but we want to be clear there. > > > > Note: this patch will cause we can't use settimeofday with time > > earlier than current time on system which timekeeping_init > > initialize the xtime, boot and monotonic to 0 before set > > current time to a more reasonable time point. > > If everything is initialized to 0 (aka 1970), then setting the time to > prior to (relatively) shortly after boot is a pretty reasonable > constraint. So you might want to reword this a little bit. I get it and will reword this in v3.
> This basically seems to come down to the fact that you can't set the > CLOCK_REALTIME time prior to (1970 + system uptime), right? Yes, that's strict and precise representation!
> > > > > > Signed-off-by: Wang YanQing <udknight@gmail.com> > > --- > > Changes v1-v2: > > 1: fix subject, use "isn't positive" instead of "is negative". > > 2: rewrite changelog. > > 3: simplify code as suggested by John Stultz. > > > > It really take me some times to realize how stupid and > > buggy the version 1 patch is, but I am ready to be told > > this version is even stupider:) > > > > Thanks. > > > > kernel/time/timekeeping.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > > index 0d784b3..b501aa6 100644 > > --- a/kernel/time/timekeeping.c > > +++ b/kernel/time/timekeeping.c > > @@ -895,6 +895,7 @@ int do_settimeofday64(const struct timespec64 *ts) > > struct timekeeper *tk = &tk_core.timekeeper; > > struct timespec64 ts_delta, xt; > > unsigned long flags; > > + int ret = 0; > > > > if (!timespec64_valid_strict(ts)) > > return -EINVAL; > > @@ -908,10 +909,15 @@ int do_settimeofday64(const struct timespec64 *ts) > > ts_delta.tv_sec = ts->tv_sec - xt.tv_sec; > > ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec; > > > > + if (timespec64_compare(&tk->wall_to_monotonic, &ts_delta) > 0) { > > + ret = -EINVAL; > > + goto out; > > + } > > + > > tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta)); > > > > tk_set_xtime(tk, ts); > > - > > +out: > > timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET); > > If we didn't set the time, should we be calling timekeeping_update here?
Because we have called timekeeping_forward_now(tk), I found a same situation in timekeeping_inject_offset:
"error: /* even if we error out, we forwarded the time, so call update */"
Thanks.
| |