Skip to content

Commit e78b7a5

Browse files
luciangfacebook-github-bot
authored andcommitted
hbt/bperf: replace compiler barrier with CPU fence std::atomic_thread_fence(std::memory_order_consume)
Summary: RDPMC is not a serializing instruction. The CPU may reorder instructions around it. https://www.felixcloutier.com/x86/rdpmc > The RDPMC instruction is not a serializing instruction; that is, it does not imply that all the events caused by the preceding instructions have been completed or that events caused by subsequent instructions have not begun. If an exact event count is desired, software must insert a serializing instruction (such as the CPUID instruction) before and/or after the RDPMC instruction. > Performing back-to-back fast reads are not guaranteed to be monotonic. To guarantee monotonicity on back-to-back reads, a serializing instruction must be placed between the two RDPMC instructions. https://www.felixcloutier.com/x86/rdtsc > The RDTSC instruction is not a serializing instruction. It does not necessarily wait until all previous instructions have been executed before reading the counter. Similarly, subsequent instructions may begin execution before the read operation is performed. The following items may guide software seeking to order executions of RDTSC: > If software requires RDTSC to be executed only after all previous instructions have executed and all previous loads are globally visible,1 it can execute LFENCE immediately before RDTSC. > If software requires RDTSC to be executed only after all previous instructions have executed and all previous loads and stores are globally visible, it can execute the sequence MFENCE;LFENCE immediately before RDTSC. > If software requires RDTSC to be executed prior to execution of any subsequent instruction (including any memory accesses), it can execute the sequence LFENCE immediately after RDTSC. Use an serializing instruction per Intel's recommandations. --- Example of differences: ``` #include <atomic> #define barrier() asm volatile("" ::: "memory") extern int x; int f() { barrier(); return x; } int g() { std::atomic_thread_fence(std::memory_order_consume); return x; } ``` ``` f(): push rbp mov rbp, rsp mov eax, DWORD PTR x[rip] pop rbp ret g(): push rbp mov rbp, rsp mov DWORD PTR [rbp-4], 1 lock or QWORD PTR [rsp], 0 nop mov eax, DWORD PTR x[rip] pop rbp ret ``` https://gcc.godbolt.org/z/vGhscPEfG This is also what we're doing in regular perf counters: https://www.internalfb.com/code/fbsource/[b6e9a1b34cb61db1222fc88ba0dc3d3fe60b496b]/fbcode/dyno/cpp/lib/DynoPerfCounter.cpp?lines=500-522 ``` do { seq = mem_->lock; std::atomic_thread_fence(std::memory_order_consume); enabled = mem_->time_enabled; running = mem_->time_running; const uint32_t idx = mem_->index; // May be zero when inactive (eg. multiplexing). if (mem_->cap_user_time) { const uint64_t cyc = __rdtsc(); const uint16_t timeShift = mem_->time_shift; const uint64_t quot = (cyc >> timeShift); const uint64_t rem = cyc & (((uint64_t)1 << timeShift) - 1); const uint64_t delta = mem_->time_offset + (quot * mem_->time_mult) + ((rem * mem_->time_mult) >> timeShift); enabled += delta; running += idx != 0 ? delta : 0; } count = mem_->offset + (idx ? __rdpmc(idx - 1) : 0); // Sign extend. count <<= (64 - mem_->pmc_width); count = static_cast<int64_t>(count) >> (64 - mem_->pmc_width); std::atomic_thread_fence(std::memory_order_consume); } while (seq != mem_->lock); ``` Differential Revision: D86381530
1 parent 5ca37d1 commit e78b7a5

1 file changed

Lines changed: 3 additions & 4 deletions

File tree

hbt/src/perf_event/BPerfPerThreadReader.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "hbt/src/perf_event/BPerfPerThreadReader.h"
77
#include <bpf/bpf.h>
88
#include <time.h>
9+
#include <atomic>
910
#include "hbt/src/perf_event/BPerfEventsGroup.h"
1011

1112
namespace facebook::hbt::perf_event {
@@ -186,8 +187,6 @@ BPerfPerThreadReader::~BPerfPerThreadReader() {
186187
disable();
187188
}
188189

189-
#define barrier() asm volatile("" ::: "memory")
190-
191190
int BPerfPerThreadReader::read(struct BPerfThreadData* data) {
192191
struct bperf_clock_param *ptr = &data_->tsc_param, p;
193192
struct bperf_thread_data raw_thread_data;
@@ -203,7 +202,7 @@ int BPerfPerThreadReader::read(struct BPerfThreadData* data) {
203202

204203
do {
205204
lock = data_->lock;
206-
barrier();
205+
std::atomic_thread_fence(std::memory_order_consume);
207206
tsc = rdtsc();
208207
p = *ptr;
209208
raw_thread_data = *data_;
@@ -216,7 +215,7 @@ int BPerfPerThreadReader::read(struct BPerfThreadData* data) {
216215
pmc_val[i] = 0;
217216
}
218217
}
219-
barrier();
218+
std::atomic_thread_fence(std::memory_order_consume);
220219
} while (lock != data_->lock);
221220

222221
data->monoTime = (((__uint128_t)tsc * p.multi) >> p.shift) + p.offset +

0 commit comments

Comments
 (0)