Hi Chao,
On Thu, Nov 6, 2025 at 8:53 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> I just reviewed the patch and got a couple of comments:
>
> 1 - 0001
> ```
> + * Create semaphores. This is done here because of a now-non-existent
> + * dependency between spinlocks, which were required for shared memory
> + * allocation, and semaphores, which were allocated in the shared memory
> + * on some platforms. Ideally it should be done in
> + * CreateOrAttachShmemStructs() where we allocate other shared memory
> + * structures.
> */
> PGReserveSemaphores(numSemas);
> ```
>
> Looking into implementations of PGReserveSemaphores(), only win32 implementation use local memory, so can we be more
specific,like changing “some platforms” to “non-windows platforms”?
>
Thanks for the comments. The patches were committed already. Please
feel free to propose changes again based on my response here.
Being specific has the advantage that people know where to look for,
instead of everywhere. But in case we grow such platforms tomorrow,
that advantage is null and could be even misleading. The risks of the
latter outweigh the advantages, I believe.
> 2 - 0002
> ```
> diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
> index e9ef0fbfe32..1893cfeba64 100644
> --- a/src/backend/storage/lmgr/proc.c
> +++ b/src/backend/storage/lmgr/proc.c
> @@ -144,6 +144,7 @@ ProcGlobalShmemSize(void)
> size = add_size(size, sizeof(PROC_HDR));
> size = add_size(size, sizeof(slock_t));
>
> + size = add_size(size, PGSemaphoreShmemSize(ProcGlobalSemas()));
> size = add_size(size, PGProcShmemSize());
> size = add_size(size, FastPathLockShmemSize());
> ```
>
> As PGReserveSemaphores() on win32 doesn’t use shared memory, do we want to skip the new “add_size” for win32?
ProcGlobalShmemSize() defined in win32_sema.c returns 0. So the effect is same.
--
Best Wishes,
Ashutosh Bapat
