]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
DEBUG: threads: merge successive idempotent lock operations in history
authorWilly Tarreau <w@1wt.eu>
Mon, 5 May 2025 16:24:40 +0000 (18:24 +0200)
committerWilly Tarreau <w@1wt.eu>
Mon, 5 May 2025 16:36:12 +0000 (18:36 +0200)
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

index d5c27cf9bfbf3122c653fcdeda41784f252f14f8..ad0ae1aa749991e4bbc6dc3e537822c7f025bfa9 100644 (file)
@@ -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; \
        })