bpf: manually handle the perf read state memory allocation #1304
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tgraf
approved these changes
Aug 12, 2017
When the Cilium monitor is under heavy load we see the following crash panic: runtime error: cgo argument has Go pointer to Go pointer The crash is occurring near the call to `C.perf_event_read(...)`. At some point in time the state struct instance contains a Go pointer to Go pointer. According to the [Rules for passing pointers between Go and C][0] this is not allowed. >Go code may pass a Go pointer to C provided that the Go memory to which it points does not contain any Go pointers. I have not studied the Go source well enough to back up any suspicions that something is wrong in the runtime code, but looking at workarounds we could bypass the crasher by disabling the cgo runtime checks, but that doesn't really make sense considering that it requires a user to make a change. The other approach, which this patch does is instead of relying on `struct_read_state` to do the right thing, we fallback to malloc / free so we can reduce the chance of mismatch between Go and C memory. After this patch I am not able to reproduce the crasher. [0]: https://github.com/golang/proposal/blob/master/design/12416-cgo-pointers.md Closes: #409 (Crash in cilium monitor (ring buffer overflow?)) Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
|
Previous failure is unrelated, re triggered build |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the Cilium monitor is under heavy load we see the following crash
The crash is occurring near the second call to
C.perf_event_read(...).At some point in time the state struct instance contains a Go pointer to
Go pointer. According to the Rules for passing pointers between Go and
C this is not allowed.
I have not studied the Go source well enough to back up any suspicions
that something is wrong in the runtime code, but looking at solutions we
could bypass the crasher by disabling the cgo runtime checks, but that
doesn't really make sense considering that it requires a user to make a
change. The other approach, which this patch does is instead of relying
on
struct_read_stateto do the right thing, we fallback to malloc /free so we can reduce the chance of mismatch between Go and C memory.
After this patch I am not able to reproduce the crasher.
Closes: #409 (Crash in cilium monitor (ring buffer overflow?))
Signed-off-by: Alexander Alemayhu alexander@alemayhu.com