From: Willy Tarreau <w@1wt.eu>
Date: Tue, 2 Apr 2024 16:27:36 +0000 (+0200)
Subject: OPTIM: peers: avoid the locking dance around peer_send_teach_process_msgs()
X-Git-Tag: v3.0-dev7~32
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6a2f09de1ceaecb333b59058a16e4c5dcfe1c032;p=thirdparty%2Fhaproxy.git

OPTIM: peers: avoid the locking dance around peer_send_teach_process_msgs()

In peer_send_msg(), we take a lock before calling
peer_send_teach_process_msgs because of the check on the flags and update
indexes, and the function then drops it then takes it again just to resume
in the same situation, so that on return we can drop it again! Not only
this is absurd because it doubles the costs of taking the lock, it's also
totally inefficient because it takes a write lock while the only usage
that is done with it is to read the indexes! Let's drop the lock from
peer_send_teach_process_msgs() and move it explicitly in its only caller
around the condition, and turn it into a read lock only.
---

diff --git a/src/peers.c b/src/peers.c
index 272445929c..628b9358c6 100644
--- a/src/peers.c
+++ b/src/peers.c
@@ -1651,13 +1651,7 @@ static inline int peer_send_teachmsgs(struct appctx *appctx, struct peer *p,
 static inline int peer_send_teach_process_msgs(struct appctx *appctx, struct peer *p,
                                                struct shared_table *st)
 {
-	int ret;
-
-	HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
-	ret = peer_send_teachmsgs(appctx, p, peer_teach_process_stksess_lookup, st);
-	HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
-
-	return ret;
+	return peer_send_teachmsgs(appctx, p, peer_teach_process_stksess_lookup, st);
 }
 
 /*
@@ -2689,18 +2683,19 @@ static inline int peer_send_msgs(struct appctx *appctx,
 			}
 
 			if (!(peer->flags & PEER_F_TEACH_PROCESS)) {
-				HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
-				if (!(peer->flags & PEER_F_LEARN_ASSIGN) &&
-					(st->last_pushed != st->table->localupdate)) {
+				int must_send;
+
+				HA_RWLOCK_RDLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
+				must_send = !(peer->flags & PEER_F_LEARN_ASSIGN) && (st->last_pushed != st->table->localupdate);
+				HA_RWLOCK_RDUNLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
 
+				if (must_send) {
 					repl = peer_send_teach_process_msgs(appctx, peer, st);
 					if (repl <= 0) {
-						HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
 						peer->stop_local_table = peer->last_local_table;
 						return repl;
 					}
 				}
-				HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
 			}
 			else if (!(peer->flags & PEER_F_TEACH_FINISHED)) {
 				if (!(st->flags & SHTABLE_F_TEACH_STAGE1)) {