-
Notifications
You must be signed in to change notification settings - Fork 165
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 test failures on SLOB / tiny #416
Conversation
Oh, boy, also, Fixes #415, which I did not include in a commit message, but that's probably ok. |
A manual test run on my fork with the "test-all-kernel-flavors" option set: (but not all python versions) |
This implementation doesn't run all kernel flavors on push to |
Oh fun, no space left on device! Looks like we'll need to avoid pre-fetching all kernels and we'll need a way to delete them after testing, for CI only: |
Would you mind splitting the setup.py and CI changes into a separate PR? I'll take a closer look tomorrow, thanks again! |
Yeah that seems wise. I'll split them up in the morning! |
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.
Looks good other than one comment, thanks!
drgn/helpers/linux/mm.py
Outdated
start_addr = pfn_to_virt(prog["min_low_pfn"]).value_() | ||
end_addr = (pfn_to_virt(prog["max_low_pfn"]) + prog["PAGE_SIZE"]).value_() | ||
return start_addr <= addr < end_addr | ||
except NotImplementedError: |
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.
Don't bother catching this exception. The helper should fail hard if virtual address translation isn't supported like the other mm helpers.
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.
Would you like this helper's callers to fail on this as well, or should I push that exception handler up to each of the three callers?
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.
Let's keep it the way it currently is: _identify_kernel_address()
should catch it, but the SLAB helpers should fail.
In the Fixes commit, as part of the refactor, the _identify_kernel_address() function started open-coding the get_slab_object_info() function. Unfortunately, this resulted in the function failing when CONFIG_SLOB was enabled, since the AttributeError for accessing "slab.slab_cache" was not caught. Open-coding was done to avoid the duplicate bounds check for in_direct_map. In fact, in_direct_map seems like a useful helper itself, and by factoring it out of _find_containing_slab() and into its callers, we can simplify things a bit. Since _find_containing_slab() can handle SLOB, this fixes the test failure too. Fixes: 48e0c51 ("helper.common.memory: recognize vmap addresses in identify_address()") Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
The test for print_annotated_memory() shouldn't assert anything when CONFIG_SLOB is enabled, because we can't find slab objects. However, the test shouldn't be skipped entirely, because print_annotated_memory() should run without error in this case. Skip the assertion only. Fixes: c771728 ("helpers.common.memory: add print_annotated_memory() helper") Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
I also added a
-f
option to thesetup.py
because I wanted to run the tests locally and noticed I had no easy way to say "just run the tiny tests". I didn't bother trying to add any logic to make-f
and-F
conflict... it's a developer tool after all.