Andres Freund <and...@anarazel.de> wrote: > On March 5, 2020 12:42:06 PM PST, Antonin Houska <a...@cybertec.at> wrote: > >Andres Freund <and...@anarazel.de> wrote: > > > >> On March 5, 2020 9:21:55 AM PST, Antonin Houska <a...@cybertec.at> > >wrote: > >> >What's the reason to use pg_atomic...read_...() and > >> >pg_atomic...write_...() > >> >functions in localbuf.c? > >> > > >> >It looks like there was an intention not to use them > >> > > >> > >>https://www.postgresql.org/message-id/CAPpHfdtfr3Aj7xJonXaKR8iY2p8uXOQ%2Be4BMpMDAM_5R4OcaDA%40mail.gmail.com > >> > > >> >but the following discussion does not explain the decision to use > >them. > >> > >> Read/write don't trigger locked/atomic operations. They just > >guarantee that > >> you're not gonna read/write a torn value. Or a cached one. Since > >> local/shared buffers share the buffer header definition, we still > >have to > >> use proper functions to access the atomic variables. > > > >Sure, the atomic operations are necessary for shared buffers, but I > >still > >don't understand why they are needed for *local* buffers - local > >buffers their > >headers (BufferDesc) in process local memory, so there should be no > >concerns > >about concurrent access. > > > >Another thing that makes me confused is this comment in > >InitLocalBuffers(): > > > > /* > > * Intentionally do not initialize the buffer's atomic variable > > * (besides zeroing the underlying memory above). That way we get > > * errors on platforms without atomics, if somebody (re-)introduces > > * atomic operations for local buffers. > > */ > > > >That sounds like there was an intention not to use the atomic functions > >in > >localbuf.c, but eventually they ended up there. Do I still miss > >something? > > Again, the read/write functions do not imply atomic instructions.
ok. What I missed is that BufferDesc.state is declared as pg_atomic_uint32 rather than plain int, so the pg_atomic_...() functions should be used regardless the buffer is shared or local. Sorry for the noise. -- Antonin Houska Web: https://www.cybertec-postgresql.com
