Re: [RFC][PATCH] timekeeping: Keep xtime_nsec remainder separatefrom ntp_error

From: Steven Rostedt
Date: Wed Dec 08 2010 - 15:15:29 EST


On Wed, 2010-12-08 at 11:59 -0800, john stultz wrote:

> Hey Steven!
>
> Thanks for the great analysis and tooling to help find these unexpected
> behaviors!
>
> Sadly, I believe your proposed change can still cause minor nsec
> inconsistencies from gettimeofday/vgettimeofday. In fact, the previous
> implementation where the nsec inconsistency error was observed preserved
> the sub-nanosecond remainder in xtime_nsec.

What inconsistencies exactly? Not to say they aren't any, but I really
want to know exactly what is happening. Even the change log that Roman
showed in that commit does not explain well what the issue was.

Here's the log:

timekeeping: fix rounding problem during clock update

Due to a rounding problem during a clock update it's possible for readers
to observe the clock jumping back by 1nsec. The following simplified
example demonstrates the problem:

cycle xtime
0 0
1000 999999.6
2000 1999999.2
3000 2999998.8
...

1500 = 1499999.4
= 0.0 + 1499999.4
= 999999.6 + 499999.8

When reading the clock only the full nanosecond part is used, while
timekeeping internally keeps nanosecond fractions. If the clock is now
updated at cycle 1500 here, a nanosecond is missing due to the truncation.

The simple fix is to round up the xtime value during the update, this also
changes the distance to the reference time, but the adjustment will
automatically take care that it stays under control.

I have no idea what he's trying to say with that "simplified example".
Where did that "1500" come from? There's a bit too much black magic
going on to make this comfortable.


>
> I suspect we may need to still round up and store the error, but tweak
> the adjustment code to handle the larger error per-iteration then it was
> originally designed for (note: the current code is still functioning
> properly, its just not often hitting the expected trivial case).

I'm afraid that may also do damage. This code needs real documentation
explaining in detail what the frick it's doing. Having someone that
shows up once ever 10 years that understands it is not something I feel
confident about.

>
> The only alternative would be to integrate the sub-ns remainder into the
> gettime caclculation (including reworking all the vsyscall
> implementations to utilize it as well).
>

First lets talk about all the issues, and perhaps even start documenting
what it currently does. The comments in the code are not enough for a
reviewer to understand the logic. I've spent today reading some RFC's to
understand more about NTP but, it still does not explain the magic
constants that are used in the code.

Thanks,

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/