BUG/MAJOR: threads/freq_ctr: fix lock on freq counters.
The wrong bit was set to keep the lock on freq counter update. And the read
functions were re-worked to use volatile.
Moreover, when a freq counter is updated, it is now rotated only if the current
counter is in the past (now.tv_sec > ctr->curr_sec). It is important with
threads because the current time (now) is thread-local. So, rounded to the
second, the time may vary by more or less 1 second. So a freq counter rotated by
one thread may be see 1 second in the future. In this case, it is updated but
not rotated.
diff --git a/include/proto/freq_ctr.h b/include/proto/freq_ctr.h
index c6ea5c2..c10ddf2 100644
--- a/include/proto/freq_ctr.h
+++ b/include/proto/freq_ctr.h
@@ -34,7 +34,7 @@
*/
static inline unsigned int update_freq_ctr(struct freq_ctr *ctr, unsigned int inc)
{
- unsigned int elapsed;
+ int elapsed;
unsigned int tot_inc;
unsigned int curr_sec;
@@ -42,23 +42,24 @@
/* remove the bit, used for the lock */
curr_sec = ctr->curr_sec & 0x7fffffff;
}
- while (!HA_ATOMIC_CAS(&ctr->curr_sec, &curr_sec, curr_sec | 0x8000000));
+ while (!HA_ATOMIC_CAS(&ctr->curr_sec, &curr_sec, curr_sec | 0x80000000));
elapsed = (now.tv_sec & 0x7fffffff)- curr_sec;
- if (unlikely(elapsed)) {
+ if (unlikely(elapsed > 0)) {
ctr->prev_ctr = ctr->curr_ctr;
ctr->curr_ctr = 0;
if (likely(elapsed != 1)) {
/* we missed more than one second */
ctr->prev_ctr = 0;
}
+ curr_sec = now.tv_sec;
}
ctr->curr_ctr += inc;
tot_inc = ctr->curr_ctr;
/* release the lock and update the time in case of rotate. */
- HA_ATOMIC_STORE(&ctr->curr_sec, now.tv_sec & 0x7fffffff);
+ HA_ATOMIC_STORE(&ctr->curr_sec, curr_sec & 0x7fffffff);
return tot_inc;
/* Note: later we may want to propagate the update to other counters */
}
diff --git a/src/freq_ctr.c b/src/freq_ctr.c
index 37fb1f3..e50ce58 100644
--- a/src/freq_ctr.c
+++ b/src/freq_ctr.c
@@ -30,18 +30,21 @@
*/
unsigned int read_freq_ctr(struct freq_ctr *ctr)
{
- unsigned int curr, past;
- unsigned int age, curr_sec;
+ unsigned int curr, past, _curr, _past;
+ unsigned int age, curr_sec, _curr_sec;
- do {
- curr = ctr->curr_ctr;
- past = ctr->prev_ctr;
- curr_sec = ctr->curr_sec;
-
- } while (curr != ctr->curr_ctr
- || past != ctr->prev_ctr
- || curr_sec != ctr->curr_sec
- || (curr_sec & 0x80000000));
+ while(1) {
+ _curr = (volatile unsigned int)ctr->curr_ctr;
+ _past = (volatile unsigned int)ctr->prev_ctr;
+ _curr_sec = (volatile unsigned int)ctr->curr_sec;
+ 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;
+ if (_curr == curr && _past == past && _curr_sec == curr_sec)
+ break;
+ }
age = now.tv_sec - curr_sec;
if (unlikely(age > 1))
@@ -64,18 +67,21 @@
*/
unsigned int freq_ctr_remain(struct freq_ctr *ctr, unsigned int freq, unsigned int pend)
{
- unsigned int curr, past;
- unsigned int age, curr_sec;
-
- do {
- curr = ctr->curr_ctr;
- past = ctr->prev_ctr;
- curr_sec = ctr->curr_sec;
+ unsigned int curr, past, _curr, _past;
+ unsigned int age, curr_sec, _curr_sec;
- } while (curr != ctr->curr_ctr
- || past != ctr->prev_ctr
- || curr_sec != ctr->curr_sec
- || (curr_sec & 0x80000000));
+ while(1) {
+ _curr = (volatile unsigned int)ctr->curr_ctr;
+ _past = (volatile unsigned int)ctr->prev_ctr;
+ _curr_sec = (volatile unsigned int)ctr->curr_sec;
+ 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;
+ if (_curr == curr && _past == past && _curr_sec == curr_sec)
+ break;
+ }
age = now.tv_sec - curr_sec;
if (unlikely(age > 1))
@@ -102,18 +108,21 @@
*/
unsigned int next_event_delay(struct freq_ctr *ctr, unsigned int freq, unsigned int pend)
{
- unsigned int curr, past;
- unsigned int wait, age, curr_sec;
-
- do {
- curr = ctr->curr_ctr;
- past = ctr->prev_ctr;
- curr_sec = ctr->curr_sec;
+ unsigned int curr, past, _curr, _past;
+ unsigned int wait, age, curr_sec, _curr_sec;
- } while (curr != ctr->curr_ctr
- || past != ctr->prev_ctr
- || curr_sec != ctr->curr_sec
- || (curr_sec & 0x80000000));
+ while(1) {
+ _curr = (volatile unsigned int)ctr->curr_ctr;
+ _past = (volatile unsigned int)ctr->prev_ctr;
+ _curr_sec = (volatile unsigned int)ctr->curr_sec;
+ 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;
+ if (_curr == curr && _past == past && _curr_sec == curr_sec)
+ break;
+ }
age = now.tv_sec - curr_sec;
if (unlikely(age > 1))
@@ -152,18 +161,21 @@
*/
unsigned int read_freq_ctr_period(struct freq_ctr_period *ctr, unsigned int period)
{
- unsigned int curr, past;
- unsigned int remain, curr_tick;
+ unsigned int _curr, _past, curr, past;
+ unsigned int remain, _curr_tick, curr_tick;
- do {
+ while(1) {
+ _curr = ctr->curr_ctr;
+ _past = ctr->prev_ctr;
+ _curr_tick = ctr->curr_tick;
+ if (_curr_tick & 0x1)
+ continue;
curr = ctr->curr_ctr;
past = ctr->prev_ctr;
curr_tick = ctr->curr_tick;
-
- } while (curr != ctr->curr_ctr
- || past != ctr->prev_ctr
- || curr_tick != ctr->curr_tick
- || (curr_tick & 0x1));
+ if (_curr == curr && _past == past && _curr_tick == curr_tick)
+ break;
+ };
remain = curr_tick + period - now_ms;
if (unlikely((int)remain < 0)) {
@@ -190,18 +202,21 @@
unsigned int freq_ctr_remain_period(struct freq_ctr_period *ctr, unsigned int period,
unsigned int freq, unsigned int pend)
{
- unsigned int curr, past;
- unsigned int remain, curr_tick;
+ unsigned int _curr, _past, curr, past;
+ unsigned int remain, _curr_tick, curr_tick;
- do {
+ while(1) {
+ _curr = ctr->curr_ctr;
+ _past = ctr->prev_ctr;
+ _curr_tick = ctr->curr_tick;
+ if (_curr_tick & 0x1)
+ continue;
curr = ctr->curr_ctr;
past = ctr->prev_ctr;
curr_tick = ctr->curr_tick;
-
- } while (curr != ctr->curr_ctr
- || past != ctr->prev_ctr
- || curr_tick != ctr->curr_tick
- || (curr_tick & 0x1));
+ if (_curr == curr && _past == past && _curr_tick == curr_tick)
+ break;
+ };
remain = curr_tick + period - now_ms;
if (likely((int)remain < 0)) {