Skip to content
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 unwinder loading on 6.8 kernels #2667

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Conversation

umanwizard
Copy link
Contributor

See comments, and mailing list thread linked therein, for details. Before this change, recent kernels (6.8 and later) cannot verify the native unwinder, because improvements in the precision of register bounds tracking cause an explosion in state space resulting in exceeding the limit of 1 million instructions processed per load.

This commit works around that issue, as the comment in the code explains.

@umanwizard umanwizard requested a review from a team as a code owner April 4, 2024 04:34
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

Wow, interesting.

Happy to merge when CI is green!

@kakkoyun
Copy link
Member

kakkoyun commented Apr 4, 2024

Is the failure of the native unwinder integration tests on arm64 expected? That should have worked, but the rest could fail.

@umanwizard
Copy link
Contributor Author

No, it's not expected. I need to look into that, thanks.

@umanwizard umanwizard marked this pull request as draft April 4, 2024 13:31
umanwizard added a commit to umanwizard/parca-agent that referenced this pull request Apr 25, 2024
See PR parca-dev#2667 for details about why this is needed, as well as the
mailing list thread
https://lore.kernel.org/bpf/[email protected]/
.

The approach tried in that PR caused a regression on older kernels;
hopefully, this one will work while also being less confusing.
See comments, and mailing list thread linked therein, for
details. Before this change, recent kernels (6.8 and later) cannot
verify the native unwinder, because improvements in the precision of
register bounds tracking cause an explosion in state space resulting
in exceeding the limit of 1 million instructions processed per load.

This commit works around that issue, as the comment in the code explains.
This lets us inline the function without letting clang/llvm
optimize out the double xor.
@umanwizard umanwizard marked this pull request as ready for review April 26, 2024 19:40
@umanwizard
Copy link
Contributor Author

OK, this finally. @kakkoyun and @brancz , can you please have another look?

We can't use function calls; everything has to be inlined in order to work on old kernels. But if we let clang/llvm inline the function, it will optimize it away...

I solved this problem by just using inline volatile asm, which the compiler won't optimize. It's an ugly solution, but it seems to work.

@brancz brancz merged commit 13db3a4 into parca-dev:main Apr 30, 2024
11 of 12 checks passed
brancz pushed a commit that referenced this pull request May 7, 2024
See PR #2667 for details about why this is needed, as well as the
mailing list thread
https://lore.kernel.org/bpf/[email protected]/
.

The approach tried in that PR caused a regression on older kernels;
hopefully, this one will work while also being less confusing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants