Base LWLock limits directly on MAX_BACKENDS
authorAndres Freund <[email protected]>
Mon, 24 Feb 2025 10:39:17 +0000 (05:39 -0500)
committerAndres Freund <[email protected]>
Mon, 24 Feb 2025 11:23:41 +0000 (06:23 -0500)
Jacob reported that comments for LW_SHARED_MASK referenced a MAX_BACKENDS
limit of 2^23-1, but that MAX_BACKENDS is actually limited to 2^18-1. The
limit was lowered in 48354581a49c, but the comment in lwlock.c wasn't updated.

Instead of just fixing the comment, it seems better to directly base the
lwlock defines on MAX_BACKENDS and add static assertions to ensure that there
is enough space. That way there's no comment that can go out of sync in the
future.

As part of that change I noticed that for some reason the high bit wasn't used
for flags, which seems somewhat odd. Redefine the flag values to start at the
highest bit.

Reported-by: Jacob Brazeal <[email protected]>
Reviewed-by: Jacob Brazeal <[email protected]>
Discussion: https://postgr.es/m/CA+COZaBO_s3LfALq=b+HcBHFSOEGiApVjrRacCe4VP9m7CJsNQ@mail.gmail.com

src/backend/storage/lmgr/lwlock.c

index 79cc48b23c486e5806860f87de5c039bbb66548f..8adf27302770975d49b85115d1922ff811a79fac 100644 (file)
 #endif
 
 
-#define LW_FLAG_HAS_WAITERS            ((uint32) 1 << 30)
-#define LW_FLAG_RELEASE_OK         ((uint32) 1 << 29)
-#define LW_FLAG_LOCKED             ((uint32) 1 << 28)
-
-#define LW_VAL_EXCLUSIVE           ((uint32) 1 << 24)
+#define LW_FLAG_HAS_WAITERS            ((uint32) 1 << 31)
+#define LW_FLAG_RELEASE_OK         ((uint32) 1 << 30)
+#define LW_FLAG_LOCKED             ((uint32) 1 << 29)
+#define LW_FLAG_BITS               3
+#define LW_FLAG_MASK               (((1<<LW_FLAG_BITS)-1)<<(32-LW_FLAG_BITS))
+
+/* assumes MAX_BACKENDS is a (power of 2) - 1, checked below */
+#define LW_VAL_EXCLUSIVE           (MAX_BACKENDS + 1)
 #define LW_VAL_SHARED              1
 
-#define LW_LOCK_MASK               ((uint32) ((1 << 25)-1))
-/* Must be greater than MAX_BACKENDS - which is 2^23-1, so we're fine. */
-#define LW_SHARED_MASK             ((uint32) ((1 << 24)-1))
+/* already (power of 2)-1, i.e. suitable for a mask */
+#define LW_SHARED_MASK             MAX_BACKENDS
+#define LW_LOCK_MASK               (MAX_BACKENDS | LW_VAL_EXCLUSIVE)
+
+
+StaticAssertDecl(((MAX_BACKENDS + 1) & MAX_BACKENDS) == 0,
+                "MAX_BACKENDS + 1 needs to be a power of 2");
+
+StaticAssertDecl((MAX_BACKENDS & LW_FLAG_MASK) == 0,
+                "MAX_BACKENDS and LW_FLAG_MASK overlap");
 
-StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
-                "MAX_BACKENDS too big for lwlock.c");
+StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0,
+                "LW_VAL_EXCLUSIVE and LW_FLAG_MASK overlap");
 
 /*
  * There are three sorts of LWLock "tranches":