Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(14)

Issue 524041: Changes to make upstart log performance stats to a file. (Closed)

Created:
10 years, 11 months ago by kmixter1
Modified:
9 years, 7 months ago
Reviewers:
petkov, scott, sosa
CC:
chromium-os-reviews_googlegroups.com
Visibility:
Public.

Description

Changes to make upstart log performance stats to a file. We currently log uptime and sectors read at each job state transition to /var/log/perf.log, enqueuing those writes which fail (such as when the stateful partition is not yet available at early boot time).

Patch Set 1 #

Total comments: 52

Patch Set 2 : Fixed some bugs, responded to petkov/sosa comments #

Patch Set 3 : Fix a lint error, create another (to match local style). #

Total comments: 3

Patch Set 4 : More tab/space stuff #

Unified diffs Side-by-side diffs Delta from patch set Stats (+668 lines, -65 lines) Patch
M debian/changelog View 1 chunk +6 lines, -0 lines 0 comments Download
M init/Makefile.am View 13 chunks +25 lines, -12 lines 0 comments Download
M init/Makefile.in View 30 chunks +96 lines, -52 lines 0 comments Download
M init/job.c View 2 chunks +2 lines, -0 lines 0 comments Download
M init/main.c View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
A init/perf_log.h View 1 chunk +33 lines, -0 lines 0 comments Download
A init/perf_log.c View 1 2 3 1 chunk +266 lines, -0 lines 0 comments Download
A init/tests/test_perf_log.c View 1 2 3 1 chunk +194 lines, -0 lines 0 comments Download
A make_pkg.sh View 1 1 chunk +15 lines, -0 lines 0 comments Download
M nih/string.c View 1 chunk +4 lines, -1 line 0 comments Download
M nih/tests/test_string.c View 2 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
kmixter1
10 years, 11 months ago (2010-01-06 02:01:49 UTC) #1
petkov
As usual, mostly nits... http://codereview.chromium.org/524041/diff/1/6 File init/main.c (right): http://codereview.chromium.org/524041/diff/1/6#newcode245 init/main.c:245: /* Start performance logging using ...
10 years, 11 months ago (2010-01-06 07:28:17 UTC) #2
sosa
Double check lint and make sure there are no new style errors other than those ...
10 years, 11 months ago (2010-01-06 23:47:12 UTC) #3
kmixter1
Addressed all comments. Scott: please note that I fixed a bug in nih_str_split which could ...
10 years, 11 months ago (2010-01-07 22:37:22 UTC) #4
petkov
LGTM (or at least -- I'm done reviewing).
10 years, 11 months ago (2010-01-08 01:15:31 UTC) #5
sosa
10 years, 11 months ago (2010-01-08 01:22:38 UTC) #6
LGTM, but it seems you introduced some weird spacing in some of your function
headers.  You might wanna check that out

http://codereview.chromium.org/524041/diff/5007/5013
File init/perf_log.c (right):

http://codereview.chromium.org/524041/diff/5007/5013#newcode179
init/perf_log.c:179: va_list	 args;
Did a tab sneak in here?

http://codereview.chromium.org/524041/diff/5007/5013#newcode240
init/perf_log.c:240: perf_log_job_state_change (Job	    *job,
Tab?

http://codereview.chromium.org/524041/diff/5007/5013#newcode241
init/perf_log.c:241: JobState  new_state)
Spacing?

Powered by Google App Engine
This is Rietveld 408576698