Messages in this thread | | | Date | Sat, 7 Mar 2015 10:29:29 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 05/12] time: Add debugging checks to warn if we see delays |
| |
* John Stultz <john.stultz@linaro.org> wrote:
> Recently there's been some request for better sanity > checking in the time code, so that its more clear > when something is going wrong since timekeeping issues > could manifest in a large number of strange ways with > various subsystems. > > Thus, this patch adds some extra infrastructure to > add a check update_wall_time to print warnings if we > see the call delayed beyond the max_cycles overflow > point, or beyond the clocksource max_idle_ns value > which is currently 50% of the overflow point.
Just a changelog style nit, but isn't this easier to read:
Thus, this patch adds some extra infrastructure to add a check to update_wall_time() to print warnings if we see the call delayed beyond the 'max_cycles' overflow point, or beyond the clocksource 'max_idle_ns' value which is currently 50% of the overflow point.
?
To mark functions with parentheses and arguments in quotes, to make them mix better with free form English sentences?
> +#ifdef CONFIG_DEBUG_TIMEKEEPING > +static void timekeeping_check_update(struct timekeeper *tk, cycle_t offset) > +{ > + > + cycle_t max_cycles = tk->tkr.clock->max_cycles; > + const char *name = tk->tkr.clock->name; > + > + if (offset > max_cycles) > + printk_deferred("ERROR: cycle offset (%lld) is larger then" > + " allowed %s max_cycles (%lld)\n", > + offset, name, max_cycles); > + else if (offset > (max_cycles >> 1)) > + printk_deferred("WARNING: cycle offset (%lld) is past" > + " the %s 50%% safety margin (%lld)\n", > + offset, name, max_cycles>>1);
s/larger then/larger than
Also, please don't break user visible messages on col80 boundaries just to pacify checkpatch: ignore checkpatch in these cases.
Also, plase use curly braces on multi-line statements, plus I'd shape it like this:
if () { } else { if () { } }
That way the second 'if' condition in the else branch does not get lost in the noise of silly line breaks. (At least during email review: in an actual editor syntax highlighting helps.)
> +config DEBUG_TIMEKEEPING > + bool "Enable extra timekeeping sanity checking" > + help > + This option will enable additional timekeeping sanity checks > + which may be helpful when diagnoising issues where timekeeping
Typo.
There's really a disproportionate ratio of typos, considering how many iterations this patch-set has been through :-/
Or maybe I'm oversensitive to small details.
Thanks,
Ingo
| |