(re-open) ZRAM info collector module (proc.plugin)#6424
Merged
vlvkobal merged 8 commits intonetdata:masterfrom Aug 12, 2019
Merged
(re-open) ZRAM info collector module (proc.plugin)#6424vlvkobal merged 8 commits intonetdata:masterfrom
vlvkobal merged 8 commits intonetdata:masterfrom
Conversation
ZRAM: Implemented zram device id detection ZRAM: Implemented zram device enumeration WIP ZRAM: Memory usage graph (needs other graphs) ZRAM: Added ratio and efficiency graph ZRAM: Added chart description and context names, code formatting
RaZeR-RBI
requested review from
cakrit,
gmosx,
ktsaou,
paulkatsoulakis and
vlvkobal
as code owners
ilyam8
reviewed
Jul 10, 2019
| { | ||
| snprintfz(filename, FILENAME_MAX, "/dev/%s", de->d_name); | ||
| if (unlikely(stat(filename, &st) != 0)) | ||
| { |
Member
There was a problem hiding this comment.
Hey, it would be nice to not add one more code style
https://docs.netdata.cloud/contributing/#code-style
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces
Contributor
|
@RaZeR-RBI I see the WIP label is still attached. Are you going to make changes to the PR? |
Contributor
Author
I've attached the WIP label in order to prevent merging due to reasons explained in the first post. If everything is ok (the bug is fixed) or if the label is misleading we can remove it 😉 |
|
|
vlvkobal
changed the title
vlvkobal
approved these changes
Aug 9, 2019
cakrit
approved these changes
Aug 9, 2019
Contributor
cakrit
left a comment
cakrit
left a comment
There was a problem hiding this comment.
I no longer get the crash I was getting before.
paulkatsoulakis
approved these changes
Aug 9, 2019
jackyhuang85
pushed a commit
to jackyhuang85/netdata
that referenced
this pull request
Jan 1, 2020
* ZRAM collector module ZRAM: Implemented zram device id detection ZRAM: Implemented zram device enumeration WIP ZRAM: Memory usage graph (needs other graphs) ZRAM: Added ratio and efficiency graph ZRAM: Added chart description and context names, code formatting * ZRAM: Proper handling of zram device removal * ZRAM: Added additional checks, removed redundant logging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-opening #6276 (initial PR with ZRAM module that was merged) in response to #6412 (double free and corruption).
I haven't reproduced the bug on my machines yet (my guess is that it may be tied to Linux distribution), but I've added a pair of additional checks that (possibly) could crash the app.
Any info about the environment where the bug was encountered is very appreciated. Sorry for inconvenience 😳