Little cleanup: Move ProcStructLock to the ProcGlobal struct

On Feb 11, 2026, at 01:39, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

For some reason, the ProcStructLock spinlock is allocated in a shared memory area of its own:

/* Create ProcStructLock spinlock, too */
ProcStructLock = (slock_t *) ShmemInitStruct("ProcStructLock spinlock",
sizeof(slock_t),
&found);
SpinLockInit(ProcStructLock);

I believe that's just for historical reasons. A long long time ago, spinlocks had to be allocated separately rather than embedded in other structs.

The spinlock protects the freeProcs list and some other fields in ProcGlobal, so let's put it together with those fields. It's good for cache locality to have it next to the thing it protects, and just makes more sense anyway.

Any objections?

- Heikki<0001-Move-ProcStructLock-to-the-ProcGlobal-struct.patch>

Hi Heikki,

I took a quick review. You moved ProcStructLock into PROC_HDR as freeProcsLock, and deleted:
```
ProcStructLock = ShmemInitStruct(...);
SpinLockInit(ProcStructLock);
```

But I don’t see a replacement like SpinLockInit(&ProcGlobal->freeProcsLock);

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Wed, Feb 11, 2026 at 8:46 AM Chao Li <li.evan.chao@gmail.com> wrote:

On Feb 11, 2026, at 01:39, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

For some reason, the ProcStructLock spinlock is allocated in a shared memory area of its own:

/* Create ProcStructLock spinlock, too */
ProcStructLock = (slock_t *) ShmemInitStruct("ProcStructLock spinlock",
sizeof(slock_t),
&found);
SpinLockInit(ProcStructLock);

I believe that's just for historical reasons. A long long time ago, spinlocks had to be allocated separately rather than embedded in other structs.

The spinlock protects the freeProcs list and some other fields in ProcGlobal, so let's put it together with those fields. It's good for cache locality to have it next to the thing it protects, and just makes more sense anyway.

Any objections?

- Heikki<0001-Move-ProcStructLock-to-the-ProcGlobal-struct.patch>

Hi Heikki,

I took a quick review. You moved ProcStructLock into PROC_HDR as freeProcsLock, and deleted:
```
ProcStructLock = ShmemInitStruct(...);
SpinLockInit(ProcStructLock);
```

But I don’t see a replacement like SpinLockInit(&ProcGlobal->freeProcsLock);

Good catch. I think the spinlock needs to be initialized somewhere in
the code block starting with
/*
* Initialize the data structures.
*/

I also checked many other shared structures which contain spinlocks in
them. All of them embed the spinlock instead of pointer to the
spinlock. This change looks inline with that.

--
Best Wishes,
Ashutosh Bapat

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:

On Wed, Feb 11, 2026 at 8:46 AM Chao Li <li.evan.chao@gmail.com> wrote:

But I don’t see a replacement like SpinLockInit(&ProcGlobal->freeProcsLock);

Good catch.

Undoubtedly, this escaped Heikki's notice because on all supported
platforms SpinLockInit() initializes the spinlock value to zero,
but shared memory starts out zeroes anyway.

We used to have better odds of catching such mistakes. My old HPPA
dinosaur would have caught it by dint of needing a nonzero initial
value, but that hardware is long gone. The test infrastructure
we used to have for emulating spinlocks with SysV semaphores would
have caught it too, I think, but that's also gone.

This is not a great situation. I wonder if we can put back some
mode that could be used by a few BF members to catch such oversights.

regards, tom lane

On 11/02/2026 13:51, Ashutosh Bapat wrote:

On Wed, Feb 11, 2026 at 8:46 AM Chao Li <li.evan.chao@gmail.com> wrote:

I took a quick review. You moved ProcStructLock into PROC_HDR as freeProcsLock, and deleted:
```
ProcStructLock = ShmemInitStruct(...);
SpinLockInit(ProcStructLock);
```

But I don’t see a replacement like SpinLockInit(&ProcGlobal->freeProcsLock);

Good catch. I think the spinlock needs to be initialized somewhere in
the code block starting with
/*
* Initialize the data structures.
*/

I also checked many other shared structures which contain spinlocks in
them. All of them embed the spinlock instead of pointer to the
spinlock. This change looks inline with that.

Fixed the initialization and pushed. Thanks!

- Heikki

On 11/02/2026 16:52, Tom Lane wrote:

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:

On Wed, Feb 11, 2026 at 8:46 AM Chao Li <li.evan.chao@gmail.com> wrote:

But I don’t see a replacement like SpinLockInit(&ProcGlobal->freeProcsLock);

Good catch.

Undoubtedly, this escaped Heikki's notice because on all supported
platforms SpinLockInit() initializes the spinlock value to zero,
but shared memory starts out zeroes anyway.

Right.

We used to have better odds of catching such mistakes. My old HPPA
dinosaur would have caught it by dint of needing a nonzero initial
value, but that hardware is long gone. The test infrastructure
we used to have for emulating spinlocks with SysV semaphores would
have caught it too, I think, but that's also gone.

This is not a great situation. I wonder if we can put back some
mode that could be used by a few BF members to catch such oversights.

Do we still support any architectures where initializing the spinlock to
all-zeros doesn't do the right thing? Could we accept that all-zeros is
a valid initialization of a spinlock? This reminds me that Thomas was
working on re-implementing spinlocks with atomics [1]/messages/by-id/CA+hUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA@mail.gmail.com. With that least,
I presume we could.

[1]: /messages/by-id/CA+hUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA@mail.gmail.com
/messages/by-id/CA+hUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA@mail.gmail.com

- Heikki

Heikki Linnakangas <hlinnaka@iki.fi> writes:

On 11/02/2026 16:52, Tom Lane wrote:

This is not a great situation. I wonder if we can put back some
mode that could be used by a few BF members to catch such oversights.

Do we still support any architectures where initializing the spinlock to
all-zeros doesn't do the right thing? Could we accept that all-zeros is
a valid initialization of a spinlock?

I'm not terribly comfortable with that: it seems short-sighted.
Even today, on platforms where we use __sync_lock_test_and_set /
__sync_lock_release, the gcc manual does not quite promise that
the released state is all-zero.

regards, tom lane