From feaac66b5e2dda4ffc0ab97ac3f3ecacf8aa86c2 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Mon, 5 May 2025 18:24:40 +0200
Subject: [PATCH] DEBUG: threads: merge successive idempotent lock operations
 in history

In order to make the lock history a bit more useful, let's try to merge
adjacent lock/unlock sequences that don't change anything for other
threads. For this we can replace the last unlock with the new operation
on the same label, and even just not store it if it was the same as the
one before the unlock, since in the end it's the same as if the unlock
had not been done.

Now loops that used to be filled with "R:LISTENER U:LISTENER" show more
useful info such as:

  S:IDLE_CONNS U:IDLE_CONNS S:PEER U:PEER S:IDLE_CONNS U:IDLE_CONNS R:LISTENER U:LISTENER
  U:STK_TABLE W:STK_SESS U:STK_SESS R:STK_TABLE U:STK_TABLE W:STK_SESS U:STK_SESS R:STK_TABLE
  R:STK_TABLE U:STK_TABLE W:STK_SESS U:STK_SESS W:STK_TABLE_UPDT U:STK_TABLE_UPDT S:PEER

It's worth noting that it can sometimes induce confusion when recursive
locks of the same label are used (a few exist on peers or stick-tables),
as in such a case the two operations would be needed. However these ones
are already undebuggable, so instead they will just have to be renamed
to make sure they use a distinct label.
---
 include/haproxy/thread.h | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/include/haproxy/thread.h b/include/haproxy/thread.h
index d5c27cf9b..ad0ae1aa7 100644
--- a/include/haproxy/thread.h
+++ b/include/haproxy/thread.h
@@ -336,15 +336,38 @@ static inline unsigned long thread_isolated()
 
 #else
 
+/* principle: each lock operation takes 8 bits, 6 of which (the highest) are
+ * the lock label, and two of which (the lowest) are the operation (_LK_*).
+ * In order to preserve as much usable history as possible, we try to merge
+ * repetitions:
+ *  - if a lock is taken just after it was released, the release is erased
+ *    from history and replace with the new operation ;
+ *  - if, when replacing an unlock, the new operation is the same as the
+ *    one before the unlock, then the new one is not added.
+ * This means that sequences like "R:foo U:foo R:foo" just become "R:foo",
+ * but that those like "R:foo U:foo W:foo U:foo" become "R:foo W:foo U:foo".
+ */
+#define _lock_wait_common(_LK_, lbl) do {				\
+		ulong _lck = ((lbl + 1) << 2) + _LK_;			\
+		if ((uint8_t)th_ctx->lock_history == (uint8_t)(((lbl + 1) << 2) + _LK_UN)) { \
+			/* re-lock of just unlocked, try to compact and possibly merge with n-2 */ \
+			th_ctx->lock_history >>= 8;			\
+			if ((uint8_t)th_ctx->lock_history != (uint8_t)_lck) \
+				th_ctx->lock_history = (th_ctx->lock_history << 8) + _lck; \
+		}							\
+		else							\
+			th_ctx->lock_history = (th_ctx->lock_history << 8) + _lck; \
+	} while (0)
+
 #define _lock_wait(_LK_, lbl, expr) do {				\
 		(void)(expr);						\
 		if (lbl != OTHER_LOCK)					\
-			th_ctx->lock_history = (th_ctx->lock_history << 8) + ((lbl + 1) << 2) + _LK_; \
+			_lock_wait_common(_LK_, lbl);			\
 	} while (0)
 #define _lock_cond(_LK_, lbl, expr) ({					\
 		typeof(expr) _expr = (expr);				\
 		if (lbl != OTHER_LOCK && !_expr)			\
-			th_ctx->lock_history = (th_ctx->lock_history << 8) + ((lbl + 1) << 2) + _LK_; \
+			_lock_wait_common(_LK_, lbl);			\
 		_expr; \
 	})