MIME-Version: 1.0
References: 
 <CAPpHfdudejddHLLw=Uk_7pZ=1g2SZXfwubSgmWUhJ7fBUtR9rg@mail.gmail.com>
 <2muwyx6a5vojkg7iegknhnkcch3lfxptsxk7icwuh7szkvvu2y@vc3ukkfvnu6i>
 <CAPpHfdvucbQ9w7_eJYefUUp6w-WZUes+SRfq+GKfhukhs7kLqg@mail.gmail.com>
 <oc4iicdkwyhdf5o5vbwsl7jdlqnds37xtf27wuxvhy3abxoo6i@4ek3xp5j6niy>
 <CAPpHfdvp=_1NRF4YFFp9Oii7mRR5V4b-C2aukbuLQjWMjynYrw@mail.gmail.com>
 <aw3hirtizbn42fkl57bjeafzws3b2bvhknimbxyoi23i43sajb@i65p2ubb6zte>
In-Reply-To: <aw3hirtizbn42fkl57bjeafzws3b2bvhknimbxyoi23i43sajb@i65p2ubb6zte>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Fri, 7 Mar 2025 19:44:20 +0200
Message-ID: 
 <CAPpHfdvp3vNVp5_Rx1RtwLqhjbzzUxwkqzvsh=1E3A+iePvWBg@mail.gmail.com>
Subject: Re: pg_atomic_compare_exchange_*() and memory barriers
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAPpHfdvp3vNVp5_Rx1RtwLqhjbzzUxwkqzvsh%3D1E3A%2BiePvWBg%40mail.gmail.com>
Precedence: bulk

On Fri, Mar 7, 2025 at 7:38=E2=80=AFPM Andres Freund <andres@anarazel.de> w=
rote:
> On 2025-03-07 19:15:23 +0200, Alexander Korotkov wrote:
> > On Fri, Mar 7, 2025 at 7:07=E2=80=AFPM Andres Freund <andres@anarazel.d=
e> wrote:
> > > What is the access pattern and the observed problems with it that mad=
e you
> > > look at the disassembly?
> >
> > Check this code.
> >
> > l1:     pg_atomic_write_u64(&XLogCtl->xlblocks[nextidx], NewPageEndPtr)=
;
>
> >         /*
> >          * Try to advance XLogCtl->InitializedUpTo.
> >          *
> >          * If the CAS operation failed, then some of previous pages are=
 not
> >          * initialized yet, and this backend gives up.
> >          *
> >          * Since initializer of next page might give up on advancing of
> >          * InitializedUpTo, this backend have to attempt advancing unti=
l it
> >          * find page "in the past" or concurrent backend succeeded at
> >          * advancing.  When we finish advancing XLogCtl->InitializedUpT=
o, we
> >          * notify all the waiters with XLogCtl->InitializedUpToCondVar.
> >          */
> > l2:     while (pg_atomic_compare_exchange_u64(&XLogCtl->InitializedUpTo=
,
> > &NewPageBeginPtr, NewPageEndPtr))
> >         {
> >             NewPageBeginPtr =3D NewPageEndPtr;
> >             NewPageEndPtr =3D NewPageBeginPtr + XLOG_BLCKSZ;
> >             nextidx =3D XLogRecPtrToBufIdx(NewPageBeginPtr);
> >
> > l3:         if (pg_atomic_read_u64(&XLogCtl->xlblocks[nextidx]) !=3D
> > NewPageEndPtr)
> >             {
> >                 /*
> >                  * Page at nextidx wasn't initialized yet, so we cann't=
 move
> >                  * InitializedUpto further. It will be moved by backend
> > which
> >                  * will initialize nextidx.
> >                  */
> >
> > ConditionVariableBroadcast(&XLogCtl->InitializedUpToCondVar);
> >                 break;
> >             }
> >         }
> >
> > Consider the following execution order with process 1 (p1) and process =
2
> > (p2).
>
> On 2025-03-07 19:24:39 +0200, Alexander Korotkov wrote:
> > Sorry, I messed this up.
> > The correct sequence is following.
> >
> > 1. p1 executes l1
> > 2. p1 executes l2 with failure
> > 3. p2 executes l2 with success
> > 4. p2 execute l3, but doesn't see the results of step 1, because 3
> > didn't provide enough of memory barrier
>
> Did you mean because 2) didn't provide enough of a memory barrier? Becaus=
e 3)
> does, right?

Yes, exactly.

> You could get in exactly same the situation if the p1 is scheduled out by=
 the
> OS after step 1, no?

No. In that case, p1 will execute l2 with success.  p1 executes l2
with failure only because it goes before p2 executes l2.

------
Regards,
Alexander Korotkov
Supabase