Helper to dump ftrace buffer #162
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
|
wip: fixing some .dat formatting issue |
richl9
force-pushed
the
richard/ftrace
branch
2 times, most recently
from
1918fa0 to
7c6b874
Compare
richl9
force-pushed
the
richard/ftrace
branch
4 times, most recently
from
8d4d837 to
86f44f9
Compare
Orabug: 37944905 Signed-off-by: Richard Li <[email protected]>
|
|
||
| from drgn_tools.corelens import CorelensModule | ||
|
|
||
| PAGE_SIZE = 4096 |
There was a problem hiding this comment.
I would prefer you not store this as a constant. It's totally fine to load it once at the beginning of a function. Believe it or not, it's quite efficient to get this from the Program. PAGE_SIZE is implemented as a special-case, which means that drgn doesn't need to do any work to determine its value.
https://github.com/osandov/drgn/blob/main/libdrgn/linux_kernel_object_find.inc.strswitch#L13-L16
https://github.com/osandov/drgn/blob/main/libdrgn/linux_kernel.c#L170-L182
| count = 0 | ||
| while True: | ||
| if count >= cpu_buffer.nr_pages: | ||
| break |
There was a problem hiding this comment.
Would this be simpler as:
for count in range(1, cpu_buffer.nr_pages):
....| if reader_page_addr: | ||
| linear_pages.append(reader_page_addr) |
There was a problem hiding this comment.
Reading the ring buffer source code, I'm not sure I understand this. The reader_page is fully removed from the ring buffer. If the ring buffer has overwritten, then the reader page may not be contiguous with any of the data in the ring buffer. Is it a good idea to include its data here?
Actually, I admit I'm not fully understanding this code, even as I've spent some time with kernel/trace/ring_buffer.c! I think some comments would be useful, and maybe you could walk me through it on zoom.
| magic = b"Dtracing6\0\0\0" | ||
| magic_tag = 0x17 if is_little_endian else 0x18 | ||
| f.write(magic_tag.to_bytes(1, "little")) | ||
| f.write(0x08.to_bytes(1, "little")) |
There was a problem hiding this comment.
0x08.to_bytes(1, "little")) could be replaced by the simpler b'\x08' literal. And the same could be done with the magic_tag above.
| f.write( | ||
| (0).to_bytes(4, "little" if is_little_endian else "big") | ||
| ) # event format | ||
| f.write( | ||
| (0).to_bytes(4, "little" if is_little_endian else "big") | ||
| ) # event systems | ||
| f.write( | ||
| (0).to_bytes(4, "little" if is_little_endian else "big") | ||
| ) # kallsyms | ||
| f.write( | ||
| (0).to_bytes(4, "little" if is_little_endian else "big") | ||
| ) # ftrace_printk | ||
| f.write( | ||
| (0).to_bytes(8, "little" if is_little_endian else "big") | ||
| ) # cmdlines |
There was a problem hiding this comment.
You might want to consider using the struct module for much of this. It's designed exactly for converting basic data types into a packed binary encoding, and it's a lot more concise then doing the .to_bytes() thing (which, admittedly, I did not know about, so thanks for teaching me that!).
You could replace this, for example, with:
| f.write( | |
| (0).to_bytes(4, "little" if is_little_endian else "big") | |
| ) # event format | |
| f.write( | |
| (0).to_bytes(4, "little" if is_little_endian else "big") | |
| ) # event systems | |
| f.write( | |
| (0).to_bytes(4, "little" if is_little_endian else "big") | |
| ) # kallsyms | |
| f.write( | |
| (0).to_bytes(4, "little" if is_little_endian else "big") | |
| ) # ftrace_printk | |
| f.write( | |
| (0).to_bytes(8, "little" if is_little_endian else "big") | |
| ) # cmdlines | |
| endian = '<' if is_little_endian else '>' | |
| f.write(struct.pack(f'{endian}LLLLQ', 0, 0, 0, 0, 0)) |
There are also tools for placing nul-padded constant width strings in (e.g. "flyrecord" below). I think with struct this whole function will get a lot simpler!
|
Hi @brenns10 , thanks for the comment. This is still wip. It will role out soon:) |
|
Got it, I've marked it as a draft, you can change it back once you're ready for review. |
No description provided.