Better stack walking APIs #448
Conversation
The StackWalker iterator lets us call `stack_trace.walk(id)` to iterate through the addresses in any given stack. The constructor of this iterator takes an optional `resolver` to convert the addresses in the iterator into symbols (or to format them according to the users' needs).
This simple example script traces all calls to `malloc` in a process and prints the callsite using the new `BPF_STACK_TRACE` table API.
| @@ -412,9 +412,37 @@ def __init__(self, *args, **kwargs): | |||
| raise Exception("Unsupported") | |||
|
|
|||
| class StackTrace(TableBase): | |||
| MAX_DEPTH = 127 | |||
There was a problem hiding this comment.
This value is hardcoded in the kernel. I am not sure whether it'd be worth it to dig into the FFI ctypes to figure it out dynamically. Thoughts?
There was a problem hiding this comment.
@4ast it makes sense to me to leave this unless the kernel value is ever likely to change. How likely is that?
The problem with getting this dynamically will be that the kernel headers may update but the libc-headers, where the userspace value of this will be most readily available, is very slow to update. I tend to come across (and am one myself) lagging userspace header files, which is why bcc includes a bunch of compat header files and hardcoded things.
There was a problem hiding this comment.
PERF_MAX_STACK_DEPTH is highly unlikely to change. All sorts of abi concerns will pop up, including increase in size of perf.data files, since 127 is also default for 'perf record -g'
There was a problem hiding this comment.
Good! Let's leave it hardcoded then.
|
Looks awesome! By the way, when this gets merged, the man and example files also need to be updated to point out that the stack support is not experimental anymore, and to remove the stack depth command line arguments. |
|
It might make sense to split out the updates to table.py into a separate PR, since that can merge right away, while the others we might want to discuss about backwards compat. |
|
Thanks! I was looking for this specific support yesterday and saw it hadn't been done yet. Might want a hello world simple example in /examples, or, (either now or later) we can update stackcount/stacksnoop in /tools which can serve as the same thing. Also, as tools get rewritten to use the new stack support, I'd put old copies of those tools (which used unrolling hacks) in /tools/old -- particularly for Ubuntu Xenial users. FWIW, the stackdepth limit of 127 will be suitable for many cases, but we're already exceeding it for Java/Groovy stacks (which can get as deep as >512). |
|
While we are at it, would it make sense to extend the user symbol resolution support with data from perf-map-agent? That would give us call stacks for Java and Node, right? @brendangregg |
|
@goldshtn right, it does make sense, now or later. User symbol resolution should check for /tmp/perf-PID.map files. Format is tools/perf/Documentation/jit-interface.txt . |
Done! This PR now only contains the changes to I've opened #450 where I'm working on converting the actual tools to use 4.6.
This is done in #450.
Also done in #450 👌 |
Hi folks!
Here's a hot take on replacing @brendangregg's lovingly crafted eBPF stack walker code (teehee) with the brand new StackTrace table APIs.
I totally get the kernel is not quite there yet, but the APIs in Torvald's mainline seem to be stabilized now and running a nightly kernel makes this work like a charm. I'll just twiddle my fingers until 4.6 comes out, but I wanted to open this PR early to discuss it. 👌
Looking at the (minimal) changes, the design for the new table seems just right, and the resulting Python APIs make all the stack-related code significantly simpler. Details on the changes are described in each individual commit message.
There's a couple points I'm not sure about -- I'm leaving inline comments with questions.
Thanks for the amazing work with
bcc. This is by far the coolest thing going on right now re. perf work in Linux.