-
Notifications
You must be signed in to change notification settings - Fork 109
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
Add bpftrace to pbench-agent-tools #1108
Add bpftrace to pbench-agent-tools #1108
Conversation
|
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.
Great start. This is really cool.
Can you be sure to follow the model of the systemtap tool as well?
@atheurer 2. The syscount.bt doesn't need compiling into the kernel, bpftrace using libbpf dynamically loads it into the kernel. |
Yes, I understand it can do this, but my concern is that libbpf doing this at tool-start time might require a lot of [cpu] resources, or might require a little bit of time to do this. As a comparison, for systemtap, this is why we "pre" compile the script when we register the tool, instead of just doing it automatically at start-tools. Maybe this is not possible with BPF, but I guess we can see how it behaves with the way you have it now, and then see later if it becomes an issue. |
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.
Almost, just a few things to clean up.
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.
Almost there, looking good.
Can you also rebase and squash this commits? |
Signed-off-by: T K Sourab <sourabhtk37@gmail.com> : Base bpftrace tool like systemtap tool : add command to tool_cmd_file Signed-off-by: T K Sourab <sourabhtk37@gmail.com> Clean up bpftrace tool-script Signed-off-by: T K Sourab <sourabhtk37@gmail.com> Added checks for finding location of bpftrace binary Signed-off-by: T K Sourab <sourabhtk37@gmail.com> Cleanup: - Add check_install_rpm function - Required argument checking (--script) - Use default rpm install location Signed-off-by: T K Sourab <sourabhtk37@gmail.com> Add comments to example usage Signed-off-by: T K Sourab <sourabhtk37@gmail.com> Cleanup: - check non-empty $script - check for directory creation success - typos Signed-off-by: T K Sourab <sourabhtk37@gmail.com> Some checking to add to tool script
fbee462
to
5d1fc6c
Compare
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.
Looks good, tried it out with a mock of /usr/bin/bpftrace
and it seemed to work fine.
Todo
/usr/bin/
or/usr/local/bin
This is for tracking purpose. I used the existing jmap tool to base this. Need your inputs/reviews on implementing the post-processing and other features needed for pbench.
Here's the sample stdout from a run, which uses a basic bpftrace program, syscount.bt:
Signed-off-by: T K Sourab sourabhtk37@gmail.com