| From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Little cleanup: Move ProcStructLock to the ProcGlobal struct |
| Date: | 2026-02-11 11:51:55 |
| Message-ID: | CAExHW5tjUZdYhAGYO6V70F3X4ajBOSyCRROLLVF1dYr=khLPRQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Feb 11, 2026 at 8:46 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Feb 11, 2026, at 01:39, Heikki Linnakangas <hlinnaka(at)iki(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-02-11 11:53:44 | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |
| Previous Message | cca5507 | 2026-02-11 11:46:56 | Re: tuple radix sort |