Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2188043003:
Linux: Increase storage size of vmstat data (Closed)
|
Created:
4 years, 4 months ago by Tom (Use chromium acct) Modified:
4 years, 4 months ago Reviewers:
Lei Zhang CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionLinux: Increase storage size of vmstat data
The linux kernel stores and prints the data associated with /proc/vmstat
as unsigned longs, so increase Chrome's storage of these fields
accordingly.
https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf844296/include/linux/vmstat.h#L25
Committed: https://crrev.com/2b684a8a660073c2ae0f0967a394545eff255a91
Cr-Commit-Position: refs/heads/master@{#408774}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Fix test #
Total comments: 2
Patch Set 3 : Swap EXPECT order #
Messages
Total messages: 25 (16 generated)
Description was changed from ========== Linux: Increase storage size of vmstat data BUG= ========== to ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ==========
thomasanderson@google.com changed reviewers: + thestig@chromium.org
The CQ bit was checked by thomasanderson@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ========== to ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ==========
Additionally, the man page for proc(5) specifies that most fields in /proc/meminfo are unsigned long, but we're using ints for those as well. Right now, >4TB of memory is unlikely, but we may also want to widen the other fields in the future.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
Description was changed from ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ========== to ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ==========
https://codereview.chromium.org/2188043003/diff/1/base/process/process_metric... File base/process/process_metrics_linux.cc (right): https://codereview.chromium.org/2188043003/diff/1/base/process/process_metric... base/process/process_metrics_linux.cc:684: meminfo->pswpin = val; Can we just make pswpin and friends uint64_t? Aren't there systems where long is only 4 bytes?
Oh, and red trybots.
The CQ bit was checked by thomasanderson@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/07/28 18:38:05, Lei Zhang wrote: > https://codereview.chromium.org/2188043003/diff/1/base/process/process_metric... > File base/process/process_metrics_linux.cc (right): > > https://codereview.chromium.org/2188043003/diff/1/base/process/process_metric... > base/process/process_metrics_linux.cc:684: meminfo->pswpin = val; > Can we just make pswpin and friends uint64_t? Aren't there systems where long is > only 4 bytes? Yes, but then the metrics will be counted using 4 bytes in the kernel as well.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2188043003/diff/20001/base/process/process_me... File base/process/process_metrics_unittest.cc (right): https://codereview.chromium.org/2188043003/diff/20001/base/process/process_me... base/process/process_metrics_unittest.cc:289: EXPECT_EQ(meminfo.pswpin, 179LU); Flip the arguments to expected / actual, like in every other CL from today. :)
https://codereview.chromium.org/2188043003/diff/20001/base/process/process_me... File base/process/process_metrics_unittest.cc (right): https://codereview.chromium.org/2188043003/diff/20001/base/process/process_me... base/process/process_metrics_unittest.cc:289: EXPECT_EQ(meminfo.pswpin, 179LU); On 2016/07/29 00:50:13, Lei Zhang wrote: > Flip the arguments to expected / actual, like in every other CL from today. :) Done.
The CQ bit was checked by thomasanderson@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org Link to the patchset: https://codereview.chromium.org/2188043003/#ps40001 (title: "Swap EXPECT order")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ========== to ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... ========== to ========== Linux: Increase storage size of vmstat data The linux kernel stores and prints the data associated with /proc/vmstat as unsigned longs, so increase Chrome's storage of these fields accordingly. https://github.com/torvalds/linux/blob/e4fba88d00d21f50239eac7d653027bfaf8442... Committed: https://crrev.com/2b684a8a660073c2ae0f0967a394545eff255a91 Cr-Commit-Position: refs/heads/master@{#408774} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/2b684a8a660073c2ae0f0967a394545eff255a91 Cr-Commit-Position: refs/heads/master@{#408774} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
