From 9f5cb435b650b19eba37e438bfde6a8b98a64fb1 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 11 Oct 2022 18:17:58 +0000 Subject: [PATCH] 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. --- include/haproxy/stick_table.h | 2 +- src/stick_table.c | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/include/haproxy/stick_table.h b/include/haproxy/stick_table.h index a495e7738e..e1b6d0633f 100644 --- 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 1a24ed1cd4..141bf9c736 100644 --- 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 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 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 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 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 is NULL. * Note that we still need to take the read lock because a number of other places