[PATCH] D39175: [XRay][compiler-rt] Remove C++ STL from the buffer queue implementation
Martin Pelikán via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 22 23:17:14 PDT 2017
pelikan accepted this revision.
pelikan added inline comments.
This revision is now accepted and ready to land.
================
Comment at: compiler-rt/lib/xray/xray_buffer_queue.cc:68
+ if (*I == Buf.Buffer) {
+ Found = true;
+ break;
----------------
I'm slightly worried about what happens if we mark this "true", then a signal happens, the handler will be instrumented and this thing will be reentered and succeeds past the mutex-guarded thing at the bottom. We will resume afterwards and continue with incrementing First yet again, possibly wasting a buffer.
SpinMutex is clever enough to yield in case there is a different thread waiting (which isn't here) but not to be aware of signal processing. I think this should be addressed in a different diff though, using a lock-less mechanism. Shouldn't be too tricky for this simple case.
================
Comment at: compiler-rt/lib/xray/xray_buffer_queue.h:39
+ Buffer Buffer;
+ bool Used = false;
+ };
----------------
I was having a bit of a problem realizing whether this means "is being used" or "has been used". Maybe you can think of a small comment to improve this, maybe not.
https://reviews.llvm.org/D39175
More information about the llvm-commits
mailing list