python: combine python and native unwinder to avoid tail call limits

[gnurizen]

python: combine python and native unwinder into single loop

Python, especially pytorch programs can exhaust the tail call limit
by switching from python to native unwinders more than 29 times.
This happens because of eval/delegation patterns where one python
frame will be decorated with a couple native frames.

In order to unwind these stack successfully fold the native unwinder
into the python unwinder so at each frame a python or native frame
can be unwound.

Replace the separate walk_python_stack inner loop and outer
transition loop with a single switch-in-loop structure using
step_python and step_native helper functions. This reduces
tail call usage from one per batch to one per loop budget
exhaustion.

Move native unwinder map externs (exe_id_to_*_stack_deltas,
stack_delta_page_to_info, unwind_info_array) out of the
TESTING_COREDUMP guard in extmaps.h so python_tracer.ebpf.c
can include native_stack_trace.h.

Python loop iters is now a ro_vars entry so it can be set low by
default and jacked up with debug_prints are disabled which allows for
much bigger stacks. 29 * 12 (384) remains the deepest python stack we
support but its 29 * 4 w/ debug prints enabled.

[gnurizen]

New coredump test: https://drive.google.com/file/d/1c6fCXz7UoMuPu-Gg-bHUluYucILv2gIb

[gnurizen]

@fabled @florianl tagging you guys for review consideration, no hurry just want to make sure this gets on the appropriate radars. Thanks!

[fabled]

Seems ruby has same issue. See #1335 .

I wonder if something more elaborate could be done. Or is it better to bundle native unwinder with the interpreters that need it due to mixing native/HLL frames every few frames.

[gnurizen]

Rebased to PR #1286. Yeah I'd love to get @dalehamel 's thought on the applicability of this approach to the Ruby situation.

[dalehamel]

Seems ruby has same issue. See #1335 .

I wonder if something more elaborate could be done. Or is it better to bundle native unwinder with the interpreters that need it due to mixing native/HLL frames every few frames.

Yes especially in production we see this problem. With yjit, the problem is masked by the fact that the jit is only the leaf frame and we don't run with jit frame pointers in production for performance reasons.

However the ruby unwinder is already quite instruction heavy as we need to do the complex CME resolution for each ruby frame, so it might be hard to get all frames if we add the native unwinder into it too. In reality we mostly really care about the actual 'ruby' stack state (which shouldn't really matter if the frame is jit or interpreter backed, as it might be either with zjit) + jit leaf state the majority of the time and that's been fine for our purposes.

If we could manage to actually continue the native unwinding without exhausting tail calls, that would certainly be the best of both worlds, but i wouldn't say it's the highest priority.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: gnurizen / name: Tommy Reilly (173671d, 23507cb, 501e196, f020348)

[gnurizen]

This has been rebased to main, is passing all the kernel tests and should be green when the new coredump test is uploaded. @florianl @fabled if you could give this another pass when you can that would be much appreciated!

[gnurizen]

@florianl @fabled what are your thoughts on splitting off the first commit here and trying to land that first?

[gnurizen]

@fabled @florianl friendly ping on this. What if this change was a runtime opt-in (or could be build time I guess) so we could land it w/o changing anything functionally and Parca could just switch it on for our agent? ebpf programs are small, no harm in having some extra ones in the binary. I haven't worked out exactly how that would work but seems doable.

[fabled]

I think this is probably the simple thing to go forward to solve a real problem at the hand. I'd prefer to do something else, but it probably is a bigger job and/or not feasible at this time. So I'm ok to do this at this time. Let's keep do this just for everyone (no opt-in switch imho). The less there is configuration / build/runtime switch the more maintainable it is.

@christos68k @florianl Thoughts?

[christos68k]

I think this is probably the simple thing to go forward to solve a real problem at the hand. I'd prefer to do something else, but it probably is a bigger job and/or not feasible at this time. So I'm ok to do this at this time. Let's keep do this just for everyone (no opt-in switch imho). The less there is configuration / build/runtime switch the more maintainable it is.

@christos68k @florianl Thoughts?

I'm fine for now with this PR as-is, the change is not that extensive and it's conceptually clean and fairly simple. The alternative @gnurizen proposed here is also fine, though I prefer the current PR.

[gnurizen]

I redid the math on the number of frames we do, #1422 really opens up the doors on the upper bound on the *_PER_PROGRAM frame loops! But alas we still need this patch to get around the tail call limit with pytorch's checkered zebra stacks.

@florianl can we make the next step uploading the coredump test so we can get a green CI here? See this comment: #1288 (comment)

[fabled]

can we make the next step uploading the coredump test so we can get a green CI here? See this comment: #1288 (comment)

Uploaded and reran tests now. Can you merge with main and update ebpf blobs? Thanks!