-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
libbpf-tools: Allow biostacks to run on old kernels #4550
base: master
Are you sure you want to change the base?
Conversation
SEC("fentry/blk_account_io_start") | ||
int BPF_PROG(blk_account_io_start, struct request *rq) | ||
{ | ||
return trace_start(ctx, rq, false); |
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.
Missing the true
case. fentry
progs should have as many as the kprobe
ones.
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.
blk_account_io_{start, done} were renamed to _blk_account_io{start, done} from v5.16, see commit a184f09(libbpf-tools: Fix bio tools).
bpf_program__set_attach_target() can be used for that, fentry
progs are not always same as kprobe
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.
I mean blk_account_io_merge_bio
is missing.
libbpf-tools/biostacks.c
Outdated
"blk_account_io_start"); | ||
bpf_program__set_attach_target(obj->progs.blk_account_io_done, 0, | ||
"blk_account_io_done"); | ||
if (kprobe_exists("blk_account_io_start")) { |
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.
The userspace part looks way to complicated, could you please simplify that ?
Let's first check if fentry is supported:
- if yes, disable all kprobe progs, and do auto-attach
- if no, disable all fentry progs, and do auto-attach
You may need to disable some progs if symbols not exist
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.
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.
done
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.
Hi @chenhengqi
I did some simplification follow you suggestion, do you have some more advice?
And there is a failure in #455, but I cann't see the cause of that request?
Fallback to use kprobes if kernel lacks support for fentry/fexit. Signed-off-by: mickey_zhu <[email protected]>
Fallback to use kprobes if kernel lacks support for fentry/fexit.