-
Notifications
You must be signed in to change notification settings - Fork 18
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
Display loaded modules, their dependencies and parameter set #33
Conversation
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.
This will be a quite useful helper, thank you.
I do actually believe that this would be even better suited for contributing directly to drgn. While drgn already has a basic lsmod feature (see contrib/lsmod.py
), helpers that can load module parameters would be nice to contribute upstream. We can talk about more after you've gone through the suggestions here.
6f632e4
to
f8c4db5
Compare
Not sure if you're ready for review yet: there are still some failures on the gitlab vmcore tests. Let me know when you're ready for another look! |
aabdc30
to
bc7c151
Compare
drgn_tools/lsmod.py
Outdated
try: | ||
param_type = prog.symbol(kp.ops.get).name | ||
except LookupError: | ||
continue |
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.
In the case of a LookupError, I think we can still print out the name, and indicate that we didn't know what to do with it. EG:
table_value.append([kp.name.string_().decode("utf-8"), kern_parm, "UNKNOWN", "UNKNOWN"])
continue
This way, module parameters are not skipped over just because we had trouble with them.
drgn_tools/lsmod.py
Outdated
try: | ||
parm_valf = parm_valf_address[0].value_() | ||
except FaultError: | ||
parm_valf = "(page fault)" | ||
continue |
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.
Did you encounter page faults with integer types? I wouldn't expect that. If you're not working around a known issue, I'd prefer you not catch this error.
Regardless, with the continue
statement here, the parameter will get skipped and won't be added to the list in the FaultError
case. We should be including all parameters even on error. It would be best to remove the continue
statement.
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.
Yes we have encountered and below is the localtest done on the vmcore
[apalakun@lse-india-drgn drgn-tools]$ python3 -m drgn_tools.corelens /drgn_vmcore/githubCI/rds-uek5/vmcore --debuginfo /drgn_vmcore/githubCI/rds-uek5/ -M lsmod > ~/4.txt
error: Encountered exception in lsmod:
Traceback (most recent call last):
File "/drgn_home/apalakun/github/drgn-tools/drgn_tools/corelens.py", line 340, in run_module
mod.run(prog, args)
File "/drgn_home/apalakun/github/drgn-tools/drgn_tools/lsmod.py", line 157, in run
print_module_parameters(prog)
File "/drgn_home/apalakun/github/drgn-tools/drgn_tools/lsmod.py", line 72, in print_module_parameters
parm_valf = parm_valf_address[0].value()
_drgn.FaultError: could not read memory from kdump: Cannot get page I/O address: Page not present: pte[477] = 0x0: 0xffffffffc05dd000
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.
That's wild, I guess it is also __initdata
!
drgn_tools/lsmod.py
Outdated
str(parm_valf), | ||
] | ||
) | ||
ker_parm = hex(int(ker_parm, 16) + int(40)) |
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.
I'm assuming that 40 is the size of struct kernel_param
and you're adjusting the hex value to point to the next parameter?
This is contrary to the entire point of drgn: it knows the size of structures, and it can tell you the address of objects. Rather than maintaining your own variable, and hard-coding structure sizes, let's have drgn maintain everything for us.
Please delete the kpr_parm
variable entirely. When you need the hex address of the struct kernel_param
, you can do hex(kp.address_of_())
.
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.
added string "array_length" to the output showing the length of the array
MODULE NAME: parport_pc
PARAM COUNT: 5
ADDRESS : 0xffffffffc0327390
PARAMETER ADDRESS TYPE VALUE
init_mode 0xffffffffc03268a0 charp (null)
dma 0xffffffffc03268c0 array array_length: 16
irq 0xffffffffc03268e0 array array_length: 16
io_hi 0xffffffffc0326900 array array_length: 17
io 0xffffffffc0326920 array array_length: 17
Signed-off-by: Anil Palakunnathu Kunnengeri <[email protected]>
Ok, I think the logic for the module parameter parsing all looks sound. However, I think the
I pushed a commit to my own branch which modifies your implementation and addresses those issues, just to illustrate my point. I tried to simplify things as much as possible, and I split it out into three main functions:
Here is my patch. We can just use that as-is, or if you want to incorporate the ideas yourself in your own way, that's great too. Just let me know. |
From email discussion, we'll merge this and apply my patch on top. |
No description provided.