Re: [PATCH 2/2] time: Cleanup offs_real/wall_to_mono and offs_boot/total_sleep_timeupdates

From: John Stultz
Date: Mon Jul 23 2012 - 15:34:18 EST

On 07/19/2012 02:33 AM, Ingo Molnar wrote:
* John Stultz <john.stultz@xxxxxxxxxx> 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.


@@ -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

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.


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at