On 07/19/2012 02:33 AM, Ingo Molnar wrote:
* John Stultz <john.stu...@linaro.org> wrote:
+static void tk_set_sleep_time(struct timekeeper *tk, struct timespec t)
+{
+ /* Verify consistency before modifying */
+ WARN_ON_ONCE(tk->offs_boot.tv64 !=
+ timespec_to_ktime(tk->total_sleep_time).tv64);
asserts like this can be put into a single line - we rarely need
to read them if they don't trigger - and making them
non-intrusive oneliners is a bonus.
Ack.
@@ -456,8 +478,8 @@ int timekeeping_inject_offset(struct timespec *ts)
tk_xtime_add(&timekeeper, ts);
- timekeeper.wall_to_monotonic =
- timespec_sub(timekeeper.wall_to_monotonic, *ts);
+ tk_set_wall_to_mono(&timekeeper,
+ timespec_sub(timekeeper.wall_to_monotonic, *ts));
There's a *lot* of unnecessary ugliness here and in many other
places in kernel/time/ due to repeating patterns of
"timekeeper.", which gets repeated many times and blows up line
length.
Please add this to the highest level (system call, irq handler,
etc.) code:
struct timekeeper *tk = &timekeeper;
and pass that down to lower levels. The tk-> pattern will be the
obvious thing to type in typical time handling functions.
This increases readability and clarifies the data flow and might
bring other advantages in the future.
Sounds good. Are you ok if this is done in a follow-on patch?
Stray newline.
I see a pattern with the newlines there - and this isnt the
first patch from you that has that problem.
Yea, I personally prefer more space between functions, so its a bad
habit I have (and checkpatch doesn't catch). I'll try to be more
watchful of it going forward.
thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
