-
Notifications
You must be signed in to change notification settings - Fork 192
scx_lavd: Lower the verifier pressure. #3033
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
Conversation
arighi
left a comment
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 looks good, but have you tried setting -Os or -Oz in BPF_BASE_CFLAGS for lavd? Maybe in this way clang can already produce smaller code without adding any extra overhead or requiring code changes (in some cases it might even bring better performance).
Thanks for the review, @arighi ! I didn't but I found that -E2BIG is more related to the number of branch instructions (especially loop) rather than the actual binary size. That makes sense considering the search space that the verifier should explore is exponentially increases as the number of branch instructions grows. |
pick_any_bit() often causes a -E2BIG verifier error because it has a 64-iteration loop. Let's replace the naive loop-based logic with loopless logic using circular rotation of the bitmap and the trailing zeros count (ctz). This is more verifier-friendly, faster, and even accelerated when __builtin_ctzll() is supported (on x86). Signed-off-by: Changwoo Min <[email protected]>
pick_most_loaded_cpu() contains nested loops, so it often causes an -E2BIG verifier error. Let's prevent inlining the function to reduce the verifier burden on its callers. Signed-off-by: Changwoo Min <[email protected]>
struct pick_ctx is allocated on the stack, and it is big! It imposes high stack pressure, causing an EACCESS verifier error in the worst case. Let's reduce its size by using bitfields for boolean flags. Signed-off-by: Changwoo Min <[email protected]>
Separate out variables using only in test_cpu_stickable() from struct pick_ctx and create struct sticky_ctx. This reduces the size of struct pick_ctx, lowering the stack pressure. test_cpu_stickable() also tweaked to workaround a BPF verifier error, "bitwise operator |= on pointer prohibited", on clang-18 for ARM64. Signed-off-by: Changwoo Min <[email protected]>
To reduce the size of struct pick_ctx, let's rearrange its fields to eliminate holes between them. Signed-off-by: Changwoo Min <[email protected]>
Clean up stack variables in lavd_select_cpu() tio reduce stack usage. Signed-off-by: Changwoo Min <[email protected]>
c22e76a to
bfebddb
Compare
|
Currently, CI fails due to the @JakeHillion -- What do you think? Do you think if it is okay to merge the PR and exempt lavd from veristat for a while until the bug is fixed? |
|
Hmm... |
scx_lavd has suffered from verifier errors in some kernel/compiler/architecture combinations, failing with
-E2BIG(the BPF program is too complex) or-EACCESS(the stack consumption is beyond 512 bytes).This PR aims to lower the verifier pressure:
For the
-E2BIGerrorFor
-EACCESSerrorstruct pick_ctx, which is allocated on the stack.lavd_select_cpu().No functional changes are intended.