New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 739573 link

Starred by 4 users

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

zram stats don't show up in feedback anymore as of R60

Project Member Reported by sonnyrao@chromium.org, Jul 6 2017

Issue description

I've seen two reports from R60 which are missing the zram information

instead we just see the following:
zram compressed data size=<empty>
zram original data size=<empty>
zram total memory used=<empty>
zram total reads=<empty>
zram total writes=<empty>
 
Mmm those are probably from the more recent kernels, where they changed the zram sysfs.
Cc: diand...@chromium.org
re #1 -- ah ok, I guess kernels which have applied this patch will have the issue

 c87d1655c2   UPSTREAM: zram: remove obsolete sysfs attrs

in  crbug.com/721955 

It looks like we fixed metrics daemon but didn't fix feedback reports or chrome:system

unfortunately the new interface is much harder to read -- maybe we can do a little parsing of the mm_stat file to make feedback reports easier to read.

Owner: semenzato@chromium.org
May I take this?
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4164de4b7dfd0315a0e56557a5fb9a1c344da427

commit 4164de4b7dfd0315a0e56557a5fb9a1c344da427
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Fri Jul 07 04:13:52 2017

debugd: support both old and new zram sysfs formats

Old kernels have individual entries in /sys/block/zram0 for
various quantities of interest.  Newer kernels put them all
together into mm_stat.  Kernels in between have both.  This
change supports both old and new formats by collecting
everything and ignoring what's missing.

BUG= chromium:739573 
TEST=none

Change-Id: Ia0ce5d08b5fb04e14539905dd67ecf9f4e24c3fd
Reviewed-on: https://chromium-review.googlesource.com/562296
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/4164de4b7dfd0315a0e56557a5fb9a1c344da427/debugd/src/log_tool.cc

Fixed? We need this on M60
Theoretically yes, fixed.  Want to test before backport?
did a manual test on canary -- doesn't quite look totally fixed -- we didn't get the line showing the names of the fields.
Oh bother.
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0253b140b7830939c81526c0add77ce44ef3563f

commit 0253b140b7830939c81526c0add77ce44ef3563f
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Sat Jul 08 04:30:07 2017

debugd: fix name/value display of new zram stats

Can't have two fields with the same name...

BUG= chromium:739573 
TEST=tested this time, with chrome://system

Change-Id: I43dfe5ee903782d7be643005038a659838be2de6
Reviewed-on: https://chromium-review.googlesource.com/564318
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/0253b140b7830939c81526c0add77ce44ef3563f/debugd/src/log_tool.cc

Cc: chungsheng@google.com vovoy@chromium.org
Sorry guys.  :(  I didn't realize I had to change two places.  It sounds like this is fixed now (thanks).  Please yell if there's something I can do to help.
No problem, I had forgotten about that as well.
Labels: Merge-Request-60
Requesting merge.  This is a very simple fix and has been tested.
Project Member

Comment 14 by sheriffbot@chromium.org, Jul 10 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: josa...@chromium.org
Owner: bhthompson@chromium.org
Labels: -Hotlist-Merge-Review -Merge-Review-60 Merge-Approved-60
Owner: semenzato@chromium.org
Merge approved if this has been verified on ToT. 
Project Member

Comment 18 by bugdroid1@chromium.org, Jul 10 2017

Labels: merge-merged-release-R60-9592.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0a5f755693a1494696da8a013f54b63f9f4b8627

commit 0a5f755693a1494696da8a013f54b63f9f4b8627
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Mon Jul 10 23:21:50 2017

debugd: support both old and new zram sysfs formats

Old kernels have individual entries in /sys/block/zram0 for
various quantities of interest.  Newer kernels put them all
together into mm_stat.  Kernels in between have both.  This
change supports both old and new formats by collecting
everything and ignoring what's missing.

BUG= chromium:739573 
TEST=none

Change-Id: Ia0ce5d08b5fb04e14539905dd67ecf9f4e24c3fd
Reviewed-on: https://chromium-review.googlesource.com/562296
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
(cherry picked from commit 4164de4b7dfd0315a0e56557a5fb9a1c344da427)
Reviewed-on: https://chromium-review.googlesource.com/565380

[modify] https://crrev.com/0a5f755693a1494696da8a013f54b63f9f4b8627/debugd/src/log_tool.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/78668bdd21d54f3e83833fdb894426bc86d2e9bb

commit 78668bdd21d54f3e83833fdb894426bc86d2e9bb
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Mon Jul 10 23:21:57 2017

debugd: fix name/value display of new zram stats

Can't have two fields with the same name...

BUG= chromium:739573 
TEST=tested this time, with chrome://system

Reviewed-on: https://chromium-review.googlesource.com/564318
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I43dfe5ee903782d7be643005038a659838be2de6
Reviewed-on: https://chromium-review.googlesource.com/565654
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/78668bdd21d54f3e83833fdb894426bc86d2e9bb/debugd/src/log_tool.cc

Project Member

Comment 20 by sheriffbot@chromium.org, Jul 14 2017

Cc: bhthompson@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-60 Merge-Merged
Status: Fixed (was: Assigned)

Comment 22 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment