RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Emeric Brun 2017-10-30 18:04:28 +01:00 committed by Willy Tarreau
parent a1ae7e81cd
commit 6e0128630b
2 changed files with 69 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
include/proto/freq_ctr.h
View File

@ -34,7 +34,7 @@
*/ */
static inline unsigned int update_freq_ctr(struct freq_ctr *ctr, unsigned int inc) 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 tot_inc;
unsigned int curr_sec; unsigned int curr_sec;
@ -42,23 +42,24 @@ static inline unsigned int update_freq_ctr(struct freq_ctr *ctr, unsigned int in
/* remove the bit, used for the lock */ /* remove the bit, used for the lock */
curr_sec = ctr->curr_sec & 0x7fffffff; 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; elapsed = (now.tv_sec & 0x7fffffff)- curr_sec;
if (unlikely(elapsed)) { if (unlikely(elapsed > 0)) {
ctr->prev_ctr = ctr->curr_ctr; ctr->prev_ctr = ctr->curr_ctr;
ctr->curr_ctr = 0; ctr->curr_ctr = 0;
if (likely(elapsed != 1)) { if (likely(elapsed != 1)) {
/* we missed more than one second */ /* we missed more than one second */
ctr->prev_ctr = 0; ctr->prev_ctr = 0;
} }
curr_sec = now.tv_sec;
} }
ctr->curr_ctr += inc; ctr->curr_ctr += inc;
tot_inc = ctr->curr_ctr; tot_inc = ctr->curr_ctr;
/* release the lock and update the time in case of rotate. */ /* 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; return tot_inc;
/* Note: later we may want to propagate the update to other counters */ /* Note: later we may want to propagate the update to other counters */
} }

113
src/freq_ctr.c
View File

@ -30,18 +30,21 @@
*/ */
unsigned int read_freq_ctr(struct freq_ctr *ctr) unsigned int read_freq_ctr(struct freq_ctr *ctr)
{ {
unsigned int curr, past; unsigned int curr, past, _curr, _past;
unsigned int age, curr_sec; unsigned int age, curr_sec, _curr_sec;
do { while(1) {
curr = ctr->curr_ctr; _curr = (volatile unsigned int)ctr->curr_ctr;
past = ctr->prev_ctr; _past = (volatile unsigned int)ctr->prev_ctr;
curr_sec = ctr->curr_sec; _curr_sec = (volatile unsigned int)ctr->curr_sec;
if (_curr_sec & 0x80000000)
} while (curr != ctr->curr_ctr continue;
|| past != ctr->prev_ctr curr = (volatile unsigned int)ctr->curr_ctr;
|| curr_sec != ctr->curr_sec past = (volatile unsigned int)ctr->prev_ctr;
|| (curr_sec & 0x80000000)); curr_sec = (volatile unsigned int)ctr->curr_sec;
if (_curr == curr && _past == past && _curr_sec == curr_sec)
break;
}
age = now.tv_sec - curr_sec; age = now.tv_sec - curr_sec;
if (unlikely(age > 1)) if (unlikely(age > 1))
@ -64,18 +67,21 @@ unsigned int read_freq_ctr(struct freq_ctr *ctr)
*/ */
unsigned int freq_ctr_remain(struct freq_ctr *ctr, unsigned int freq, unsigned int pend) unsigned int freq_ctr_remain(struct freq_ctr *ctr, unsigned int freq, unsigned int pend)
{ {
unsigned int curr, past; unsigned int curr, past, _curr, _past;
unsigned int age, curr_sec; unsigned int age, curr_sec, _curr_sec;
do { while(1) {
curr = ctr->curr_ctr; _curr = (volatile unsigned int)ctr->curr_ctr;
past = ctr->prev_ctr; _past = (volatile unsigned int)ctr->prev_ctr;
curr_sec = ctr->curr_sec; _curr_sec = (volatile unsigned int)ctr->curr_sec;
if (_curr_sec & 0x80000000)
} while (curr != ctr->curr_ctr continue;
|| past != ctr->prev_ctr curr = (volatile unsigned int)ctr->curr_ctr;
|| curr_sec != ctr->curr_sec past = (volatile unsigned int)ctr->prev_ctr;
|| (curr_sec & 0x80000000)); curr_sec = (volatile unsigned int)ctr->curr_sec;
if (_curr == curr && _past == past && _curr_sec == curr_sec)
break;
}
age = now.tv_sec - curr_sec; age = now.tv_sec - curr_sec;
if (unlikely(age > 1)) if (unlikely(age > 1))
@ -102,18 +108,21 @@ unsigned int freq_ctr_remain(struct freq_ctr *ctr, unsigned int freq, unsigned i
*/ */
unsigned int next_event_delay(struct freq_ctr *ctr, unsigned int freq, unsigned int pend) unsigned int next_event_delay(struct freq_ctr *ctr, unsigned int freq, unsigned int pend)
{ {
unsigned int curr, past; unsigned int curr, past, _curr, _past;
unsigned int wait, age, curr_sec; unsigned int wait, age, curr_sec, _curr_sec;
do { while(1) {
curr = ctr->curr_ctr; _curr = (volatile unsigned int)ctr->curr_ctr;
past = ctr->prev_ctr; _past = (volatile unsigned int)ctr->prev_ctr;
curr_sec = ctr->curr_sec; _curr_sec = (volatile unsigned int)ctr->curr_sec;
if (_curr_sec & 0x80000000)
} while (curr != ctr->curr_ctr continue;
|| past != ctr->prev_ctr curr = (volatile unsigned int)ctr->curr_ctr;
|| curr_sec != ctr->curr_sec past = (volatile unsigned int)ctr->prev_ctr;
|| (curr_sec & 0x80000000)); curr_sec = (volatile unsigned int)ctr->curr_sec;
if (_curr == curr && _past == past && _curr_sec == curr_sec)
break;
}
age = now.tv_sec - curr_sec; age = now.tv_sec - curr_sec;
if (unlikely(age > 1)) if (unlikely(age > 1))
@ -152,18 +161,21 @@ unsigned int next_event_delay(struct freq_ctr *ctr, unsigned int freq, unsigned
*/ */
unsigned int read_freq_ctr_period(struct freq_ctr_period *ctr, unsigned int period) unsigned int read_freq_ctr_period(struct freq_ctr_period *ctr, unsigned int period)
{ {
unsigned int curr, past; unsigned int _curr, _past, curr, past;
unsigned int remain, curr_tick; 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; curr = ctr->curr_ctr;
past = ctr->prev_ctr; past = ctr->prev_ctr;
curr_tick = ctr->curr_tick; curr_tick = ctr->curr_tick;
if (_curr == curr && _past == past && _curr_tick == curr_tick)
} while (curr != ctr->curr_ctr break;
|| past != ctr->prev_ctr };
|| curr_tick != ctr->curr_tick
|| (curr_tick & 0x1));
remain = curr_tick + period - now_ms; remain = curr_tick + period - now_ms;
if (unlikely((int)remain < 0)) { if (unlikely((int)remain < 0)) {
@ -190,18 +202,21 @@ unsigned int read_freq_ctr_period(struct freq_ctr_period *ctr, unsigned int peri
unsigned int freq_ctr_remain_period(struct freq_ctr_period *ctr, unsigned int period, unsigned int freq_ctr_remain_period(struct freq_ctr_period *ctr, unsigned int period,
unsigned int freq, unsigned int pend) unsigned int freq, unsigned int pend)
{ {
unsigned int curr, past; unsigned int _curr, _past, curr, past;
unsigned int remain, curr_tick; 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; curr = ctr->curr_ctr;
past = ctr->prev_ctr; past = ctr->prev_ctr;
curr_tick = ctr->curr_tick; curr_tick = ctr->curr_tick;
if (_curr == curr && _past == past && _curr_tick == curr_tick)
} while (curr != ctr->curr_ctr break;
|| past != ctr->prev_ctr };
|| curr_tick != ctr->curr_tick
|| (curr_tick & 0x1));
remain = curr_tick + period - now_ms; remain = curr_tick + period - now_ms;
if (likely((int)remain < 0)) { if (likely((int)remain < 0)) {