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

kernfs_memcg: Add helpers to gather memcgroup related data #96

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

imran-kn
Copy link
Contributor

@imran-kn imran-kn commented Aug 1, 2024

This as of now is just a dump of some of my bespoke debug scripts.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 1, 2024
@imran-kn imran-kn changed the title DRAFT: kernfs_memcg: starting work. kernfs_memcg: Add helpers to gather memcgroup related data Oct 21, 2024
@imran-kn
Copy link
Contributor Author

This as of now is just a dump of some of my bespoke debug scripts.

I have added other helpers and modified the earlier ones, so that they work with other UEK versions as well

Copy link
Member

@biger410 biger410 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating memcg helpers. I added couple comments. And beside that, please create a bug for it and put the number in the git log.

"""
Dump hierarchy of active mem cgroups.
"""
cgroup_subsys = prog["cgroup_subsys"][4]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment why it's 4 here, what is the kernel definition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 is cgroup subsystem id for memory cgroups. I have replaced it with a const, explaining what is represents

print(f"dumping: {cgroup_subsys.name.string_().decode()} hierarchy")
for pos in css_for_each_descendant_pre(css):
print(
f"path: {cgroup_path(pos.cgroup).decode()} flags: 0x{pos.flags.value_():x}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make flags human readable? For example, mention zombie cgroup not 0x0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Now the dump looks like the snippet shown below:
`age: 0xffffc35c99dbb040 cgroup: /user.slice/user-0.slice/session-7128.scope state: CSS_ONLINE|CSS_VISIBLE path: oracle_230124_0745EST/diag/asm/cell/scaqat06celadm02/incpkg/pkg_121/seq_1/cell/cellcli.lst.root.0

page: 0xffffc35c99dbb080 cgroup: /user.slice/user-0.slice/session-7128.scope state: CSS_ONLINE|CSS_VISIBLE path: oracle_230124_0745EST/diag/asm/cell/scaqat06celadm02/incpkg/pkg_121/seq_1/cell/cellsrv_pid.lst

page: 0xffffc35c99dbb0c0 cgroup: /user.slice/user-0.slice/session-7128.scope state: CSS_ONLINE|CSS_VISIBLE path: oracle_230124_0745EST/diag/asm/cell/scaqat06celadm02/incpkg/pkg_121/seq_1/cell/cellsrv_pid.lst

page: 0xffffc35c99dbb100 cgroup: /user.slice/user-0.slice/session-7128.scope state: CSS_ONLINE|CSS_VISIBLE path: oracle_230124_0745EST/diag/asm/cell/scaqat06celadm02/incpkg/pkg_121/seq_1/cell/cellsrv_pid.lst
.........................................................................
page: 0xffffc35c99dbb140 cgroup: /user.slice/user-0.slice/session-7126.scope state: ZOMBIE path: oracle_230124_0745EST/diag/asm/cell/scaqat06celadm02/incpkg/pkg_121/seq_1/cell/cellsrv_pid.lst
`
Please let me know if it looks okay now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "state" looks good, but the file path doesn't look like a full path, it looks like a relative path to some mounted volume, please fix it.

Copy link
Contributor Author

@imran-kn imran-kn Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed path info. It turns out neither dentry_path nor inode_path give the full path (including the mount point). drgn_tools.dentry.dentry_path_any_mount gives the full path

continue
dentry = Object(prog, "struct dentry", address=dentry_addr)
print(
f"page: 0x{page.value_():x} cgroup: {cgroup_path(cgrp).decode()} flags: {cgrp.self.flags.value_()} dpath: {dentry_path(dentry.address_of_()).decode()}\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sound like inode_path drgn helper can get the path from inode, with that, you can remove the logic to locate dentry.

Again please make that flags human readable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

imran-kn added a commit that referenced this pull request Nov 25, 2024
@imran-kn
Copy link
Contributor Author

Thanks @biger410 for reviewing this. I have addressed your review comments. Could you please have a look and let me know if you have any further feedback

Add kernfs based helpers to extract memcg related information,
like number of active and inactive memcgroups, page cache pages
pinning memcgroups etc.

Orabug: 37322867
Signed-off-by: Imran Khan <[email protected]>
Orabug: 37322867
Signed-off-by: Imran Khan <[email protected]>
Orabug: 37322867
Signed-off-by: Imran Khan <[email protected]>
@biger410
Copy link
Member

The new changes look good to me. One more request is that can we add a corelen module for it? It should run either with -M option or -A option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants