-
Notifications
You must be signed in to change notification settings - Fork 20
Add the list_lru interator helper #74
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
base: main
Are you sure you want to change the base?
Conversation
No hurry with this helper. I am writing a test that walks through a memcg aware lru (any filesystem's dentry lru) and a memcg unaware lru (the xfs_buf lru if the filesystem type is xfs). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're already doing a respin, I wanted to give a little bit of feedback that should reduce the round trips on this review. The code logically is looking great, but stylistically, there's some things that don't fit with the project's existing code style. I don't want to waste developer's time or mental effort on code style though, so we use some automatic code formatters & linters to help catch these things at commit time.
It's documented here, the checks happen via a pre-commit hook. You can set it up via:
# OL9
dnf install -y pre-commit
# OL8
dnf install python39-pip
python3.9 -m pip install --user pre-commit
# Then, for both:
cd path/to/drgn-tools
pre-commit install --install-hooks
Next time you commit (including amending a commit), the hook will check code. Some checks (e.g. code style) automatically make the changes to your code, so you'll just need to git add
those changes. Other checks are just linters, so they may surface an issue you'll need to manually address.
I wrote a simple drgn script that uses list_lru to count the dentry and negative dentry for one or more fileystems using for_each_mount(). https://github.com/marktinguely/examples/blob/main/count_neg_dentry.py |
75d3467
to
8643d71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the inexcusable delay here. I picked these changes and played around with them in the dentry cache. I ended up having some improvements I'd like to see, but otherwise I'm excited to get these merged.
da31d9c
to
83e7c52
Compare
I have been debating for a while about changing the API returning the numa node id and memcg with the list_lru_for_each_entry(). The caller can easily ignore them with a "_, _, object = list_lru_for_each_entry(llru)". I still have to split list_lru_from_memcg_node_for_each_entry() into two routine and there is a lot of testing to redo. I ran it on LUCI for giggles and it works. |
4c0dc97
to
2605bf6
Compare
894c19a
to
ffcb879
Compare
Please do ping me on Slack when you're ready for additional review & merge, that way I know when to check your update (and I don't delay too long). Thanks! |
I dId the split of the foreach into list/entry. I changed the API to return the memcg and NUMA node id back with the lru entry. Today, I decided to use this to do a quick (just verify the memcg and NUMA node O(n)) versus deep test (walk the memcg and NUMA node subtree to find the entry which is basically O(n^2)). This has been tested on uek5 to Luci. I tried on a 1, 2 and 8 node NUMA. With a few to 5 million dentrys. The slab code doesn't give me what I need to find the memcg. The memcg and NUMA node from kmem routines are really would be used in a test environment. |
drgn_tools/list_lru.py
Outdated
if not list_empty(llru1.list.address_of_()): | ||
yield llru1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just like above, let's return the empty list_lru_one
here.
6d08edc
to
1bc9934
Compare
The drgn iterator for list_lru and test that walks filesystem(s) and verifies the memcg and NUMA node id. Provides the functions: list_lru_for_each_list() iterate the list_lru and return each list_lru_one, the NUMA node and memcg number. list_lru_for_each_entry() iterate the list_lru and return each entry of specified type, the NUMA node and memcg number. list_lru_from_memcg_node_for_each_list() iterate the list_lru for the specified NUMA node and memcg id and return each list_lru_one. list_lru_from_memcg_node_for_each_entry() iterate the list_lru for the specified NUMA node and memcg id and return each entry of specified type. Helpers: slab_object_to_memcgidx() return the memcg index for the specified list_lru slab object. slab_object_to_nodeid() return the NUMA node id for the specified list_lru object. The test defaults to the quick verification of the information from list_lru_for_each() but adding "verify", the test walks the memcg/NUMA node portion of the list_lru to verify the entry exists. The test raises an exception if the memcg or nodeid lookup does not match the reported value. The optional arguement, "maxitems", limits the number of checked items in a filsystem. Signed-off-by: Mark Tinguely <[email protected]>
list_lru iterators for UEK5, UEK6, UEK7 and UEK8.
The list_lru iteration can iterate all memcg and NUMA nodes or one memcg index and one NUMA node.
list_lru_kmem_to_memcgidx() is provided to calculate the memcg index for a list_lru kvm address. This routine is written for list_lru kernel address and assumes slab allocated addresses.
list_lru_kmem_to_nodeid() is provided to calculate the NUMA node id for a list_lru kvm address.
tests/test_list_lru.py uses every function to walk filesystem(s) and gets the memcg index and NUMA node id of the dentry (and xfs_buf on XFS filesystems) and uses those values to verify that list_lru_kmem_to_memcgidx() finds the entry.
There is an example that counts negative dentry at:
https://github.com/marktinguely/examples/blob/main/count_neg_dentry.py