Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix PSS graph on recent kernels #50

Merged
merged 1 commit into from
Aug 28, 2023
Merged

fix PSS graph on recent kernels #50

merged 1 commit into from
Aug 28, 2023

Conversation

amurzeau
Copy link
Contributor

It seems the smaps file layout in /proc changed in some kernel version. This commit fix it by:

  • Using smaps_rollup instead of smaps to avoid having to loop over all entries (smaps_rollup being a summary of smaps)
  • Searching "Pss:" instead of using hardcoded offsets (which will probably go wrong again in the future).

Fixes: #46

Note that the issue mentions the entropy graph too, but this one is working fine, only the PSS graph was corrupted (because of random value, some of them negative).

@bluca
Copy link
Member

bluca commented Aug 28, 2023

Is this change compatible with both old and new kernels?

@amurzeau
Copy link
Contributor Author

amurzeau commented Aug 28, 2023

The "Pss:" term exists since the PSS implementation (2.6.25, torvalds/linux@ec4dd3e), but the use of smaps_rollup requires a 4.14 kernel.
smaps_rollup was introduced here: torvalds/linux@493b0e9

To avoid the requirement of the 4.14 kernel, the PR needs to be changed to use smaps instead and compute the sum of Pss: for all occurrences.

@bluca
Copy link
Member

bluca commented Aug 28, 2023

Why not just check if the smaps_rollup file exists, and if so use the new implementation, and if not fallback to the previous one?

@amurzeau
Copy link
Contributor Author

amurzeau commented Aug 28, 2023

The previous way was assuming a fixed number of byte for each block in smaps.
There is no real reason except code it was easier to do like this, but this check can be implemented for sure.

It seems the smaps file layout in /proc changed in some kernel version.
This commit fix it by:
- Using smaps_rollup (available since kernel 4.14) instead of smaps to
  avoid having to loop over all entries (smaps_rollup being a summary
  of smaps).

- If smaps_rollup is not available (kernel < 4.14), use smaps instead as
  before and sum all PSS lines.

- Searching "Pss:" instead of using hardcoded offsets (which will
  probably go wrong again in the future).

Fixes: systemd#46
@amurzeau
Copy link
Contributor Author

I've introduced the check for the presence of smaps_rollup.
I've also replaced continue case of failure to open smaps with goto catch_rename to not skip remaining sampling operations.

@bluca bluca merged commit b8c1c50 into systemd:main Aug 28, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Entropy and memory graph heights explode
2 participants