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 e50ce589e627a2436734e539355d42eedd5b23fa..e6e0168930d5123f2aed4ab606582eafbfd952d9 100644 (file)
--- a/src/freq_ctr.c
+++ b/src/freq_ctr.c
@@ -33,15 +33,21 @@ unsigned int read_freq_ctr(struct freq_ctr *ctr)
        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 freq_ctr_remain(struct freq_ctr *ctr, unsigned int freq, unsigned i
        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 next_event_delay(struct freq_ctr *ctr, unsigned int freq, unsigned
        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 read_freq_ctr_period(struct freq_ctr_period *ctr, unsigned int peri
        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 freq_ctr_remain_period(struct freq_ctr_period *ctr, unsigned int pe
        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;
        };