Re: Cleanup: PGProc->links doesn't need to be the first field anymore

From: Andres Freund <andres(at)anarazel(dot)de>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Cleanup: PGProc->links doesn't need to be the first field anymore
Date: 2024-07-04 20:20:46
Message-ID: 20240704202046.ox7yj3gaback6722@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2024-07-04 01:54:18 +0300, Heikki Linnakangas wrote:
> pgproc.h has this:
>
> > struct PGPROC
> > {
> > /* proc->links MUST BE FIRST IN STRUCT (see ProcSleep,ProcWakeup,etc) */
> > dlist_node links; /* list link if process is in a list */
> > dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
> > ...
>
> I don't see any particular reason for 'links' to be the first field. We used
> to do things like "proc = (PGPROC *) waitQueue->links.next", but since
> commit 5764f611e1, this has been a "dlist", and dlist_container() can handle
> the list link being anywhere in the struct.

Indeed.

> I tried moving it and ran the regression tests. That revealed one place
> where we still don't use dlist_container:
>
> > if (!dlist_is_empty(procgloballist))
> > {
> > MyProc = (PGPROC *) dlist_pop_head_node(procgloballist);
> > ...
>
> I believe that was just an oversight. Trivial patch attached.

Oops. Yes, I clearly should have used dlist_container() here.

+1

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2024-07-04 20:36:16 Re: Linux likely() unlikely() for PostgreSQL
Previous Message Said Assemlal 2024-07-04 20:18:11 Re: CREATE OR REPLACE MATERIALIZED VIEW