Fix perf breaking#80
Conversation
lmb
left a comment
There was a problem hiding this comment.
I left a few comments. Thanks for finding out about ENODEV, that is very valuable. To be (imo bug) compatible with libbpf we'd have to ignore EINVAL as well, which is cumbersome and seems like the wrong thing to do. I'd really like to avoid it.
Can you use max_elems: 0 in your code? If no, can you explain why not? Maybe we can find a solution together.
| for i := 0; i < nCPU; i++ { | ||
| ring, err := newPerfEventRing(i, perCPUBuffer, opts.Watermark) | ||
| if err != nil { | ||
| if xerrors.Is(err, syscall.ENODEV) { |
There was a problem hiding this comment.
The code expects you to append to rings for each CPU, and right now you skip this because of the continue. I need to look into how to handle that.
| t.Fatal("perf event array: ", err) | ||
| } | ||
| fd, err = createPerfEvent(onlineCPU, 1) | ||
| if !xerrors.Is(err, syscall.ENODEV) { |
There was a problem hiding this comment.
If you check the Linux sources you'll find that ENODEV is only returned if:
- onlineCPU is less than nr_cpu_ids
- and the CPU is offline
I think you're violating the first condition, and getting EINVAL therefore.
|
According to @iAklis he can use max_elems: 0, hence I'll close this. |
I found the error is not EINVAL but ENODEV, this could fix my code. Could you help me review? @lmb
Alternative solution #78