From 80e4d720dddef6d354e8d6fa523b08c29f9fcc75 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sun, 26 Jan 2025 14:15:31 -0500 Subject: [PATCH v1 3/3] WIP: Base LWLock limits directly on MAX_BACKENDS 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 Discussion: https://postgr.es/m/CA+COZaBO_s3LfALq=b+HcBHFSOEGiApVjrRacCe4VP9m7CJsNQ@mail.gmail.com --- src/include/pg_config_manual.h | 4 +++- src/backend/storage/lmgr/lwlock.c | 26 ++++++++++++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 9874302946e..55db22eb095 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -51,8 +51,10 @@ * the ProcNumber as a 3-byte signed integer, b) INT_MAX/4 because some places * compute 4*MaxBackends without any overflow check. This is rechecked in the * relevant GUC check hooks and in RegisterBackgroundWorker(). + * + * Various places currently assume that MAX_BACKENDS + 1 is a power of two. */ -#define MAX_BACKENDS 0x3FFFF +#define MAX_BACKENDS 0x3FFFFU /* * When creating a product derived from PostgreSQL with changes that cause diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 328b1bab659..eea51ec601c 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -90,18 +90,28 @@ #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_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< (uint32) MAX_BACKENDS, + +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(MAX_BACKENDS < LW_VAL_EXCLUSIVE, "MAX_BACKENDS too big for lwlock.c"); /* -- 2.48.1.76.g4e746b1a31.dirty