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

example opensnoop doesn't run and I've fixed it #48

Closed
wants to merge 2 commits into from

Conversation

lilydjwg
Copy link

First try with the exmple program, I get todo: oh no. It seems that bpf_get_next_key for the last CPU will fail so I fixed it.

Second try, I get SIGSEGV at the callback. The pointer passed to bpf_open_perf_buffer refers to an object on stack.

rust-bcc/src/perf.rs

Lines 117 to 122 in f06554a

let mut callback = PerfCallback { raw_cb };
let reader = unsafe {
bpf_open_perf_buffer(
Some(raw_callback),
None,
&mut callback as *mut _ as MutPointer,

I didn't figure out what PerfCallback does so I replaced it altogether with plain function pointers and it seems to work.

I didn't update other examples for now. If the changes seem good, I can update others.

Copy link
Collaborator

@brayniac brayniac left a comment

Choose a reason for hiding this comment

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

Hi @lilydjwg - thanks for your PR. I'm still taking a look, but I believe there are a couple issues with the changes in this PR.

let mut cur = Cursor::new(leaf);

let cpus = cpuonline::get()?;
for cpu in cpus.iter() {
for (i, cpu) in cpus.iter().enumerate() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this will break the case where lower number cores are offlined.

@brayniac
Copy link
Collaborator

@lilydjwg - I've tested the opensnoop example and it works as it is in master. I'd prefer to work this through an issue to figure out what's going on before making code changes to try to address what you're seeing.

I'm not comfortable with the changes in this PR as they don't appear to be addressing a confirmed issue. I'm closing this PR without merging.

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.

2 participants