MEDIUM: threads/freq_ctr: Make the frequency counters thread-safe

When a frequency counter must be updated, we use the curr_sec/curr_tick fields
as a lock, by setting the MSB to 1 in a compare-and-swap to lock and by reseting
it to unlock. And when we need to read it, we loop until the counter is
unlocked. This way, the frequency counters are thread-safe without any external
lock. It is important to avoid increasing the size of many structures (global,
proxy, server, stick_table).

diff --git a/include/proto/freq_ctr.h b/include/proto/freq_ctr.h
index 0d0411041a92969c5962350d0d1c07e5a85e62b2..c6ea5c24c79ca52b5c73c41bbbe3bba7cbb448fe 100644 (file)
--- a/include/proto/freq_ctr.h
+++ b/include/proto/freq_ctr.h
@@ -24,22 +24,9 @@
 
 #include <common/config.h>
 #include <common/time.h>
+#include <common/hathreads.h>
 #include <types/freq_ctr.h>
 
-/* Rotate a frequency counter when current period is over. Must not be called
- * during a valid period. It is important that it correctly initializes a null
- * area.
- */
-static inline void rotate_freq_ctr(struct freq_ctr *ctr)
-{
-       ctr->prev_ctr = ctr->curr_ctr;
-       if (likely(now.tv_sec - ctr->curr_sec != 1)) {
-               /* we missed more than one second */
-               ctr->prev_ctr = 0;
-       }
-       ctr->curr_sec = now.tv_sec;
-       ctr->curr_ctr = 0; /* leave it at the end to help gcc optimize it away */
-}
 
 /* Update a frequency counter by <inc> incremental units. It is automatically
  * rotated if the period is over. It is important that it correctly initializes
@@ -47,32 +34,33 @@ static inline void rotate_freq_ctr(struct freq_ctr *ctr)
  */
 static inline unsigned int update_freq_ctr(struct freq_ctr *ctr, unsigned int inc)
 {
-       if (likely(ctr->curr_sec == now.tv_sec)) {
-               ctr->curr_ctr += inc;
-               return ctr->curr_ctr;
+       unsigned int elapsed;
+       unsigned int tot_inc;
+       unsigned int curr_sec;
+
+       do {
+               /* remove the bit, used for the lock */
+               curr_sec = ctr->curr_sec & 0x7fffffff;
        }
-       rotate_freq_ctr(ctr);
-       ctr->curr_ctr = inc;
-       return ctr->curr_ctr;
-       /* Note: later we may want to propagate the update to other counters */
-}
+       while (!HA_ATOMIC_CAS(&ctr->curr_sec, &curr_sec, curr_sec | 0x8000000));
 
-/* Rotate a frequency counter when current period is over. Must not be called
- * during a valid period. It is important that it correctly initializes a null
- * area. This one works on frequency counters which have a period different
- * from one second.
- */
-static inline void rotate_freq_ctr_period(struct freq_ctr_period *ctr,
-                                         unsigned int period)
-{
-       ctr->prev_ctr = ctr->curr_ctr;
-       ctr->curr_tick += period;
-       if (likely(now_ms - ctr->curr_tick >= period)) {
-               /* we missed at least two periods */
-               ctr->prev_ctr = 0;
-               ctr->curr_tick = now_ms;
+       elapsed = (now.tv_sec & 0x7fffffff)- curr_sec;
+       if (unlikely(elapsed)) {
+               ctr->prev_ctr = ctr->curr_ctr;
+               ctr->curr_ctr = 0;
+               if (likely(elapsed != 1)) {
+                       /* we missed more than one second */
+                       ctr->prev_ctr = 0;
+               }
        }
-       ctr->curr_ctr = 0; /* leave it at the end to help gcc optimize it away */
+
+       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);
+       return tot_inc;
+       /* Note: later we may want to propagate the update to other counters */
 }
 
 /* Update a frequency counter by <inc> incremental units. It is automatically
@@ -83,13 +71,31 @@ static inline void rotate_freq_ctr_period(struct freq_ctr_period *ctr,
 static inline unsigned int update_freq_ctr_period(struct freq_ctr_period *ctr,
                                                  unsigned int period, unsigned int inc)
 {
-       if (likely(now_ms - ctr->curr_tick < period)) {
-               ctr->curr_ctr += inc;
-               return ctr->curr_ctr;
+       unsigned int tot_inc;
+       unsigned int curr_tick;
+
+       do {
+               /* remove the bit, used for the lock */
+               curr_tick = (ctr->curr_tick >> 1) << 1;
+       }
+       while (!HA_ATOMIC_CAS(&ctr->curr_tick, &curr_tick, curr_tick | 0x1));
+
+       if (now_ms - curr_tick >= period) {
+               ctr->prev_ctr = ctr->curr_ctr;
+               ctr->curr_ctr = 0;
+               curr_tick += period;
+               if (likely(now_ms - curr_tick >= period)) {
+                       /* we missed at least two periods */
+                       ctr->prev_ctr = 0;
+                       curr_tick = now_ms;
+               }
        }
-       rotate_freq_ctr_period(ctr, period);
-       ctr->curr_ctr = inc;
-       return ctr->curr_ctr;
+
+       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_tick, (curr_tick >> 1) << 1);
+       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 6dec970024973d486d7b3e708e094fe263cc920b..37fb1f385e2ec7d2f38c0fff16a881450ffdf7b1 100644 (file)
--- a/src/freq_ctr.c
+++ b/src/freq_ctr.c
@@ -31,17 +31,25 @@
 unsigned int read_freq_ctr(struct freq_ctr *ctr)
 {
        unsigned int curr, past;
-       unsigned int age;
+       unsigned int age, curr_sec;
 
-       age = now.tv_sec - ctr->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));
+
+       age = now.tv_sec - curr_sec;
        if (unlikely(age > 1))
                return 0;
 
-       curr = 0;               
-       past = ctr->curr_ctr;
-       if (likely(!age)) {
-               curr = past;
-               past = ctr->prev_ctr;
+       if (unlikely(age)) {
+               past = curr;
+               curr = 0;
        }
 
        if (past <= 1 && !curr)
@@ -57,16 +65,25 @@ 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 curr, past;
-       unsigned int age;
+       unsigned int age, curr_sec;
+
+       do {
+               curr = ctr->curr_ctr;
+               past = ctr->prev_ctr;
+               curr_sec = ctr->curr_sec;
 
-       curr = 0;               
-       age = now.tv_sec - ctr->curr_sec;
+       } while (curr != ctr->curr_ctr
+                || past != ctr->prev_ctr
+                || curr_sec !=  ctr->curr_sec
+                || (curr_sec & 0x80000000));
 
-       if (likely(age <= 1)) {
-               past = ctr->curr_ctr;
-               if (likely(!age)) {
-                       curr = past;
-                       past = ctr->prev_ctr;
+       age = now.tv_sec - curr_sec;
+       if (unlikely(age > 1))
+               curr = 0;
+       else {
+               if (unlikely(age == 1)) {
+                       past = curr;
+                       curr = 0;
                }
                curr += mul32hi(past, ms_left_scaled);
        }
@@ -86,17 +103,25 @@ 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 curr, past;
-       unsigned int wait, age;
+       unsigned int wait, age, curr_sec;
+
+       do {
+               curr = ctr->curr_ctr;
+               past = ctr->prev_ctr;
+               curr_sec = ctr->curr_sec;
 
-       past = 0;
-       curr = 0;               
-       age = now.tv_sec - ctr->curr_sec;
+       } while (curr != ctr->curr_ctr
+                || past != ctr->prev_ctr
+                || curr_sec !=  ctr->curr_sec
+                || (curr_sec & 0x80000000));
 
-       if (likely(age <= 1)) {
-               past = ctr->curr_ctr;
-               if (likely(!age)) {
-                       curr = past;
-                       past = ctr->prev_ctr;
+       age = now.tv_sec - curr_sec;
+       if (unlikely(age > 1))
+               curr = 0;
+       else {
+               if (unlikely(age == 1)) {
+                       past = curr;
+                       curr = 0;
                }
                curr += mul32hi(past, ms_left_scaled);
        }
@@ -128,12 +153,19 @@ 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 curr, past;
-       unsigned int remain;
+       unsigned int remain, curr_tick;
 
-       curr = ctr->curr_ctr;
-       past = ctr->prev_ctr;
+       do {
+               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));
 
-       remain = ctr->curr_tick + period - now_ms;
+       remain = curr_tick + period - now_ms;
        if (unlikely((int)remain < 0)) {
                /* We're past the first period, check if we can still report a
                 * part of last period or if we're too far away.
@@ -159,12 +191,19 @@ unsigned int freq_ctr_remain_period(struct freq_ctr_period *ctr, unsigned int pe
                                    unsigned int freq, unsigned int pend)
 {
        unsigned int curr, past;
-       unsigned int remain;
+       unsigned int remain, curr_tick;
+
+       do {
+               curr = ctr->curr_ctr;
+               past = ctr->prev_ctr;
+               curr_tick = ctr->curr_tick;
 
-       curr = ctr->curr_ctr;
-       past = ctr->prev_ctr;
+       } while (curr != ctr->curr_ctr
+                ||  past != ctr->prev_ctr
+                || curr_tick !=  ctr->curr_tick
+                || (curr_tick & 0x1));
 
-       remain = ctr->curr_tick + period - now_ms;
+       remain = curr_tick + period - now_ms;
        if (likely((int)remain < 0)) {
                /* We're past the first period, check if we can still report a
                 * part of last period or if we're too far away.