MINOR: stick-table: move the write lock inside stktable_touch_with_exp()

Taking the write lock prior to entering that function is a problem
because this function is full of conditions that most of the time can
lead to eliminating the lock.

This commit first moves the write lock inside the function and passes
the extra argument required to implement stktable_touch_remote() and
stktable_touch_local(). It also renames the function to remove the
underscores since there's no other variant and it's exported under
this name (probably an old rename that was not propagated). The code
was stressed under 48 threads using 3 trackers on the same table. It
already shows a tiny 3% improvement from 187k to 193k rps.

diff --git a/include/haproxy/stick_table.h b/include/haproxy/stick_table.h
index a495e7738e55e96891ef82fc40576937da890aa3..e1b6d0633f76a92ae540b3f764207cc18266872c 100644 (file)
--- a/include/haproxy/stick_table.h
+++ b/include/haproxy/stick_table.h
@@ -50,7 +50,7 @@ int parse_stick_table(const char *file, int linenum, char **args,
                       struct stktable *t, char *id, char *nid, struct peers *peers);
 struct stksess *stktable_get_entry(struct stktable *table, struct stktable_key *key);
 struct stksess *stktable_set_entry(struct stktable *table, struct stksess *nts);
-void stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int decrefcount, int expire);
+void stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int decrefcount, int expire, int decrefcnt);
 void stktable_touch_remote(struct stktable *t, struct stksess *ts, int decrefcnt);
 void stktable_touch_local(struct stktable *t, struct stksess *ts, int decrefccount);
 struct stksess *stktable_lookup(struct stktable *t, struct stksess *ts);

diff --git a/src/stick_table.c b/src/stick_table.c
index 1a24ed1cd4abf4882e162154e2a4cddb3624bc2a..141bf9c73697eb6860f7d5d53e71248928471fdb 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -385,11 +385,16 @@ struct stksess *stktable_lookup(struct stktable *t, struct stksess *ts)
 /* Update the expiration timer for <ts> but do not touch its expiration node.
  * The table's expiration timer is updated if set.
  * The node will be also inserted into the update tree if needed, at a position
- * depending if the update is a local or coming from a remote node
+ * depending if the update is a local or coming from a remote node.
+ * If <decrefcnt> is set, the ts entry's ref_cnt will be decremented. The table's
+ * write lock may be taken.
  */
-void __stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int local, int expire)
+void stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int local, int expire, int decrefcnt)
 {
        struct eb32_node * eb;
+
+       HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->lock);
+
        ts->expire = expire;
        if (t->expire) {
                t->exp_task->expire = t->exp_next = tick_first(ts->expire, t->exp_next);
@@ -427,6 +432,10 @@ void __stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int local
                        }
                }
        }
+
+       if (decrefcnt)
+               ts->ref_cnt--;
+       HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->lock);
 }
 
 /* Update the expiration timer for <ts> but do not touch its expiration node.
@@ -437,11 +446,7 @@ void __stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int local
  */
 void stktable_touch_remote(struct stktable *t, struct stksess *ts, int decrefcnt)
 {
-       HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->lock);
-       __stktable_touch_with_exp(t, ts, 0, ts->expire);
-       if (decrefcnt)
-               ts->ref_cnt--;
-       HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->lock);
+       stktable_touch_with_exp(t, ts, 0, ts->expire, decrefcnt);
 }
 
 /* Update the expiration timer for <ts> but do not touch its expiration node.
@@ -454,11 +459,7 @@ void stktable_touch_local(struct stktable *t, struct stksess *ts, int decrefcnt)
 {
        int expire = tick_add(now_ms, MS_TO_TICKS(t->expire));
 
-       HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->lock);
-       __stktable_touch_with_exp(t, ts, 1, expire);
-       if (decrefcnt)
-               ts->ref_cnt--;
-       HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->lock);
+       stktable_touch_with_exp(t, ts, 1, expire, decrefcnt);
 }
 /* Just decrease the ref_cnt of the current session. Does nothing if <ts> is NULL.
  * Note that we still need to take the read lock because a number of other places