BUG/MAJOR: threads/freq_ctr: use a memory barrier to detect changes
commit 6e01286 (BUG/MAJOR: threads/freq_ctr: fix lock on freq counters)
attempted to fix the loop using volatile but that doesn't work depending
on the level of optimization, resulting in situations where the threads
could remain looping forever. Here we use memory barriers between reads
to enforce a strict ordering and the asm code produced does exactly what
the C code does and works perfectly, with a 3-digit measurement accuracy
observed during a test.
diff --git a/src/freq_ctr.c b/src/freq_ctr.c
index e50ce58..e6e0168 100644
--- a/src/freq_ctr.c
+++ b/src/freq_ctr.c
@@ -33,15 +33,21 @@
unsigned int curr, past, _curr, _past;
unsigned int age, curr_sec, _curr_sec;
- while(1) {
- _curr = (volatile unsigned int)ctr->curr_ctr;
- _past = (volatile unsigned int)ctr->prev_ctr;
- _curr_sec = (volatile unsigned int)ctr->curr_sec;
+ while (1) {
+ _curr = ctr->curr_ctr;
+ HA_BARRIER();
+ _past = ctr->prev_ctr;
+ HA_BARRIER();
+ _curr_sec = ctr->curr_sec;
+ HA_BARRIER();
if (_curr_sec & 0x80000000)
continue;
- curr = (volatile unsigned int)ctr->curr_ctr;
- past = (volatile unsigned int)ctr->prev_ctr;
- curr_sec = (volatile unsigned int)ctr->curr_sec;
+ curr = ctr->curr_ctr;
+ HA_BARRIER();
+ past = ctr->prev_ctr;
+ HA_BARRIER();
+ curr_sec = ctr->curr_sec;
+ HA_BARRIER();
if (_curr == curr && _past == past && _curr_sec == curr_sec)
break;
}
@@ -70,15 +76,21 @@
unsigned int curr, past, _curr, _past;
unsigned int age, curr_sec, _curr_sec;
- while(1) {
- _curr = (volatile unsigned int)ctr->curr_ctr;
- _past = (volatile unsigned int)ctr->prev_ctr;
- _curr_sec = (volatile unsigned int)ctr->curr_sec;
+ while (1) {
+ _curr = ctr->curr_ctr;
+ HA_BARRIER();
+ _past = ctr->prev_ctr;
+ HA_BARRIER();
+ _curr_sec = ctr->curr_sec;
+ HA_BARRIER();
if (_curr_sec & 0x80000000)
continue;
- curr = (volatile unsigned int)ctr->curr_ctr;
- past = (volatile unsigned int)ctr->prev_ctr;
- curr_sec = (volatile unsigned int)ctr->curr_sec;
+ curr = ctr->curr_ctr;
+ HA_BARRIER();
+ past = ctr->prev_ctr;
+ HA_BARRIER();
+ curr_sec = ctr->curr_sec;
+ HA_BARRIER();
if (_curr == curr && _past == past && _curr_sec == curr_sec)
break;
}
@@ -111,15 +123,21 @@
unsigned int curr, past, _curr, _past;
unsigned int wait, age, curr_sec, _curr_sec;
- while(1) {
- _curr = (volatile unsigned int)ctr->curr_ctr;
- _past = (volatile unsigned int)ctr->prev_ctr;
- _curr_sec = (volatile unsigned int)ctr->curr_sec;
+ while (1) {
+ _curr = ctr->curr_ctr;
+ HA_BARRIER();
+ _past = ctr->prev_ctr;
+ HA_BARRIER();
+ _curr_sec = ctr->curr_sec;
+ HA_BARRIER();
if (_curr_sec & 0x80000000)
continue;
- curr = (volatile unsigned int)ctr->curr_ctr;
- past = (volatile unsigned int)ctr->prev_ctr;
- curr_sec = (volatile unsigned int)ctr->curr_sec;
+ curr = ctr->curr_ctr;
+ HA_BARRIER();
+ past = ctr->prev_ctr;
+ HA_BARRIER();
+ curr_sec = ctr->curr_sec;
+ HA_BARRIER();
if (_curr == curr && _past == past && _curr_sec == curr_sec)
break;
}
@@ -164,15 +182,21 @@
unsigned int _curr, _past, curr, past;
unsigned int remain, _curr_tick, curr_tick;
- while(1) {
+ while (1) {
_curr = ctr->curr_ctr;
+ HA_BARRIER();
_past = ctr->prev_ctr;
+ HA_BARRIER();
_curr_tick = ctr->curr_tick;
+ HA_BARRIER();
if (_curr_tick & 0x1)
continue;
curr = ctr->curr_ctr;
+ HA_BARRIER();
past = ctr->prev_ctr;
+ HA_BARRIER();
curr_tick = ctr->curr_tick;
+ HA_BARRIER();
if (_curr == curr && _past == past && _curr_tick == curr_tick)
break;
};
@@ -205,15 +229,21 @@
unsigned int _curr, _past, curr, past;
unsigned int remain, _curr_tick, curr_tick;
- while(1) {
+ while (1) {
_curr = ctr->curr_ctr;
+ HA_BARRIER();
_past = ctr->prev_ctr;
+ HA_BARRIER();
_curr_tick = ctr->curr_tick;
+ HA_BARRIER();
if (_curr_tick & 0x1)
continue;
curr = ctr->curr_ctr;
+ HA_BARRIER();
past = ctr->prev_ctr;
+ HA_BARRIER();
curr_tick = ctr->curr_tick;
+ HA_BARRIER();
if (_curr == curr && _past == past && _curr_tick == curr_tick)
break;
};